Add scheme to redirect table

Add the redirect URL scheme to the redirect learning table.
Next CL will use the value of the scheme for preconnect.

Change-Id: I9b129a5a2f1dce3f378f1a44f7e92587f84eae03
Bug: 983234
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1700886
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680059}

chrome/browser/predictors/loading_test_util.cc

@@ -31,11 +31,12 @@ MockResourcePrefetchPredictor::MockResourcePrefetchPredictor(
 MockResourcePrefetchPredictor::~MockResourcePrefetchPredictor() = default;
 
 void InitializeRedirectStat(RedirectStat* redirect,
-                            const std::string& url,
+                            const GURL& url,
                             int number_of_hits,
                             int number_of_misses,
                             int consecutive_misses) {
-  redirect->set_url(url);
+  redirect->set_url(url.host());
+  redirect->set_url_scheme(url.scheme());
   redirect->set_number_of_hits(number_of_hits);
   redirect->set_number_of_misses(number_of_misses);
   redirect->set_consecutive_misses(consecutive_misses);
@@ -168,9 +169,9 @@ std::ostream& operator<<(std::ostream& os, const RedirectData& data) {
 }
 
 std::ostream& operator<<(std::ostream& os, const RedirectStat& redirect) {
-  return os << "[" << redirect.url() << "," << redirect.number_of_hits() << ","
-            << redirect.number_of_misses() << ","
-            << redirect.consecutive_misses() << "]";
+  return os << "[" << redirect.url() << "," << redirect.url_scheme() << ","
+            << redirect.number_of_hits() << "," << redirect.number_of_misses()
+            << "," << redirect.consecutive_misses() << "]";
 }
 
 std::ostream& operator<<(std::ostream& os, const OriginData& data) {
@@ -237,7 +238,7 @@ bool operator==(const RedirectData& lhs, const RedirectData& rhs) {
 }
 
 bool operator==(const RedirectStat& lhs, const RedirectStat& rhs) {
-  return lhs.url() == rhs.url() &&
+  return lhs.url() == rhs.url() && lhs.url_scheme() == rhs.url_scheme() &&
          lhs.number_of_hits() == rhs.number_of_hits() &&
          lhs.number_of_misses() == rhs.number_of_misses() &&
          lhs.consecutive_misses() == rhs.consecutive_misses();

chrome/browser/predictors/loading_test_util.h

@@ -36,7 +36,7 @@ class MockResourcePrefetchPredictor : public ResourcePrefetchPredictor {
 };
 
 void InitializeRedirectStat(RedirectStat* redirect,
-                            const std::string& url,
+                            const GURL& url,
                             int number_of_hits,
                             int number_of_misses,
                             int consecutive_misses);

chrome/browser/predictors/resource_prefetch_predictor.cc

@@ -193,9 +193,10 @@ void ResourcePrefetchPredictor::RecordPageRequestSummary(
     return;
   }
 
-  const std::string& host = summary->main_frame_url.host();
-  LearnRedirect(summary->initial_url.host(), host, host_redirect_data_.get());
-  LearnOrigins(host, summary->main_frame_url.GetOrigin(), summary->origins);
+  LearnRedirect(summary->initial_url.host(), summary->main_frame_url,
+                host_redirect_data_.get());
+  LearnOrigins(summary->main_frame_url.host(),
+               summary->main_frame_url.GetOrigin(), summary->origins);
 
   if (observer_)
     observer_->OnNavigationLearned(*summary);
@@ -292,7 +293,7 @@ void ResourcePrefetchPredictor::DeleteUrls(const history::URLRows& urls) {
 }
 
 void ResourcePrefetchPredictor::LearnRedirect(const std::string& key,
-                                              const std::string& final_redirect,
+                                              const GURL& final_redirect,
                                               RedirectDataMap* redirect_data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // If the primary key is too long reject it.
@@ -305,18 +306,36 @@ void ResourcePrefetchPredictor::LearnRedirect(const std::string& key,
     data.set_primary_key(key);
     data.set_last_visit_time(base::Time::Now().ToInternalValue());
     RedirectStat* redirect_to_add = data.add_redirect_endpoints();
-    redirect_to_add->set_url(final_redirect);
+    redirect_to_add->set_url(final_redirect.host());
     redirect_to_add->set_number_of_hits(1);
+    redirect_to_add->set_url_scheme(final_redirect.scheme());
   } else {
     data.set_last_visit_time(base::Time::Now().ToInternalValue());
 
     bool need_to_add = true;
     for (RedirectStat& redirect : *(data.mutable_redirect_endpoints())) {
-      if (redirect.url() == final_redirect) {
+      const bool host_mismatch = redirect.url() != final_redirect.host();
+
+      // When the existing scheme in database is empty, then difference in
+      // schemes is not considered a scheme mismatch. This case is treated
+      // specially since scheme was added later to the database, and previous
+      // entries would have empty scheme. In such case, we do not consider this
+      // as a mismatch, and simply update the scheme in the database.
+      const bool url_scheme_mismatch =
+          !redirect.url_scheme().empty() &&
+          redirect.url_scheme() != final_redirect.scheme();
+
+      if (!host_mismatch && !url_scheme_mismatch) {
+        // No mismatch.
         need_to_add = false;
         redirect.set_number_of_hits(redirect.number_of_hits() + 1);
         redirect.set_consecutive_misses(0);
+
+        // If existing scheme in database is empty, then update it.
+        if (redirect.url_scheme().empty())
+          redirect.set_url_scheme(final_redirect.scheme());
       } else {
+        // A real mismatch.
         redirect.set_number_of_misses(redirect.number_of_misses() + 1);
         redirect.set_consecutive_misses(redirect.consecutive_misses() + 1);
       }
@@ -324,8 +343,9 @@ void ResourcePrefetchPredictor::LearnRedirect(const std::string& key,
 
     if (need_to_add) {
       RedirectStat* redirect_to_add = data.add_redirect_endpoints();
-      redirect_to_add->set_url(final_redirect);
+      redirect_to_add->set_url(final_redirect.host());
       redirect_to_add->set_number_of_hits(1);
+      redirect_to_add->set_url_scheme(final_redirect.scheme());
     }
   }
 

chrome/browser/predictors/resource_prefetch_predictor.h

@@ -237,7 +237,7 @@ class ResourcePrefetchPredictor : public history::HistoryServiceObserver {
   // Updates information about final redirect destination for the |key| in
   // |redirect_data| and correspondingly updates the predictor database.
   void LearnRedirect(const std::string& key,
-                     const std::string& final_redirect,
+                     const GURL& final_redirect,
                      RedirectDataMap* redirect_data);
 
   void LearnOrigins(const std::string& host,

chrome/browser/predictors/resource_prefetch_predictor.proto

@@ -14,12 +14,19 @@ package predictors;
 option optimize_for = LITE_RUNTIME;
 
 // Represents a single redirect chain endpoint.
+// When adding a field here, please also update the equality operator and the
+// output operator in
+// chrome/browser/predictors/loading_test_util.cc.
 message RedirectStat {
   // Represents the host for RedirectData in a host-based table.
   optional string url = 1;
   optional uint32 number_of_hits = 2;
   optional uint32 number_of_misses = 3;
   optional uint32 consecutive_misses = 4;
+  // |url_scheme| is typically either "http" or "https". This field was added
+  // in M-77 without wiping the database. As such, it may be empty in some
+  // cases even when originally |url| had a non-empty scheme.
+  optional string url_scheme = 5;
 }
 
 // Represents a mapping from URL or host to a list of redirect endpoints.

chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc

@@ -69,9 +69,11 @@ class ResourcePrefetchPredictorTablesTest : public testing::Test {
   void TestOriginStatsAreEqual(const std::vector<OriginStat>& lhs,
                                const std::vector<OriginStat>& rhs) const;
 
-  void AddKey(RedirectDataMap* m, const std::string& key) const;
+  void AddKey(RedirectDataMap* m, const GURL& url) const;
   void AddKey(OriginDataMap* m, const std::string& key) const;
 
+  std::string GetKeyForRedirectStat(const RedirectStat& stat) const;
+
   RedirectDataMap test_host_redirect_data_;
   OriginDataMap test_origin_data_;
 };
@@ -140,7 +142,7 @@ void ResourcePrefetchPredictorTablesTest::TestDeleteData() {
 
   RedirectDataMap expected_host_redirect_data;
   OriginDataMap expected_origin_data;
-  AddKey(&expected_host_redirect_data, "bbc.com");
+  AddKey(&expected_host_redirect_data, GURL("http://bbc.com"));
   AddKey(&expected_origin_data, "abc.xyz");
 
   TestRedirectDataAreEqual(expected_host_redirect_data,
@@ -150,10 +152,10 @@ void ResourcePrefetchPredictorTablesTest::TestDeleteData() {
 
 void ResourcePrefetchPredictorTablesTest::TestUpdateData() {
   RedirectData microsoft = CreateRedirectData("microsoft.com", 21);
-  InitializeRedirectStat(microsoft.add_redirect_endpoints(), "m.microsoft.com",
-                         5, 7, 1);
-  InitializeRedirectStat(microsoft.add_redirect_endpoints(), "microsoft.org", 7,
-                         2, 0);
+  InitializeRedirectStat(microsoft.add_redirect_endpoints(),
+                         GURL("https://m.microsoft.com"), 5, 7, 1);
+  InitializeRedirectStat(microsoft.add_redirect_endpoints(),
+                         GURL("https://microsoft.org"), 7, 2, 0);
 
   tables_->ExecuteDBTaskOnDBSequence(
       base::BindOnce(&LoadingPredictorKeyValueTable<RedirectData>::UpdateData,
@@ -174,7 +176,7 @@ void ResourcePrefetchPredictorTablesTest::TestUpdateData() {
   RedirectDataMap expected_host_redirect_data;
   OriginDataMap expected_origin_data;
 
-  AddKey(&expected_host_redirect_data, "bbc.com");
+  AddKey(&expected_host_redirect_data, GURL("https://bbc.com"));
   expected_host_redirect_data.insert(
       std::make_pair("microsoft.com", microsoft));
 
@@ -225,16 +227,19 @@ void ResourcePrefetchPredictorTablesTest::TestRedirectsAreEqual(
 
   std::map<std::string, RedirectStat> lhs_index;
   // Repeated redirects are not allowed.
-  for (const auto& r : lhs)
-    EXPECT_TRUE(lhs_index.insert(std::make_pair(r.url(), r)).second);
+  for (const auto& r : lhs) {
+    EXPECT_TRUE(
+        lhs_index.insert(std::make_pair(GetKeyForRedirectStat(r), r)).second)
+        << " r.url()=" << r.url();
+  }
 
   for (const auto& r : rhs) {
-    auto lhs_it = lhs_index.find(r.url());
+    auto lhs_it = lhs_index.find(GetKeyForRedirectStat(r));
     if (lhs_it != lhs_index.end()) {
       EXPECT_EQ(r, lhs_it->second);
       lhs_index.erase(lhs_it);
     } else {
-      ADD_FAILURE() << r.url();
+      ADD_FAILURE() << r.url() << " " << r.url_scheme();
     }
   }
 
@@ -282,8 +287,8 @@ void ResourcePrefetchPredictorTablesTest::TestOriginStatsAreEqual(
 }
 
 void ResourcePrefetchPredictorTablesTest::AddKey(RedirectDataMap* m,
-                                                 const std::string& key) const {
-  auto it = test_host_redirect_data_.find(key);
+                                                 const GURL& url) const {
+  auto it = test_host_redirect_data_.find(url.host());
   EXPECT_TRUE(it != test_host_redirect_data_.end());
   m->insert(*it);
 }
@@ -295,6 +300,11 @@ void ResourcePrefetchPredictorTablesTest::AddKey(OriginDataMap* m,
   m->insert(*it);
 }
 
+std::string ResourcePrefetchPredictorTablesTest::GetKeyForRedirectStat(
+    const RedirectStat& stat) const {
+  return stat.url() + "," + stat.url_scheme();
+}
+
 void ResourcePrefetchPredictorTablesTest::DeleteAllData() {
   tables_->ExecuteDBTaskOnDBSequence(base::BindOnce(
       &LoadingPredictorKeyValueTable<RedirectData>::DeleteAllData,
@@ -318,14 +328,22 @@ void ResourcePrefetchPredictorTablesTest::GetAllData(
 void ResourcePrefetchPredictorTablesTest::InitializeSampleData() {
   {  // Host redirect data.
     RedirectData bbc = CreateRedirectData("bbc.com", 9);
-    InitializeRedirectStat(bbc.add_redirect_endpoints(), "www.bbc.com", 8, 4,
-                           1);
-    InitializeRedirectStat(bbc.add_redirect_endpoints(), "m.bbc.com", 5, 8, 0);
-    InitializeRedirectStat(bbc.add_redirect_endpoints(), "bbc.co.uk", 1, 3, 0);
+    InitializeRedirectStat(bbc.add_redirect_endpoints(),
+                           GURL("https://www.bbc.com"), 8, 4, 1);
+    InitializeRedirectStat(bbc.add_redirect_endpoints(),
+                           GURL("https://m.bbc.com"), 5, 8, 0);
+    InitializeRedirectStat(bbc.add_redirect_endpoints(),
+                           GURL("https://bbc.co.uk"), 1, 3, 0);
+    InitializeRedirectStat(bbc.add_redirect_endpoints(),
+                           GURL("http://www.bbc.com"), 8, 4, 1);
+    InitializeRedirectStat(bbc.add_redirect_endpoints(),
+                           GURL("http://m.bbc.com"), 5, 8, 0);
+    InitializeRedirectStat(bbc.add_redirect_endpoints(),
+                           GURL("http://bbc.co.uk"), 1, 3, 0);
 
     RedirectData microsoft = CreateRedirectData("microsoft.com", 10);
     InitializeRedirectStat(microsoft.add_redirect_endpoints(),
-                           "www.microsoft.com", 10, 0, 0);
+                           GURL("https://www.microsoft.com"), 10, 0, 0);
 
     test_host_redirect_data_.clear();
     test_host_redirect_data_.insert(std::make_pair(bbc.primary_key(), bbc));