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

chrome/browser/ui/omnibox/omnibox_edit_model.cc

Index: chrome/browser/ui/omnibox/omnibox_edit_model.cc
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 5f6160ff4b06e760bff38f3b106ef0f56088c6a7..69855dc60f34a853ad47e7c109b3a9010032603e 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -19,6 +19,7 @@
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
+#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"
 #include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/bookmark_stats.h"
@@ -784,7 +785,8 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
   // entry of the match used.
   ACMatches fake_single_entry_matches;
   fake_single_entry_matches.push_back(match);
-  AutocompleteResult fake_single_entry_result;
+  AutocompleteResult fake_single_entry_result(
+      make_scoped_ptr(new ChromeAutocompleteProviderClient(profile_)).get());

[Peter Kasting, 2015/06/29 05:04:59]

This seems kind of scary.  I think the scope of the temporary scoped_ptr you
create only extends to the end of this statement?  In which case the
AutocompleteResult is now holding a dangling raw pointer to a deleted object.

There are a whole variety of potential fixes here, but honestly, I don't think
AutocompleteResult should take a pointer to this object at all.  This is partly
because construction here is a bit awkward, partly because
AutocompleteProviderClient is meant to be a "provider client" (see name) and not
a "result client" or "general autocomplete system client", but mostly because
the only actual use of this is to get the correct languages to use, and
dependency injection principles suggest that, in that case, it would be better
to pass in the languages to use, or even better, to pass those directly to the
function that needs them, AutocompleteResult::SortAndCull(). Doing this requires
touching a dozen or so callsites, but outside of tests, only a couple matter:
one call from the AutocompleteController, and one call from CopyOldMatches(),
which is in turn called from the AutocompleteController.  The controller, of
course, owns the main provider client already and can get the necessary
languages directly.  And while this does require changing some callsites in
tests, it also allows you to avoid adding mock provider client construction to
those same tests.  Yay.

[Mark P, 2015/06/30 04:23:17]

That's a good alternate design.  I thought of one alternate design involving
passing
languages, but that involved passing languages into the AutocompleteResult
constructor (a la how this changelist passes the AutocompleteProviderClient).
However, I realize that doesn't work well because the user's language may change
and we only construct AutocompleteResult once (when we build the controller).
I'm embarrassed I didn't think of your suggestion, simply passing it directly
into ComputeStrippedDestinationUrl and friends.

Done.

   fake_single_entry_result.AppendMatches(input_, fake_single_entry_matches);
   OmniboxLog log(
       input_text,