Google search subdomains included for Safesearch

components/google/core/browser/google_util_unittest.cc

Index: components/google/core/browser/google_util_unittest.cc
diff --git a/components/google/core/browser/google_util_unittest.cc b/components/google/core/browser/google_util_unittest.cc
index d30d03b56a5f6b8061c6c9afe33753d90dff0c13..6a36386f9c88f621f32d3e43e48c87e82d280754 100644
--- a/components/google/core/browser/google_util_unittest.cc
+++ b/components/google/core/browser/google_util_unittest.cc
@@ -64,6 +64,14 @@ TEST(GoogleUtilTest, GoodHomePagesNonSecure) {
   EXPECT_TRUE(IsHomePage("http://www.google.com/ig/foo"));
   EXPECT_TRUE(IsHomePage("http://www.google.com/ig?rlz=TEST"));
   EXPECT_TRUE(IsHomePage("http://www.google.com/ig/foo?rlz=TEST"));
+
+  // Accepted subdomains.
+  EXPECT_TRUE(IsHomePage("http://ipv4.google.com/"));
+  EXPECT_TRUE(IsHomePage("http://ipv6.google.com/"));
+
+  // Trailing dots.
+  EXPECT_TRUE(IsHomePage("http://ipv4.google.com./"));
+  EXPECT_TRUE(IsHomePage("http://google.com./"));
 }
 
 TEST(GoogleUtilTest, GoodHomePagesSecure) {
@@ -121,38 +129,52 @@ TEST(GoogleUtilTest, BadHomePages) {
 
   // Path is case sensitive.
   EXPECT_FALSE(IsHomePage("https://www.google.com/WEBHP"));
+
+  // Only .com subdomain and no www.
+  EXPECT_FALSE(IsHomePage("http://ipv4.google.co.uk"));
+  EXPECT_FALSE(IsHomePage("http://www.ipv4.google.com"));
 }
 
 TEST(GoogleUtilTest, GoodSearches) {
   const std::string patterns[] = {
-    // Queries with path "/search" need to have the query parameter in either
-    // the url parameter or the hash fragment.
-    "%s://www.google.com/search?%s=something",
-    "%s://www.google.com/search#%s=something",
-    "%s://www.google.com/search?name=bob&%s=something",
-    "%s://www.google.com/search?name=bob#%s=something",
-    "%s://www.google.com/search?name=bob#age=24&%s=thng",
-    "%s://www.google.co.uk/search?%s=something",
-    // It's actually valid for both to have the query parameter.
-    "%s://www.google.com/search?%s=something#q=other",
-
-    // Queries with path "/webhp", "/" or "" need to have the query parameter in
-    // the hash fragment.
-    "%s://www.google.com/webhp#%s=something",
-    "%s://www.google.com/webhp#name=bob&%s=something",
-    "%s://www.google.com/webhp?name=bob#%s=something",
-    "%s://www.google.com/webhp?name=bob#age=24&%s=thing",
-
-    "%s://www.google.com/#%s=something",
-    "%s://www.google.com/#name=bob&%s=something",
-    "%s://www.google.com/?name=bob#%s=something",
-    "%s://www.google.com/?name=bob#age=24&%s=something",
-
-    "%s://www.google.com#%s=something",
-    "%s://www.google.com#name=bob&%s=something",
-    "%s://www.google.com?name=bob#%s=something",
-    "%s://www.google.com?name=bob#age=24&%s=something"
-  };
+      // Queries with path "/search" need to have the query parameter in either
+      // the url parameter or the hash fragment.

[Ilya Sherman, 2017/05/11 00:33:36]

Why did you indent these lines?  Is this "git cl format"'s output?  I'm
surprised -- I don't think these need to be indented.

[igorcov, 2017/05/11 11:30:05]

For this and bellow, it's the git cl format output. I've put it all back and
ignored the warning. Do you know where could I file a bug for this?

[Peter Kasting, 2017/05/11 15:37:58]

Honestly, the new indenting looks more style-guide-correct to me, since this is
a continuation of the previous statement rather than a new scope.  I would
probably leave git cl format's output alone and not file a bug, but if you think
it's wrong I would leave it alone and file one :)

[Ilya Sherman, 2017/05/11 19:15:04]

I agree that if this is git cl format output, there's no need to change it in
this CL.  However, I do think this formatting is poor, so I filed a bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=721496

[Ilya Sherman, 2017/05/11 19:59:17]

Nico says this is working as intended, so I'll need to update my mental style
guide =)

+      "%s://www.google.com/search?%s=something",

[Ilya Sherman, 2017/05/11 00:33:36]

Hmm, what's the diff on this line?  Is the diff tool just confused, or am I? =)

+      "%s://www.google.com/search#%s=something",
+      "%s://www.google.com/search?name=bob&%s=something",
+      "%s://www.google.com/search?name=bob#%s=something",
+      "%s://www.google.com/search?name=bob#age=24&%s=thng",
+      "%s://www.google.co.uk/search?%s=something",
+      // It's actually valid for both to have the query parameter.
+      "%s://www.google.com/search?%s=something#q=other",
+
+      // Queries with path "/webhp", "/" or "" need to have the query parameter
+      // in the hash fragment.
+      "%s://www.google.com/webhp#%s=something",
+      "%s://www.google.com/webhp#name=bob&%s=something",
+      "%s://www.google.com/webhp?name=bob#%s=something",
+      "%s://www.google.com/webhp?name=bob#age=24&%s=thing",
+
+      "%s://www.google.com/#%s=something",
+      "%s://www.google.com/#name=bob&%s=something",
+      "%s://www.google.com/?name=bob#%s=something",
+      "%s://www.google.com/?name=bob#age=24&%s=something",
+
+      "%s://www.google.com#%s=something",
+      "%s://www.google.com#name=bob&%s=something",
+      "%s://www.google.com?name=bob#%s=something",
+      "%s://www.google.com?name=bob#age=24&%s=something",
+
+      // Google subdomain queries.
+      "%s://ipv4.google.com/search?%s=something",
+      "%s://ipv4.google.com#name=bob&%s=something",
+      "%s://ipv6.google.com?name=bob#%s=something",
+      "%s://ipv6.google.com?name=bob#age=24&%s=something",
+
+      // Trailing dots in the hosts.
+      "%s://www.google.com./#%s=something", "%s://www.google.de./#%s=something",

[Peter Kasting, 2017/05/11 00:48:08]

Nit: One per line

[Ilya Sherman, 2017/05/11 19:15:04]

This is also git cl format output, right?

Ah! I wonder: does adding a trailing comma cause git cl format to "do the right
thing"?

[Ilya Sherman, 2017/05/11 19:59:17]

Per the clang-format bug, it seems like a tailing comma should indeed improve
the formatting here.

+      "%s://ipv4.google.com./#%s=something",
+      "%s://ipv6.google.com./#%s=something"};
 
   for (const std::string& pattern : patterns) {
     for (const std::string& scheme : kValidSearchSchemes) {