[OriginChip] Show the URL on Esc key pressed

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 307ba14ddf24350c417df96f8594a44b12f1bd9f..c5f60c40b3c9fda9bad1d9da391a1ab660201cdc 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -501,24 +501,15 @@ void OmniboxEditModel::SetInputInProgress(bool 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);
   }
 
-  // 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);
+  //   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);
@@ -970,6 +961,19 @@ bool OmniboxEditModel::OnEscapeKeyPressed() {
     view_->Update();
   }
 
+  // If URL replacement is enabled, disable it, update and revert in order to
+  // bring the URL back.  Unless search term replacement is being performed, in
+  // which case we want to keep the search terms.
+  if (controller_->GetToolbarModel()->url_replacement_enabled() &&
+      !controller_->GetToolbarModel()->WouldPerformSearchTermReplacement(
+          true)) {
+    controller_->GetToolbarModel()->set_url_replacement_enabled(false);
+    UpdatePermanentText();

[Peter Kasting, 2014/03/27 21:10:09]

How come we need to call this?

[Justin Donnelly, 2014/03/27 21:52:17]

UpdatePermanentText is what calls ToolbarModelImpl::GetText() to determine what
the text should be given the new state.  This is the same as what OmniboxView
does to show and hide the URL.

+    view_->RevertWithoutResettingSearchTermReplacement();
+    view_->SelectAll(true);
+    return true;

[Peter Kasting, 2014/03/27 21:10:09]

I think this skips the histogram below when we don't want to.

Maybe instead we need to do something like:

  bool should_disable_url_replacement =
      controller_->GetToolbarModel()->url_replacement_enabled() &&
      !controller_->GetToolbarModel()->WouldPerformSearchTermReplacement(true);
  ...
  if (!has_zero_suggest_match && !should_disable_url_replacement &&
      !user_input_in_progress_ && view_->IsSelectAll())
    return false;
  ...
  if (should_disable_url_replacement)
    controller_->GetToolbarModel()->set_url_replacement_enabled(false);
  view_->RevertWithoutResettingSearchTermReplacement();
  ...

[Justin Donnelly, 2014/03/27 21:52:17]

Done.

+  }
+
   // If the user wasn't editing, but merely had focus in the edit, allow <esc>
   // to be processed as an accelerator, so it can still be used to stop a load.
   // When the permanent text isn't all selected we still fall through to the
@@ -986,7 +990,7 @@ bool OmniboxEditModel::OnEscapeKeyPressed() {
                               OMNIBOX_USER_TEXT_CLEARED_WITH_ESCAPE,
                               OMNIBOX_USER_TEXT_CLEARED_NUM_OF_ITEMS);
   }
-  view_->RevertAll();
+  view_->RevertWithoutResettingSearchTermReplacement();
   view_->SelectAll(true);
   return true;
 }