Fix ZeroSuggestProvider bug where hitting ctrl-L then delete won't erase the URL from the omnibox.

Fix DCHECK failure unsanitized URL description.

BUG=237284

Review URL: https://chromiumcodereview.appspot.com/17170003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@208369 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 35 insertions, 79 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_browsertest.cc b/chrome/browser/autocomplete/autocomplete_browsertest.cc
index 510d82b..8161ce37 100644
--- a/chrome/browser/autocomplete/autocomplete_browsertest.cc
+++ b/chrome/browser/autocomplete/autocomplete_browsertest.cc
@@ -120,12 +120,12 @@ IN_PROC_BROWSER_TEST_F(AutocompleteBrowserTest, MAYBE_Autocomplete) {
   AutocompleteController* autocomplete_controller = GetAutocompleteController();
 
   {
+    OmniboxView* location_entry = location_bar->GetLocationEntry();
+    location_entry->model()->SetInputInProgress(true);
     autocomplete_controller->Start(AutocompleteInput(
         ASCIIToUTF16("chrome"), string16::npos, string16(), GURL(), true, false,
         true, AutocompleteInput::SYNCHRONOUS_MATCHES));
 
-    OmniboxView* location_entry = location_bar->GetLocationEntry();
-
     EXPECT_TRUE(autocomplete_controller->done());
     EXPECT_TRUE(location_bar->GetInputString().empty());
     EXPECT_TRUE(location_entry->GetText().empty());
diff --git a/chrome/browser/autocomplete/autocomplete_controller.cc b/chrome/browser/autocomplete/autocomplete_controller.cc
index 81921bf..37eb6dc 100644
--- a/chrome/browser/autocomplete/autocomplete_controller.cc
+++ b/chrome/browser/autocomplete/autocomplete_controller.cc
@@ -310,12 +310,11 @@ void AutocompleteController::Stop(bool clear_result) {
 }
 
 void AutocompleteController::StartZeroSuggest(const GURL& url,
-                                              const string16& user_text,
                                               const string16& permanent_text) {
   if (zero_suggest_provider_ != NULL) {
     DCHECK(!in_start_);  // We should not be already running a query.
     in_zero_suggest_ = true;
-    zero_suggest_provider_->StartZeroSuggest(url, user_text, permanent_text);
+    zero_suggest_provider_->StartZeroSuggest(url, permanent_text);
   }
 }
 
@@ -377,6 +376,7 @@ void AutocompleteController::ResetSession() {
   for (ACProviders::const_iterator i(providers_.begin()); i != providers_.end();
        ++i)
     (*i)->ResetSession();
+  in_zero_suggest_ = false;
 }
 
 void AutocompleteController::UpdateResult(
diff --git a/chrome/browser/autocomplete/autocomplete_controller.h b/chrome/browser/autocomplete/autocomplete_controller.h
index 38a658d..6812c79 100644
--- a/chrome/browser/autocomplete/autocomplete_controller.h
+++ b/chrome/browser/autocomplete/autocomplete_controller.h
@@ -80,11 +80,10 @@ class AutocompleteController : public AutocompleteProviderListener {
   // Begin asynchronously fetching zero-suggest suggestions for |url|.
   // |user_text| is the text entered in the omnibox, which may be non-empty if
   // the user previously focused in the omnibox during this interaction.
-  // |permanent_text| is the text version of |url| displayed in the omnibox.
+  // |permanent_text| is the omnibox text for the current page.
   // TODO(jered): Rip out |user_text| once the first match is decoupled from
   // the current typing in the omnibox.
   void StartZeroSuggest(const GURL& url,
-                        const string16& user_text,
                         const string16& permanent_text);
 
   // Cancels any pending zero-suggest fetch.
diff --git a/chrome/browser/autocomplete/zero_suggest_provider.cc b/chrome/browser/autocomplete/zero_suggest_provider.cc
index 6cc2fd5..a1927ec 100644
--- a/chrome/browser/autocomplete/zero_suggest_provider.cc
+++ b/chrome/browser/autocomplete/zero_suggest_provider.cc
@@ -80,13 +80,6 @@ ZeroSuggestProvider* ZeroSuggestProvider::Create(
 
 void ZeroSuggestProvider::Start(const AutocompleteInput& input,
                                 bool /*minimal_changes*/) {
-  CheckIfTextModfied(input.text());
-  // Clear results only if the user text was modified.
-  Stop(user_text_modified_);
-  ConvertResultsToAutocompleteMatches(input.text(), false);
-  // listener_->OnProviderUpdate() does not need to be called because this
-  // function is only called in the synchronous pass when a user has performed
-  // an action (such as typing a character in the omnobox).
 }
 
 void ZeroSuggestProvider::Stop(bool clear_cached_results) {
@@ -125,6 +118,7 @@ void ZeroSuggestProvider::ResetSession() {
   // |field_trial_triggered_in_session_| unchanged and set
   // |field_trial_triggered_| to false since zero suggest is inactive now.
   field_trial_triggered_ = false;
+  Stop(true);
 }
 
 void ZeroSuggestProvider::OnURLFetchComplete(const net::URLFetcher* source) {
@@ -150,13 +144,12 @@ void ZeroSuggestProvider::OnURLFetchComplete(const net::URLFetcher* source) {
   done_ = true;
 
   if (have_results) {
-    ConvertResultsToAutocompleteMatches(original_user_text_, true);
+    ConvertResultsToAutocompleteMatches();
     listener_->OnProviderUpdate(true);
   }
 }
 
 void ZeroSuggestProvider::StartZeroSuggest(const GURL& url,
-                                           const string16& user_text,
                                            const string16& permanent_text) {
   Stop(true);
   field_trial_triggered_ = false;
@@ -165,11 +158,9 @@ void ZeroSuggestProvider::StartZeroSuggest(const GURL& url,
     return;
   verbatim_relevance_ = kDefaultVerbatimZeroSuggestRelevance;
   done_ = false;
-  original_user_text_ = user_text;
   permanent_text_ = permanent_text;
   current_query_ = url.spec();
   current_url_match_ = MatchForCurrentURL();
-  user_text_modified_ = false;
   // TODO(jered): Consider adding locally-sourced zero-suggestions here too.
   // These may be useful on the NTP or more relevant to the user than server
   // suggestions, if based on local browsing history.
@@ -182,7 +173,6 @@ ZeroSuggestProvider::ZeroSuggestProvider(
     : AutocompleteProvider(listener, profile,
           AutocompleteProvider::TYPE_ZERO_SUGGEST),
       template_url_service_(TemplateURLServiceFactory::GetForProfile(profile)),
-      user_text_modified_(false),
       have_pending_request_(false),
       verbatim_relevance_(kDefaultVerbatimZeroSuggestRelevance),
       field_trial_triggered_(false),
@@ -367,7 +357,12 @@ AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
   AutocompleteMatch::ClassifyLocationInString(string16::npos, 0,
       match.contents.length(), ACMatchClassification::URL,
       &match.contents_class);
-  match.description = navigation.description();
+
+  match.description =
+      AutocompleteMatch::SanitizeString(navigation.description());
+  AutocompleteMatch::ClassifyLocationInString(string16::npos, 0,
+      match.description.length(), ACMatchClassification::NONE,
+      &match.description_class);
   return match;
 }
 
@@ -414,11 +409,6 @@ void ZeroSuggestProvider::Run() {
   LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_SENT);
 }
 
-void ZeroSuggestProvider::CheckIfTextModfied(const string16& user_text) {
-  if (!user_text.empty() && user_text != permanent_text_)
-    user_text_modified_ = true;
-}
-
 void ZeroSuggestProvider::ParseSuggestResults(const Value& root_val) {
   SearchProvider::SuggestResults suggest_results;
   FillResults(root_val, &verbatim_relevance_,
@@ -431,8 +421,7 @@ void ZeroSuggestProvider::ParseSuggestResults(const Value& root_val) {
                          &query_matches_map_);
 }
 
-void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches(
-    string16 user_text, bool update_histograms) {
+void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches() {
   matches_.clear();
 
   const TemplateURL* default_provider =
@@ -444,21 +433,15 @@ void ZeroSuggestProvider::ConvertResultsToAutocompleteMatches(
   const int num_query_results = query_matches_map_.size();
   const int num_nav_results = navigation_results_.size();
   const int num_results = num_query_results + num_nav_results;
-  if (update_histograms) {
-    UMA_HISTOGRAM_COUNTS("ZeroSuggest.QueryResults", num_query_results);
-    UMA_HISTOGRAM_COUNTS("ZeroSuggest.URLResults",  num_nav_results);
-    UMA_HISTOGRAM_COUNTS("ZeroSuggest.AllResults", num_results);
-  }
+  UMA_HISTOGRAM_COUNTS("ZeroSuggest.QueryResults", num_query_results);
+  UMA_HISTOGRAM_COUNTS("ZeroSuggest.URLResults",  num_nav_results);
+  UMA_HISTOGRAM_COUNTS("ZeroSuggest.AllResults", num_results);
 
-  if (num_results == 0 || user_text_modified_)
+  if (num_results == 0)
     return;
 
   // TODO(jered): Rip this out once the first match is decoupled from the
   // current typing in the omnibox.
-  // If the user text is empty, we can autocomplete to the URL.  Otherwise,
-  // don't modify the omnibox text.
-  current_url_match_.inline_autocomplete_offset = user_text.empty() ?
-      0 : string16::npos;
   matches_.push_back(current_url_match_);
 
   for (SearchProvider::MatchMap::const_iterator it(query_matches_map_.begin());
@@ -479,6 +462,7 @@ AutocompleteMatch ZeroSuggestProvider::MatchForCurrentURL() {
       HistoryURLProvider::SuggestExactInput(this, input,
                                             !HasHTTPScheme(input.text())));
   match.is_history_what_you_typed_match = false;
+  match.inline_autocomplete_offset = string16::npos;
 
   // The placeholder suggestion for the current URL has high relevance so
   // that it is in the first suggestion slot and inline autocompleted. It
diff --git a/chrome/browser/autocomplete/zero_suggest_provider.h b/chrome/browser/autocomplete/zero_suggest_provider.h
index 155f0af..8b372e7 100644
--- a/chrome/browser/autocomplete/zero_suggest_provider.h
+++ b/chrome/browser/autocomplete/zero_suggest_provider.h
@@ -70,14 +70,13 @@ class ZeroSuggestProvider : public AutocompleteProvider,
   // net::URLFetcherDelegate
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
-  // Initiates a new fetch for the given |url|, limiting suggestions to those
-  // matching |user_text|. |user_text| may be non-empty if the user previously
-  // interacted with zero-suggest suggestions and then unfocused the omnibox.
-  // |permanent_text| is the text version of |url| displayed in the omnibox.
+  // Initiates a new fetch for the given |url|. |user_text| is the user input
+  // and may be non-empty if the user previously interacted with
+  // zero-suggest suggestions and then unfocused the omnibox. |permanent_text|
+  // is the omnibox text for the current page.
   // TODO(jered): Rip out |user_text| once the first match is decoupled from
   // the current typing in the omnibox.
   void StartZeroSuggest(const GURL& url,
-                        const string16& user_text,
                         const string16& permanent_text);
 
  private:
@@ -124,21 +123,16 @@ class ZeroSuggestProvider : public AutocompleteProvider,
   AutocompleteMatch NavigationToMatch(
       const SearchProvider::NavigationResult& navigation);
 
-  // Sets |user_text_modified_| if the user has modified the omnibox text, based
-  // on the user input |user_text|.
-  void CheckIfTextModfied(const string16& user_text);
-
   // Fetches zero-suggest suggestions for |current_query_|.
   void Run();
 
   // Parses results from the zero-suggest server and updates results.
   void ParseSuggestResults(const base::Value& root_val);
 
-  // Converts the parsed results to a set of AutocompleteMatches, based on user
-  // input |user_text|, and adds them to |matches_|.  If |update_histograms| is
-  // true, also update the histograms for how many results were received.
-  void ConvertResultsToAutocompleteMatches(string16 user_text,
-                                           bool update_histograms);
+  // Converts the parsed results to a set of AutocompleteMatches and adds them
+  // to |matches_|.  Also update the histograms for how many results were
+  // received.
+  void ConvertResultsToAutocompleteMatches();
 
   // Returns an AutocompleteMatch for the current URL. The match should be in
   // the top position so that pressing enter has the effect of reloading the
@@ -151,12 +145,8 @@ class ZeroSuggestProvider : public AutocompleteProvider,
   // The URL for which a suggestion fetch is pending.
   std::string current_query_;
 
-  // What the user has typed.
-  string16 original_user_text_;
   // Copy of OmniboxEditModel::permanent_text_.
   string16 permanent_text_;
-  // Whether the user has modified the omnibox since the zero suggest request.
-  bool user_text_modified_;
 
   // Fetcher used to retrieve results.
   scoped_ptr<net::URLFetcher> fetcher_;
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 2c9e8ce..131bcd8 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -213,7 +213,8 @@ bool OmniboxEditModel::UpdatePermanentText(const string16& new_permanent_text) {
   string16 instant_suggestion = view_->GetInstantSuggestion();
   const bool visibly_changed_permanent_text =
       (permanent_text_ != new_permanent_text) &&
-      (!user_input_in_progress_ || !has_focus()) &&
+      (!has_focus() ||
+       (!user_input_in_progress_ && !popup_model()->IsOpen())) &&
       (instant_suggestion.empty() ||
        new_permanent_text != user_text_ + instant_suggestion);
 
@@ -818,7 +819,6 @@ void OmniboxEditModel::OnSetFocus(bool control_down) {
     // the actual underlying current URL, e.g. if we're on the NTP and the
     // |permanent_text_| is empty.
     autocomplete_controller()->StartZeroSuggest(delegate_->GetURL(),
-                                                user_text_,
                                                 permanent_text_);
   }
 
@@ -893,26 +893,8 @@ bool OmniboxEditModel::OnEscapeKeyPressed() {
 }
 
 void OmniboxEditModel::OnControlKeyChanged(bool pressed) {
-  // Don't change anything unless the key state is actually toggling.
-  if (pressed == (control_key_state_ == UP)) {
-    ControlKeyState old_state = control_key_state_;
-    control_key_state_ = pressed ? DOWN_WITHOUT_CHANGE : UP;
-    if ((control_key_state_ == DOWN_WITHOUT_CHANGE) && has_temporary_text_) {
-      // Arrowing down and then hitting control accepts the temporary text as
-      // the input text.
-      InternalSetUserText(UserTextFromDisplayText(view_->GetText()));
-      has_temporary_text_ = false;
-      is_temporary_text_set_by_instant_ = false;
-      selected_instant_autocomplete_match_index_ = OmniboxPopupModel::kNoMatch;
-      is_instant_temporary_text_a_search_query_ = false;
-    }
-    if ((old_state != DOWN_WITH_CHANGE) && popup_model()->IsOpen()) {
-      // Autocomplete history provider results may change, so refresh the
-      // popup.  This will force user_input_in_progress_ to true, but if the
-      // popup is open, that should have already been the case.
-      view_->UpdatePopup();
-    }
-  }
+  // TODO(beaudoin): Remove this function entirely and all the code that calls
+  // it.
 }
 
 void OmniboxEditModel::OnUpOrDownKeyPressed(int count) {
@@ -1002,6 +984,7 @@ void OmniboxEditModel::OnPopupDataChanged(
   bool call_controller_onchanged = true;
   inline_autocomplete_text_ = text;
 
+  string16 user_text = user_input_in_progress_ ? user_text_ : permanent_text_;
   if (keyword_state_changed && KeywordIsSelected()) {
     // If we reach here, the user most likely entered keyword mode by inserting
     // a space between a keyword name and a search string (as pressing space or
@@ -1018,11 +1001,11 @@ void OmniboxEditModel::OnPopupDataChanged(
     // temporary text back to a default match that's a keyword search, but in
     // that case the RevertTemporaryText() call below will reset the caret or
     // selection correctly so the caret positioning we do here won't matter.
-    view_->SetWindowTextAndCaretPos(DisplayTextFromUserText(user_text_), 0,
+    view_->SetWindowTextAndCaretPos(DisplayTextFromUserText(user_text), 0,
                                                             false, false);
   } else if (view_->OnInlineAutocompleteTextMaybeChanged(
-             DisplayTextFromUserText(user_text_ + inline_autocomplete_text_),
-             DisplayTextFromUserText(user_text_).length())) {
+             DisplayTextFromUserText(user_text + inline_autocomplete_text_),
+             DisplayTextFromUserText(user_text).length())) {
     call_controller_onchanged = false;
   }