Add port number to redirect table

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

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

chrome/browser/predictors/loading_test_util.cc

@@ -37,6 +37,7 @@ void InitializeRedirectStat(RedirectStat* redirect,
                             int consecutive_misses) {
   redirect->set_url(url.host());
   redirect->set_url_scheme(url.scheme());
+  redirect->set_url_port(url.EffectiveIntPort());
   redirect->set_number_of_hits(number_of_hits);
   redirect->set_number_of_misses(number_of_misses);
   redirect->set_consecutive_misses(consecutive_misses);
@@ -170,8 +171,9 @@ std::ostream& operator<<(std::ostream& os, const RedirectData& data) {
 
 std::ostream& operator<<(std::ostream& os, const RedirectStat& redirect) {
   return os << "[" << redirect.url() << "," << redirect.url_scheme() << ","
-            << redirect.number_of_hits() << "," << redirect.number_of_misses()
-            << "," << redirect.consecutive_misses() << "]";
+            << redirect.url_port() << "," << redirect.number_of_hits() << ","
+            << redirect.number_of_misses() << ","
+            << redirect.consecutive_misses() << "]";
 }
 
 std::ostream& operator<<(std::ostream& os, const OriginData& data) {
@@ -239,6 +241,7 @@ bool operator==(const RedirectData& lhs, const RedirectData& rhs) {
 
 bool operator==(const RedirectStat& lhs, const RedirectStat& rhs) {
   return lhs.url() == rhs.url() && lhs.url_scheme() == rhs.url_scheme() &&
+         lhs.url_port() == rhs.url_port() &&
          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/resource_prefetch_predictor.cc

@@ -296,6 +296,7 @@ void ResourcePrefetchPredictor::LearnRedirect(const std::string& key,
                                               const GURL& final_redirect,
                                               RedirectDataMap* redirect_data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   // If the primary key is too long reject it.
   if (key.length() > ResourcePrefetchPredictorTables::kMaxStringLength)
     return;
@@ -309,6 +310,7 @@ void ResourcePrefetchPredictor::LearnRedirect(const std::string& key,
     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());
+    redirect_to_add->set_url_port(final_redirect.EffectiveIntPort());
   } else {
     data.set_last_visit_time(base::Time::Now().ToInternalValue());
 
@@ -325,15 +327,26 @@ void ResourcePrefetchPredictor::LearnRedirect(const std::string& key,
           !redirect.url_scheme().empty() &&
           redirect.url_scheme() != final_redirect.scheme();
 
-      if (!host_mismatch && !url_scheme_mismatch) {
+      // When the existing port in database is empty, then difference in
+      // ports is not considered a mismatch. This case is treated
+      // specially since port was added later to the database, and previous
+      // entries would have empty value. In such case, we simply update the port
+      // in the database.
+      const bool url_port_mismatch =
+          redirect.has_url_port() &&
+          redirect.url_port() != final_redirect.EffectiveIntPort();
+
+      if (!host_mismatch && !url_scheme_mismatch && !url_port_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 existing scheme or port in database are empty, then update them.
         if (redirect.url_scheme().empty())
           redirect.set_url_scheme(final_redirect.scheme());
+        if (!redirect.has_url_port())
+          redirect.set_url_port(final_redirect.EffectiveIntPort());
       } else {
         // A real mismatch.
         redirect.set_number_of_misses(redirect.number_of_misses() + 1);
@@ -346,6 +359,7 @@ void ResourcePrefetchPredictor::LearnRedirect(const std::string& key,
       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());
+      redirect_to_add->set_url_port(final_redirect.EffectiveIntPort());
     }
   }
 

chrome/browser/predictors/resource_prefetch_predictor.proto

@@ -27,6 +27,10 @@ message RedirectStat {
   // 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;
+  // |url_port| 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
+  // port number.
+  optional int32 url_port = 6;
 }
 
 // Represents a mapping from URL or host to a list of redirect endpoints.

chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/sequenced_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "chrome/browser/predictors/loading_test_util.h"
@@ -302,7 +303,8 @@ void ResourcePrefetchPredictorTablesTest::AddKey(OriginDataMap* m,
 
 std::string ResourcePrefetchPredictorTablesTest::GetKeyForRedirectStat(
     const RedirectStat& stat) const {
-  return stat.url() + "," + stat.url_scheme();
+  return stat.url() + "," + stat.url_scheme() + "," +
+         base::NumberToString(stat.url_port());
 }
 
 void ResourcePrefetchPredictorTablesTest::DeleteAllData() {

chrome/browser/predictors/resource_prefetch_predictor_unittest.cc

@@ -535,6 +535,10 @@ TEST_F(ResourcePrefetchPredictorTest, RedirectUrlInDB_MultipleSchemes) {
                            .data_[host_redirect_data_https.primary_key()]
                            .redirect_endpoints(0)
                            .url_scheme());
+    EXPECT_EQ(443, mock_tables_->host_redirect_table_
+                       .data_[host_redirect_data_https.primary_key()]
+                       .redirect_endpoints(0)
+                       .url_port());
   }
   {
     std::vector<content::mojom::ResourceLoadInfoPtr> resources_http;
@@ -569,6 +573,10 @@ TEST_F(ResourcePrefetchPredictorTest, RedirectUrlInDB_MultipleSchemes) {
                           .data_[host_redirect_data_http.primary_key()]
                           .redirect_endpoints(1)
                           .url_scheme());
+    EXPECT_EQ(80, mock_tables_->host_redirect_table_
+                      .data_[host_redirect_data_http.primary_key()]
+                      .redirect_endpoints(1)
+                      .url_port());
   }
 }