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

chrome/browser/autocomplete/history_quick_provider.cc

Index: chrome/browser/autocomplete/history_quick_provider.cc
diff --git a/chrome/browser/autocomplete/history_quick_provider.cc b/chrome/browser/autocomplete/history_quick_provider.cc
index 1871bf2ba8947a5cd7a2c5b6519d5780a542965e..b1505dd25ee6740c130b4732ff21ff48f088d6f9 100644
--- a/chrome/browser/autocomplete/history_quick_provider.cc
+++ b/chrome/browser/autocomplete/history_quick_provider.cc
@@ -14,6 +14,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
@@ -269,8 +270,11 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
     match.allowed_to_be_default_match = match.inline_autocompletion.empty() ||
         !PreventInlineAutocomplete(autocomplete_input_);
   }
+  std::vector<base::string16> words;
+  base::SplitString(autocomplete_input_.text(), ' ', &words);

[Peter Kasting, 2015/06/01 21:04:08]

I'm concerned about all these files doing a manual split based on spaces.  It
seems like we should ask ICU or other i18n code to tokenize instead, shouldn't
we?  After all, some languages (e.g. Thai) might not use whitespace to separate
words.

Separately, the fact that each file splits the input, passes the words to these
utility functions, and they search through the input tokens, seems both
cumbersome and inefficient to me.  (If the input has many tokens, this could
dramatically slow down input processing.)  Maybe we should instead be setting a
member variable on the AutocompleteInput during initial parsing like "vector of
input tokens that look like a scheme plus something".  Then we could just look
at those tokens, if any existed, in the implementation of this utility.

[Mark P, 2015/06/01 22:05:50]

I would think any decent break iterator would split on punctuation strings like
://
Even if ICU in some locales does not do that now, I don't we should trust it to
remain that way in the future.
I think splitting on spaces is reasonable.
In fact, I would not want it split more narrowly.  Imagine you have a user that
typed
http://helloworld
and we realized that hello and world should be broken based on the language
model.  Then
we would be searching for URLs that match http://hello and modifying how they're
dup-eliminated.  That doesn't seem right.
This current structure slightly bothers me too.  I definitely want to pass in
the set of tokens because sometimes computestrippedurl() is called in loops
(e.g., see shortcuts provider).  We can precompute this in AutocompleteInput but
that seems heavyhanded to me.

Looking closer, maybe I should make EnsureUWYTIsAllowedToBeDefault() do the word
splitting.  Then only the direct callers of computestrippedurl() will need to
split on input, and that merely means AutocompleteResult and ShortcutsProvider,
the two places that call this in a loop.

[Peter Kasting, 2015/06/01 22:21:58]

Fair enough.
If you make this change I'll take another look at what it does to the code. 
It's hard for me to imagine it in my head.

[Mark P, 2015/06/04 23:31:31]

Done.  Let me know what you think.

   match.EnsureUWYTIsAllowedToBeDefault(
       autocomplete_input_.canonicalized_url(),
+      words,
       TemplateURLServiceFactory::GetForProfile(profile_));
 
   // Format the description autocomplete presentation.