[omnibox] Trigger Desktop ZeroSuggest only on explicit gestures

This CL takes out ZeroSuggest triggering on-focus for Desktop, as well
as some blacklisting logic.

Instead, we now trigger ZeroSuggest specifically on a select whitelist
of gestures.

This enables ZeroSuggest to work on repeated Ctrl+L presses now.

It also likely turns off ZeroSuggest for some focus events that are
triggered via Accessibility features. That's probably a good thing
though, as those users should get as little "noise" as possible.

TBR=thestig@chromium.org

Bug: 1026948
Change-Id: Iabbaca73a5ef93ae5676a363c1e44ece61cc74d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935208
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722692}

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

@@ -434,9 +434,11 @@ void OmniboxViewViews::SetFocus(bool is_user_initiated) {
             ->GetRevealedLock(ImmersiveModeController::ANIMATE_REVEAL_YES));
   }
 
-  suppress_on_focus_suggestions_ = !is_user_initiated;
   RequestFocus();
-  suppress_on_focus_suggestions_ = false;
+
+  // |is_user_initiated| is true for focus events from keyboard accelerators.
+  if (is_user_initiated)
+    model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
 
   // Restore caret visibility if focus is explicitly requested. This is
   // necessary because if we already have invisible focus, the RequestFocus()
@@ -1130,6 +1132,12 @@ bool OmniboxViewViews::OnMousePressed(const ui::MouseEvent& event) {
     saved_selection_for_focus_change_ = gfx::Range::InvalidRange();
   }
 
+  // Show on-focus suggestions if either:
+  //  - The textfield doesn't already have focus.
+  //  - Or if the textfield is empty, to cover the NTP ZeroSuggest case.
+  if (event.IsOnlyLeftMouseButton() && (!HasFocus() || GetText().empty()))
+    model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
+
   bool handled = views::Textfield::OnMousePressed(event);
 
   // This ensures that when the user makes a double-click partial select, we
@@ -1141,13 +1149,6 @@ bool OmniboxViewViews::OnMousePressed(const ui::MouseEvent& event) {
     filter_drag_events_for_unelision_ = true;
   }
 
-  // This is intended to cover the NTP case where the omnibox starts focused.
-  // The user can explicitly request on-focus suggestions by clicking or tapping
-  // the omnibox. Restricted to empty textfield to avoid disrupting selections.
-  if (HasFocus() && GetText().empty() && event.IsOnlyLeftMouseButton()) {
-    model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
-  }
-
   return handled;
 }
 
@@ -1198,6 +1199,12 @@ void OmniboxViewViews::OnGestureEvent(ui::GestureEvent* event) {
     saved_selection_for_focus_change_ = gfx::Range::InvalidRange();
   }
 
+  // Show on-focus suggestions if either:
+  //  - The textfield doesn't already have focus.
+  //  - Or if the textfield is empty, to cover the NTP ZeroSuggest case.
+  if (!HasFocus() || GetText().empty())
+    model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
+
   views::Textfield::OnGestureEvent(event);
 
   if (select_all_on_gesture_tap_ && event->type() == ui::ET_GESTURE_TAP) {
@@ -1213,13 +1220,6 @@ void OmniboxViewViews::OnGestureEvent(ui::GestureEvent* event) {
       event->type() == ui::ET_GESTURE_LONG_TAP) {
     select_all_on_gesture_tap_ = false;
   }
-
-  // This is intended to cover the NTP case where the omnibox starts focused.
-  // The user can explicitly request on-focus suggestions by clicking or tapping
-  // the omnibox. Restricted to empty textfield to avoid disrupting selections.
-  if (HasFocus() && GetText().empty() && event->type() == ui::ET_GESTURE_TAP) {
-    model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
-  }
 }
 
 void OmniboxViewViews::AboutToRequestFocusFromTabTraversal(bool reverse) {
@@ -1342,14 +1342,8 @@ void OmniboxViewViews::OnFocus() {
   // Investigate why it's needed and see if we can remove it.
   model()->ResetDisplayTexts();
 
-  bool suppress = suppress_on_focus_suggestions_;
-  if (GetFocusManager() &&
-      GetFocusManager()->focus_change_reason() !=
-          views::FocusManager::FocusChangeReason::kDirectFocusChange) {
-    suppress = true;
-  }
   // TODO(oshima): Get control key state.
-  model()->OnSetFocus(false, suppress);
+  model()->OnSetFocus(false);
   // Don't call controller()->OnSetFocus, this view has already acquired focus.
 
   // Restore the selection we saved in OnBlur() if it's still valid.

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

@@ -350,10 +350,6 @@ class OmniboxViewViews : public OmniboxView,
   // and gets a tap. So we use this variable to remember focus state before tap.
   bool select_all_on_gesture_tap_ = false;
 
-  // True if we should suppress on-focus suggestions, because we are currently
-  // processing a focus ovent that we know the user didn't explicitly initiate.
-  bool suppress_on_focus_suggestions_ = false;
-
   // The time of the first character insert operation that has not yet been
   // painted. Used to measure omnibox responsiveness with a histogram.
   base::TimeTicks insert_char_time_;

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

@@ -615,6 +615,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, AccessiblePopup) {
   ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &omnibox_view));
   OmniboxViewViews* omnibox_view_views =
       static_cast<OmniboxViewViews*>(omnibox_view);
+  chrome::FocusLocationBar(browser());
 
   base::string16 match_url = base::ASCIIToUTF16("https://google.com");
   AutocompleteMatch match(nullptr, 500, false,
@@ -654,7 +655,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, AccessiblePopup) {
       input, TemplateURLServiceFactory::GetForProfile(browser()->profile()));
 
   // The omnibox popup should open with suggestions displayed.
-  chrome::FocusLocationBar(browser());
   omnibox_view->model()->popup_model()->OnResultChanged();
   EXPECT_TRUE(omnibox_view->model()->popup_model()->IsOpen());
   ui::AXNodeData popup_node_data_2;
@@ -683,6 +683,7 @@ class OmniboxViewViewsUIATest : public OmniboxViewViewsTest {
 IN_PROC_BROWSER_TEST_F(OmniboxViewViewsUIATest, AccessibleOmnibox) {
   OmniboxView* omnibox_view = nullptr;
   ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &omnibox_view));
+  chrome::FocusLocationBar(browser());
 
   base::string16 match_url = base::ASCIIToUTF16("https://example.com");
   AutocompleteMatch match(nullptr, 500, false,
@@ -718,7 +719,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsUIATest, AccessibleOmnibox) {
       input, TemplateURLServiceFactory::GetForProfile(browser()->profile()));
 
   // The omnibox popup should open with suggestions displayed.
-  chrome::FocusLocationBar(browser());
   omnibox_view->model()->popup_model()->OnResultChanged();
 
   // Wait for ControllerFor property changed event.

chrome/test/base/ui_test_utils.cc

@@ -480,8 +480,7 @@ void SendToOmniboxAndSubmit(Browser* browser,
                             base::TimeTicks match_selection_timestamp) {
   LocationBar* location_bar = browser->window()->GetLocationBar();
   OmniboxView* omnibox = location_bar->GetOmniboxView();
-  omnibox->model()->OnSetFocus(/*control_down=*/false,
-                               /*suppress_on_focus_suggestions=*/false);
+  omnibox->model()->OnSetFocus(/*control_down=*/false);
   omnibox->SetUserText(base::ASCIIToUTF16(input));
   location_bar->AcceptInput(match_selection_timestamp);
 

components/omnibox/browser/omnibox_edit_model.cc

@@ -1054,8 +1054,7 @@ void OmniboxEditModel::ClearKeyword() {
   }
 }
 
-void OmniboxEditModel::OnSetFocus(bool control_down,
-                                  bool suppress_on_focus_suggestions) {
+void OmniboxEditModel::OnSetFocus(bool control_down) {
   last_omnibox_focus_ = base::TimeTicks::Now();
   user_input_since_focus_ = false;
 
@@ -1069,9 +1068,6 @@ void OmniboxEditModel::OnSetFocus(bool control_down,
   // example, if the user presses ctrl-l to focus the omnibox.
   control_key_state_ = control_down ? DOWN_AND_CONSUMED : UP;
 
-  if (!suppress_on_focus_suggestions)
-    ShowOnFocusSuggestionsIfAutocompleteIdle();
-
   if (user_input_in_progress_ || !in_revert_)
     client_->OnInputStateChanged();
 }

components/omnibox/browser/omnibox_edit_model.h

@@ -291,10 +291,7 @@ class OmniboxEditModel {
 
   // Called when the view is gaining focus.  |control_down| is whether the
   // control key is down (at the time we're gaining focus).
-  // |suppress_on_focus_suggestions| is set to true when on-focus suggestions
-  // should not appear for this focus event. For instance, for
-  // renderer-initiated focus events, it should be set to true.
-  void OnSetFocus(bool control_down, bool suppress_on_focus_suggestions);
+  void OnSetFocus(bool control_down);
 
   // Shows On-Focus Suggestions (ZeroSuggest) if no query is currently running
   // and the popup is closed. This can be called multiple times without harm,

components/omnibox/browser/omnibox_edit_model_unittest.cc

@@ -323,7 +323,7 @@ TEST_F(OmniboxEditModelTest, AlternateNavHasHTTP) {
       AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED);
   const GURL alternate_nav_url("http://abcd/");
 
-  model()->OnSetFocus(false, false);  // Avoids DCHECK in OpenMatch().
+  model()->OnSetFocus(false);  // Avoids DCHECK in OpenMatch().
   model()->SetUserText(base::ASCIIToUTF16("http://abcd"));
   model()->OpenMatch(match, WindowOpenDisposition::CURRENT_TAB,
                      alternate_nav_url, base::string16(), 0);
@@ -457,7 +457,7 @@ TEST_F(OmniboxEditModelTest, IgnoreInvalidSavedFocusStates) {
   ASSERT_EQ(OMNIBOX_FOCUS_NONE, state.focus_state);
 
   // Simulate the tab-switching system focusing the Omnibox.
-  model()->OnSetFocus(false, true);
+  model()->OnSetFocus(false);
 
   // Restoring the old saved state should not clobber the model's focus state.
   model()->RestoreState(&state);
@@ -483,9 +483,9 @@ TEST_F(OmniboxEditModelTest, ConsumeCtrlKey) {
 // Tests ctrl_key_state_ is set consumed if the ctrl key is down on focus.
 TEST_F(OmniboxEditModelTest, ConsumeCtrlKeyOnRequestFocus) {
   model()->control_key_state_ = TestOmniboxEditModel::DOWN;
-  model()->OnSetFocus(false, false);
+  model()->OnSetFocus(false);
   EXPECT_EQ(model()->control_key_state_, TestOmniboxEditModel::UP);
-  model()->OnSetFocus(true, false);
+  model()->OnSetFocus(true);
   EXPECT_EQ(model()->control_key_state_,
             TestOmniboxEditModel::DOWN_AND_CONSUMED);
 }

ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm

@@ -288,8 +288,8 @@ void OmniboxViewIOS::OnDidBeginEditing() {
       model()->set_focus_source(OmniboxFocusSource::OMNIBOX);
     }
 
-    model()->OnSetFocus(/*control_down=*/false,
-                        /*suppress_on_focus_suggestions=*/false);
+    model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
+    model()->OnSetFocus(/*control_down=*/false);
   }
 
   // If the omnibox is displaying a URL and the popup is not showing, set the