Do not fetch hints for ip addresses, local host, and canonical hosts

Bug: 968542
Change-Id: I5f2846630bb5ff1a4f648b59bbe332c7e557b121
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1871330Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707987}

components/optimization_guide/hints_fetcher.cc

@@ -297,8 +297,15 @@ std::vector<std::string> HintsFetcher::GetSizeLimitedHostsDueForHintsRefresh(
   target_hosts.reserve(hosts.size());
 
   for (const auto& host : hosts) {
-    // TODO(b/968542): Skip origins that are local hosts (e.g., IP addresses,
-    // localhost:8080 etc.).
+    // Skip over localhosts, IP addresses, and invalid hosts.
+    if (net::HostStringIsLocalhost(host))
+      continue;
+    url::CanonHostInfo host_info;
+    std::string canonicalized_host(net::CanonicalizeHost(host, &host_info));
+    if (host_info.IsIPAddress() ||
+        !net::IsCanonicalizedHostCompliant(canonicalized_host)) {
+      continue;
+    }
 
     bool host_hints_due_for_refresh = true;
 

components/optimization_guide/hints_fetcher_unittest.cc

@@ -480,6 +480,12 @@ TEST_F(HintsFetcherTest, MaxHostsForOptimizationGuideServiceHintsFetch) {
   base::HistogramTester histogram_tester;
   std::string response_content;
   std::vector<std::string> all_hosts;
+
+  // Invalid hosts, IP addresses, and localhosts should be skipped.
+  all_hosts.push_back("localhost");
+  all_hosts.push_back("8.8.8.8");
+  all_hosts.push_back("probably%20not%20Canonical");
+
   size_t max_hosts_in_fetch_request = optimization_guide::features::
       MaxHostsForOptimizationGuideServiceHintsFetch();
   for (size_t i = 0; i < max_hosts_in_fetch_request; ++i) {
@@ -497,12 +503,11 @@ TEST_F(HintsFetcherTest, MaxHostsForOptimizationGuideServiceHintsFetch) {
   DictionaryPrefUpdate hosts_fetched(
       pref_service(), prefs::kHintsFetcherHostsSuccessfullyFetched);
   EXPECT_EQ(max_hosts_in_fetch_request, hosts_fetched->size());
-  EXPECT_EQ(all_hosts.size(), max_hosts_in_fetch_request + 2);
+  EXPECT_EQ(all_hosts.size(), max_hosts_in_fetch_request + 5);
 
-  for (size_t i = 0; i < all_hosts.size(); ++i) {
-    // Only the first |max_hosts_in_fetch_request| should be requested.
-    EXPECT_EQ(i < max_hosts_in_fetch_request,
-              WasHostCoveredByFetch(all_hosts[i]));
+  for (size_t i = 0; i < max_hosts_in_fetch_request; ++i) {
+    EXPECT_TRUE(
+        WasHostCoveredByFetch("host" + base::NumberToString(i) + ".com"));
   }
 }