[HaTS] Fix survey check url

The url used by HaTS survey status check was incorrect due to a missing
slash before the path. This CL also added a test to verify the url.

BUG=1048385

Change-Id: I6ef6e29a13f61a56147bfd258af101cd103bbadc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2114163
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753700}

chrome/browser/ui/hats/hats_survey_status_checker.cc

@@ -69,8 +69,8 @@ void HatsSurveyStatusChecker::CheckSurveyStatus(
 
   // Send the request and check the response header.
   auto request = std::make_unique<network::ResourceRequest>();
-  std::string url(HatsSurveyURL());
-  request->url = GURL(url + site_id);
+  std::string url_without_id(HatsSurveyURLWithoutId());
+  request->url = GURL(url_without_id + site_id);
   // Send stored cookie along with the request, but don't save any cookie.
   request->attach_same_site_cookies = true;
   request->load_flags = net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
@@ -119,8 +119,8 @@ base::OnceClosure HatsSurveyStatusChecker::CreateTimeoutCallbackForTesting() {
 
 HatsSurveyStatusChecker::HatsSurveyStatusChecker() = default;
 
-std::string HatsSurveyStatusChecker::HatsSurveyURL() {
-  std::string url("https://www.google.com");
+std::string HatsSurveyStatusChecker::HatsSurveyURLWithoutId() {
+  std::string url("https://www.google.com/");
   return url + kHatsSurveyDataPath;
 }
 

chrome/browser/ui/hats/hats_survey_status_checker.h

@@ -54,7 +54,7 @@ class HatsSurveyStatusChecker {
   HatsSurveyStatusChecker();
 
   // Overridden only by tests.
-  virtual std::string HatsSurveyURL();
+  virtual std::string HatsSurveyURLWithoutId();
   virtual int SurveyCheckTimeoutSecs();
 
  private:

chrome/browser/ui/hats/hats_survey_status_checker_browsertest.cc

@@ -24,20 +24,34 @@
 
 namespace {
 
-class FakeHatsSurveyStatusChecker : public HatsSurveyStatusChecker {
+class TestHatsSurveyStatusChecker : public HatsSurveyStatusChecker {
+ public:
+  explicit TestHatsSurveyStatusChecker(Profile* profile)
+      : HatsSurveyStatusChecker(profile) {}
+  TestHatsSurveyStatusChecker(const TestHatsSurveyStatusChecker&) = delete;
+  TestHatsSurveyStatusChecker& operator=(const TestHatsSurveyStatusChecker&) =
+      delete;
+  ~TestHatsSurveyStatusChecker() override = default;
+
+  std::string HatsSurveyURLWithoutId() override {
+    return HatsSurveyStatusChecker::HatsSurveyURLWithoutId();
+  }
+};
+
+class FakeHatsSurveyStatusChecker : public TestHatsSurveyStatusChecker {
  public:
   static constexpr int kTimeoutSecs = 1;
 
   FakeHatsSurveyStatusChecker(Profile* profile, const std::string& url)
-      : HatsSurveyStatusChecker(profile), url_(url) {}
+      : TestHatsSurveyStatusChecker(profile), url_(url) {}
   FakeHatsSurveyStatusChecker(const FakeHatsSurveyStatusChecker&) = delete;
-  ~FakeHatsSurveyStatusChecker() override = default;
-
   FakeHatsSurveyStatusChecker& operator=(const FakeHatsSurveyStatusChecker&) =
       delete;
+  ~FakeHatsSurveyStatusChecker() override = default;
 
- protected:
-  std::string HatsSurveyURL() override { return url_ + kHatsSurveyDataPath; }
+  std::string HatsSurveyURLWithoutId() override {
+    return url_ + kHatsSurveyDataPath;
+  }
   int SurveyCheckTimeoutSecs() override { return kTimeoutSecs; }
 
  private:
@@ -54,10 +68,9 @@ class HatsSurveyStatusCheckerBrowserTestBase : public InProcessBrowserTest {
   HatsSurveyStatusCheckerBrowserTestBase() = default;
   HatsSurveyStatusCheckerBrowserTestBase(
       const HatsSurveyStatusCheckerBrowserTestBase&) = delete;
-  ~HatsSurveyStatusCheckerBrowserTestBase() override = default;
-
   HatsSurveyStatusCheckerBrowserTestBase& operator=(
       const HatsSurveyStatusCheckerBrowserTestBase&) = delete;
+  ~HatsSurveyStatusCheckerBrowserTestBase() override = default;
 
   // Handles |request| by serving different response based on the site id in the
   // request.
@@ -135,6 +148,21 @@ constexpr char HatsSurveyStatusCheckerBrowserTestBase::kErrorSiteId[];
 constexpr char HatsSurveyStatusCheckerBrowserTestBase::kOutOfCapacitySiteId[];
 constexpr char HatsSurveyStatusCheckerBrowserTestBase::kNormalSiteId[];
 
+// Check the url is formatted correctly such as its host name is separated from
+// the path. So we can extract the expected host name from the url.
+IN_PROC_BROWSER_TEST_F(HatsSurveyStatusCheckerBrowserTestBase,
+                       CheckHostInStatusURLCorrect) {
+  auto checker =
+      std::make_unique<TestHatsSurveyStatusChecker>(browser()->profile());
+  GURL url(checker->HatsSurveyURLWithoutId());
+  EXPECT_STREQ("www.google.com", url.host().c_str());
+
+  auto fake_checker = std::make_unique<FakeHatsSurveyStatusChecker>(
+      browser()->profile(), embedded_test_server()->base_url().spec());
+  GURL test_url(fake_checker->HatsSurveyURLWithoutId());
+  EXPECT_EQ(embedded_test_server()->base_url().host(), test_url.host());
+}
+
 IN_PROC_BROWSER_TEST_F(HatsSurveyStatusCheckerBrowserTestBase,
                        CheckStatusResults) {
   struct SiteResult {
@@ -150,9 +178,8 @@ IN_PROC_BROWSER_TEST_F(HatsSurveyStatusCheckerBrowserTestBase,
       {HatsSurveyStatusCheckerBrowserTestBase::kNormalSiteId,
        HatsSurveyStatusChecker::Status::kSuccess}};
 
-  std::unique_ptr<FakeHatsSurveyStatusChecker> checker =
-      std::make_unique<FakeHatsSurveyStatusChecker>(
-          browser()->profile(), embedded_test_server()->base_url().spec());
+  auto checker = std::make_unique<FakeHatsSurveyStatusChecker>(
+      browser()->profile(), embedded_test_server()->base_url().spec());
   for (const auto& item : site_id_with_result) {
     base::RunLoop request_wait;
     SetClosure(request_wait.QuitClosure(),