[OriginChip] Show the URL on Esc key pressed

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 <string>
 
 #include "base/auto_reset.h"
 #include "base/format_macros.h"
@@ skipping 483 matching lines @@
   }
 
   if (user_input_in_progress_ == in_progress)
     return;
 
   user_input_in_progress_ = in_progress;
   if (user_input_in_progress_) {
     time_user_first_modified_omnibox_ = base::TimeTicks::Now();
     content::RecordAction(base::UserMetricsAction("OmniboxInputInProgress"));
     autocomplete_controller()->ResetSession();
-    // Once the user starts editing, re-enable URL replacement, so that it will
-    // kick in if applicable once the edit is committed or reverted. (While the
-    // edit is in progress, this won't have a visible effect.)
-    controller_->GetToolbarModel()->set_url_replacement_enabled(true);

[Justin Donnelly, 2014/03/27 13:47:36]

I'm not sure what this was trying to accomplish.  I think maybe to make it so
that after editing a URL the query terms would come back on Esc?

If so, the new behavior seems more consistent to me (edit a URL and Esc brings
back the URL (because url_replacement_enabled is still false) edit query terms
and Esc brings back the query terms.  And this call was making the origin chip
reappear on Esc in some cases (because WouldOmitURLDueToOriginChip depends on
the state of url_replacement_enabled).

   }
 
-  // The following code handles three cases:
+  // The following code handles two cases:
   // * For HIDE_ON_USER_INPUT, it hides the chip when user input begins.
   // * For HIDE_ON_MOUSE_RELEASE, which only hides the chip on mouse release if
   //   the omnibox is empty, it handles the "omnibox was not empty" case by
-  //   acting like HIDE_ON_USER_INPUT.  (If the omnibox was empty, it
-  //   effectively no-ops.)
-  // * For both hide behaviors, it allows the chip to be reshown once input
-  //   ends.  (The chip won't actually be re-shown until there's no pending
-  //   typed navigation; see OriginChipView::ShouldShow() and
-  //   OriginChipDecoration::ShouldShow().)
-  if (chrome::ShouldDisplayOriginChipV2())
-    controller()->GetToolbarModel()->set_origin_chip_enabled(!in_progress);

[Justin Donnelly, 2014/03/27 13:47:36]

I don't think there was ever any good reason why I did this as opposed to the
"if in_progress then set_enabled(false)" approach.  The latter (as implemented
now) works fine because focus loss always restores the chip.

+  //   acting like HIDE_ON_USER_INPUT.
+  if (chrome::ShouldDisplayOriginChipV2() && in_progress)
+    controller()->GetToolbarModel()->set_origin_chip_enabled(false);
 
   controller_->GetToolbarModel()->set_input_in_progress(in_progress);
   controller_->Update(NULL);
 
   if (user_input_in_progress_ || !in_revert_)
     delegate_->OnInputStateChanged();
 }
 
 void OmniboxEditModel::Revert() {
   SetInputInProgress(false);
@@ skipping 447 matching lines @@
       (match.provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST);
   if (!has_zero_suggest_match && !user_input_in_progress_ &&
       view_->IsSelectAll())
     return false;
 
   if (!user_text_.empty()) {
     UMA_HISTOGRAM_ENUMERATION(kOmniboxUserTextClearedHistogram,
                               OMNIBOX_USER_TEXT_CLEARED_WITH_ESCAPE,
                               OMNIBOX_USER_TEXT_CLEARED_NUM_OF_ITEMS);
   }
-  view_->RevertAll();

[Justin Donnelly, 2014/03/27 13:47:36]

As with my first note above, resetting search term replacement makes the origin
chip reappear which we don't want.

+  view_->RevertWithoutResettingSearchTermReplacement();
   view_->SelectAll(true);
   return true;
 }
 
 void OmniboxEditModel::OnControlKeyChanged(bool pressed) {
   if (pressed == (control_key_state_ == UP))
     control_key_state_ = pressed ? DOWN_WITHOUT_CHANGE : UP;
 }
 
 void OmniboxEditModel::OnPaste() {
@@ skipping 436 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);
 }