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

chrome/browser/ui/omnibox/omnibox_edit_model.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/omnibox/omnibox_edit_model.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/auto_reset.h"
 #include "base/format_macros.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #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"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/extensions/api/omnibox/omnibox_api.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor_factory.h"
 #include "chrome/browser/prerender/prerender_field_trial.h"
@@ skipping 745 matching lines @@
         base::TimeDelta::FromMilliseconds(-1);
     elapsed_time_since_user_first_modified_omnibox = default_time_delta;
     elapsed_time_since_last_change_to_default_match = default_time_delta;
   }
   // If the popup is closed or this is a paste-and-go action (meaning the
   // contents of the dropdown are ignored regardless), we record for logging
   // purposes a selected_index of 0 and a suggestion list as having a single
   // 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,
       just_deleted_text_,
       input_.type(),
       popup_model()->IsOpen(),
       (!popup_model()->IsOpen() || !pasted_text.empty()) ? 0 : index,
       !pasted_text.empty(),
       -1,  // don't yet know tab ID; set later if appropriate
       ClassifyPage(),
@@ skipping 739 matching lines @@
   // Update state and notify view if the omnibox has focus and the caret
   // visibility changed.
   const bool was_caret_visible = is_caret_visible();
   focus_state_ = state;
   if (focus_state_ != OMNIBOX_FOCUS_NONE &&
       is_caret_visible() != was_caret_visible)
     view_->ApplyCaretVisibility();
 
   delegate_->OnFocusChanged(focus_state_, reason);
 }