[omnibox] Narrow condition for resetting selection

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

Index: chrome/browser/ui/views/omnibox/omnibox_view_views.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
index 6fc3b895ef0ba5c89f7e333c373d373a25bd902d..ffce7e28c7292c6271a8f3b518af1739b93ec6bf 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -772,12 +772,13 @@ void OmniboxViewViews::OnBlur() {
   views::Textfield::OnBlur();
   model()->OnWillKillFocus();
 
-  // If ZeroSuggest is active, we may have refused to show an update to the
-  // underlying permanent URL that happened while the popup was open, so
-  // revert to ensure that update is shown now.  Otherwise, make sure to call
-  // CloseOmniboxPopup() unconditionally, so that if ZeroSuggest is in the midst
-  // of running but hasn't yet opened the popup, it will be halted.
-  if (!model()->user_input_in_progress() && model()->popup_model()->IsOpen())
+  // If ZeroSuggest is active, and there is evidence that there is a text
+  // update to show, revert to ensure that update is shown now.  Otherwise,
+  // make sure to call CloseOmniboxPopup() unconditionally, so that if
+  // ZeroSuggest is in the midst of running but hasn't yet opened the popup,
+  // it will be halted.

[Peter Kasting, 2017/04/10 23:04:13]

Nit: Maybe something in this comment should talk about why we don't just call
RevertAll() in all cases.

[Kevin Bailey, 2017/04/11 14:12:41]

It seems to me that number the cases of calling RevertAll() has reduced. What
did you have in mind?

[Peter Kasting, 2017/04/11 20:04:39]

Right.  My point is that, to a reader, it's not immediately obvious why there's
a conditional here.  The comment currently says _what_ the conditional is
checking and the effect of the revert is, but not why it would have been unsafe
to simply write:

  RevertAll();

...with no conditional check.  So I'd describe the negative side effects that
would occur if we were to call RevertAll() outside of the cases where we
currently do.

[Kevin Bailey, 2017/04/12 14:57:03]

There are a dozen things that revert does additionally, so I hope I covered
the one you were thinking of.

+  if (!model()->user_input_in_progress() && model()->popup_model()->IsOpen() &&
+      text() != model()->PermanentText())
     RevertAll();
   else
     CloseOmniboxPopup();