Omnibox - Do Not Allow HTTP/HTTPS Equivalence if User Explicitly Entered A Scheme

components/omnibox/autocomplete_match.cc

Index: components/omnibox/autocomplete_match.cc
diff --git a/components/omnibox/autocomplete_match.cc b/components/omnibox/autocomplete_match.cc
index d19f8e9b56031961555e2f2db05fc075080bfaa3..1195860f74d8642d1eb70a3b864029ee71a25cef 100644
--- a/components/omnibox/autocomplete_match.cc
+++ b/components/omnibox/autocomplete_match.cc
@@ -9,6 +9,7 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
@@ -26,6 +27,32 @@ bool IsTrivialClassification(const ACMatchClassifications& classifications) {
        (classifications.back().style == ACMatchClassification::NONE));
 }
 
+// Returns true if one of the input words includes a scheme and followed by
+// something that matches at the beginning of the URL's hostname.  This
+// indicates that the user desires this URL loaded with this scheme in

[Peter Kasting, 2015/06/06 01:31:24]

Nit: "This suggests that the user may want this particular scheme, and the
caller should not consider another URL like this one but with a different scheme
to be a duplicate."

[Mark P, 2015/06/09 19:29:38]

Done.

+// particular.
+bool InputIncludesSchemePlusPartOfHost(
+    const std::vector<base::string16>& input_words,
+    const GURL& url) {
+  // Find scheme separator and see if the stuff following it matches the
+  // beginning of the URL (after the scheme).
+  const base::string16& separator_as_utf16 =
+      base::UTF8ToUTF16(url::kStandardSchemeSeparator);
+  const base::string16& url_spec_without_scheme_as_utf16 =
+      base::UTF8ToUTF16(url.spec().substr(
+          url.scheme().length() +
+          std::string(url::kStandardSchemeSeparator).length()));
+  for (auto it : input_words) {
+    const size_t pos = it.find(separator_as_utf16);
+    if ((pos != base::string16::npos) && (pos > 0) &&

[Peter Kasting, 2015/06/06 01:31:23]

I'm still thinking about this (pos > 0) check.

I think we should actually look for "http://" and "https://" instead of just
"<anything>://".  If I type in "ftp://mozilla.com", it seems OK -- preferable,
even -- to dupe http://mozilla.com and https://mozilla.com together.  This is
even more true if I type garbage like "!@#://mozilla.com".

[Mark P, 2015/06/06 17:10:11]

Your proposal requires explicitly looking at the scheme and making decisions
based on it.  In other words, if the users enters "http" or "https" you want to
disable dedupping those two things.  There is no where to implement your code
without explicitly using those URLs constants.  Do you really want me to do such
a narrow solution?  i.e., it won't apply by default to new protocols we
introduce.

[Peter Kasting, 2015/06/06 18:38:16]

Yes.  This should be in sync with the actual scheme manipulation we do in
ComputeStrippedDestinationURL().  Right now we only convert HTTPS->HTTP, so
those are the schemes we should look at here.  If we were to add more schemes
there we would want to add them here too.  (We could even add a comment in one
or both of these places talking about the need to stay in sync.)

[Mark P, 2015/06/09 19:29:38]

Acknowledged.

+        StartsWith(url_spec_without_scheme_as_utf16,
+                  it.substr(pos + separator_as_utf16.length()), false)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 // AutocompleteMatch ----------------------------------------------------------
@@ -370,6 +397,7 @@ TemplateURL* AutocompleteMatch::GetTemplateURLWithKeyword(
 // static
 GURL AutocompleteMatch::GURLToStrippedGURL(
     const GURL& url,
+    const std::vector<base::string16>& input_words,
     TemplateURLService* template_url_service,
     const base::string16& keyword) {
   if (!url.is_valid())
@@ -415,8 +443,9 @@ GURL AutocompleteMatch::GURLToStrippedGURL(
     needs_replacement = true;
   }
 
-  // Replace https protocol with http protocol.
-  if (stripped_destination_url.SchemeIs(url::kHttpsScheme)) {
+  // Possibly replace https protocol with http protocol.

[Peter Kasting, 2015/06/06 01:31:23]

Nit: "Replace https protocol with http, as long as the user didn't explicitly
specify one of the two."  (Goes along with my above suggestion)

[Mark P, 2015/06/09 19:29:38]

Done.

+  if (!InputIncludesSchemePlusPartOfHost(input_words, url) &&
+      stripped_destination_url.SchemeIs(url::kHttpsScheme)) {
     replacements.SetScheme(url::kHttpScheme,
                            url::Component(0, strlen(url::kHttpScheme)));
     needs_replacement = true;
@@ -429,24 +458,36 @@ GURL AutocompleteMatch::GURLToStrippedGURL(
 }
 
 void AutocompleteMatch::ComputeStrippedDestinationURL(
+    const std::vector<base::string16>& input_words,
     TemplateURLService* template_url_service) {
-  stripped_destination_url =
-      GURLToStrippedGURL(destination_url, template_url_service, keyword);
+  stripped_destination_url = GURLToStrippedGURL(
+      destination_url, input_words, template_url_service, keyword);
 }
 
 void AutocompleteMatch::EnsureUWYTIsAllowedToBeDefault(
     const GURL& canonical_input_url,
+    const std::vector<base::string16>& input_words,
     TemplateURLService* template_url_service) {
   if (!allowed_to_be_default_match) {
     const GURL& stripped_canonical_input_url =
         AutocompleteMatch::GURLToStrippedGURL(
-            canonical_input_url, template_url_service, base::string16());
-    ComputeStrippedDestinationURL(template_url_service);
+            canonical_input_url, input_words,
+            template_url_service, base::string16());
+    ComputeStrippedDestinationURL(input_words, template_url_service);
     allowed_to_be_default_match =
         stripped_canonical_input_url == stripped_destination_url;
   }
 }
 
+void AutocompleteMatch::EnsureUWYTIsAllowedToBeDefault(
+    const AutocompleteInput& input,
+    TemplateURLService* template_url_service) {
+  std::vector<base::string16> words;
+  base::SplitString(input.text(), ' ', &words);
+  EnsureUWYTIsAllowedToBeDefault(
+      input.canonicalized_url(), words, template_url_service);
+}
+
 void AutocompleteMatch::GetKeywordUIState(
     TemplateURLService* template_url_service,
     base::string16* keyword,