[omnibox] Plumb FocusLocation is_user_initiated param to OmniboxView

This variable has always existed as |select_all|, but it's real meaning
is essentially |is_user_initiated|. This CL renames the parameter to
that new name, and plumbs it into OmniboxView.

It doesn't use that parameter yet. This CL has no behavior changes,
just an API change.

The followup CL will use this new parameter to prevent triggering
on-focus suggestions for renderer-initiated omnibox focuses.
Most notably, this prevents browser startup NTP load from triggering
on-focus suggestions.

TBR=aboxhall@chromium.org

Bug: 1013287, 996516
Change-Id: I9461ea6a9c925b163aed5708109f5f7caa00f0c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865616
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Dan Beam <dbeam@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707023}

chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc

@@ -159,7 +159,7 @@ IN_PROC_BROWSER_TEST_F(StickyKeysBrowserTest, SearchLeftOmnibox) {
       browser()->window()->GetLocationBar()->GetOmniboxView();
 
   // Give the omnibox focus.
-  omnibox->SetFocus();
+  omnibox->SetFocus(/*is_user_initiated=*/true);
 
   // Make sure that the AppList is not erroneously displayed and the omnibox
   // doesn't lose focus.

chrome/browser/ui/location_bar/location_bar.h

@@ -41,18 +41,19 @@ class LocationBar {
   // latency of page loads starting at user input.
   virtual void AcceptInput(base::TimeTicks match_selection_timestamp) = 0;
 
-  // Focuses the location bar.  Optionally also selects its contents.
+  // Focuses the location bar. User-initiated focuses (like pressing Ctrl+L)
+  // should have |is_user_initiated| set to true. In those cases, we want to
+  // take some extra steps, like selecting everything and maybe uneliding.
   //
-  // User-initiated focuses should have |select_all| set to true, as users
-  // are accustomed to being able to use Ctrl+L to select-all in the omnibox.
+  // Renderer-initiated focuses (like browser startup or NTP finished loading),
+  // should have |is_user_initiated| set to false, so we can avoid disrupting
+  // user actions and avoid requesting on-focus suggestions.
   //
-  // Renderer-initiated focuses should have |select_all| set to false, as the
-  // user may be in the middle of typing while the tab finishes loading.
-  // In that case, we don't want to select-all and cause the user to clobber
-  // their already-typed text.
-  virtual void FocusLocation(bool select_all) = 0;
+  // TODO(tommycli): See if there's a more descriptive name for this method.
+  virtual void FocusLocation(bool is_user_initiated) = 0;
 
   // Puts the user into keyword mode with their default search provider.
+  // TODO(tommycli): See if there's a more descriptive name for this method.
   virtual void FocusSearch() = 0;
 
   // Updates the state of the images showing the content settings status.

chrome/browser/ui/search/search_tab_helper.cc

@@ -292,7 +292,10 @@ void SearchTabHelper::FocusOmnibox(bool focus) {
     return;
 
   if (focus) {
-    omnibox_view->SetFocus();
+    // This is an invisible focus to support "realbox" implementations on NTPs
+    // (including other search providers). We shouldn't consider it as the user
+    // explicitly focusing the omnibox.
+    omnibox_view->SetFocus(/*is_user_initiated=*/false);
     omnibox_view->model()->SetCaretVisibility(false);
     // If the user clicked on the fakebox, any text already in the omnibox
     // should get cleared when they start typing. Selecting all the existing
@@ -421,8 +424,11 @@ void SearchTabHelper::PasteIntoOmnibox(const base::string16& text) {
   if (text_to_paste.empty())
     return;
 
-  if (!omnibox_view->model()->has_focus())
-    omnibox_view->SetFocus();
+  if (!omnibox_view->model()->has_focus()) {
+    // Pasting into a "realbox" should not be considered the user explicitly
+    // focusing the omnibox.
+    omnibox_view->SetFocus(/*is_user_initiated=*/false);
+  }
 
   omnibox_view->OnBeforePossibleChange();
   omnibox_view->model()->OnPaste();

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -368,15 +368,17 @@ void LocationBarView::SelectAll() {
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, public LocationBar implementation:
 
-void LocationBarView::FocusLocation(bool select_all) {
+void LocationBarView::FocusLocation(bool is_user_initiated) {
   const bool omnibox_already_focused = omnibox_view_->HasFocus();
 
-  omnibox_view_->SetFocus();
+  omnibox_view_->SetFocus(is_user_initiated);
 
   if (omnibox_already_focused)
     omnibox_view()->model()->ClearKeyword();
 
-  if (!select_all)
+  // TODO(tommycli): Since we are now passing the |is_user_initiated| parameter
+  // onto OmniboxView, we can likely move the below code into SetFocus().
+  if (!is_user_initiated)
     return;
 
   omnibox_view_->SelectAll(true);
@@ -940,7 +942,8 @@ void LocationBarView::AcceptInput(base::TimeTicks match_selection_timestamp) {
 }
 
 void LocationBarView::FocusSearch() {
-  omnibox_view_->SetFocus();
+  // This is called by keyboard accelerator, so it's user-initiated.
+  omnibox_view_->SetFocus(/*is_user_initiated=*/true);
   omnibox_view_->EnterKeywordModeForDefaultSearchProvider();
 }
 
@@ -1012,7 +1015,9 @@ void LocationBarView::OnVisibleBoundsChanged() {
 }
 
 void LocationBarView::OnFocus() {
-  omnibox_view_->SetFocus();
+  // This is only called when the user explicitly focuses the location bar.
+  // Renderer-initated focuses go through the FocusLocation() call instead.
+  omnibox_view_->SetFocus(/*is_user_initiated=*/true);
 }
 
 void LocationBarView::OnPaintBorder(gfx::Canvas* canvas) {

chrome/browser/ui/views/location_bar/location_bar_view.h

@@ -167,7 +167,7 @@ class LocationBarView : public LocationBar,
   bool ActivateFirstInactiveBubbleForAccessibility();
 
   // LocationBar:
-  void FocusLocation(bool select_all) override;
+  void FocusLocation(bool is_user_initiated) override;
   void Revert() override;
   OmniboxView* GetOmniboxView() override;
 

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

@@ -418,7 +418,7 @@ void OmniboxViewViews::RevertAll() {
   OmniboxView::RevertAll();
 }
 
-void OmniboxViewViews::SetFocus() {
+void OmniboxViewViews::SetFocus(bool is_user_initiated) {
   // Temporarily reveal the top-of-window views (if not already revealed) so
   // that the location bar view is visible and is considered focusable. When it
   // actually receives focus, ImmersiveFocusWatcher will add another lock to

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

@@ -127,7 +127,7 @@ class OmniboxViewViews : public OmniboxView,
                           base::string16::size_type* end) const override;
   void SelectAll(bool reversed) override;
   void RevertAll() override;
-  void SetFocus() override;
+  void SetFocus(bool is_user_initiated) override;
   bool IsImeComposing() const override;
   gfx::NativeView GetRelativeWindowForPopup() const override;
   bool IsImeShowingPopup() const override;

chrome/test/base/test_browser_window.h

@@ -103,7 +103,7 @@ class TestBrowserWindow : public BrowserWindow {
   bool UpdatePageActionIcon(PageActionIconType type) override;
   autofill::AutofillBubbleHandler* GetAutofillBubbleHandler() override;
   void ExecutePageActionIconForTesting(PageActionIconType type) override {}
-  void SetFocusToLocationBar(bool select_all) override {}
+  void SetFocusToLocationBar(bool is_user_initiated) override {}
   void UpdateReloadStopState(bool is_loading, bool force) override {}
   void UpdateToolbar(content::WebContents* contents) override {}
   void UpdateCustomTabBarVisibility(bool visible, bool animate) override {}

components/omnibox/browser/omnibox_view.h

@@ -143,8 +143,10 @@ class OmniboxView {
   // defines a method with that name.
   virtual void CloseOmniboxPopup();
 
-  // Sets the focus to the omnibox.
-  virtual void SetFocus() = 0;
+  // Sets the focus to the omnibox. |is_user_initiated| is true when the user
+  // explicitly focused the omnibox, and false when the omnibox was
+  // automatically focused (like for browser startup or NTP load).
+  virtual void SetFocus(bool is_user_initiated) = 0;
 
   // Shows or hides the caret based on whether the model's is_caret_visible() is
   // true.

components/omnibox/browser/test_omnibox_view.h

@@ -48,7 +48,7 @@ class TestOmniboxView : public OmniboxView {
   void SelectAll(bool reversed) override;
   void RevertAll() override {}
   void UpdatePopup() override {}
-  void SetFocus() override {}
+  void SetFocus(bool is_user_initiated) override {}
   void ApplyCaretVisibility() override {}
   void OnTemporaryTextMaybeChanged(const base::string16& display_text,
                                    const AutocompleteMatch& match,

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

@@ -84,7 +84,7 @@ class OmniboxViewIOS : public OmniboxView,
   void GetSelectionBounds(base::string16::size_type* start,
                           base::string16::size_type* end) const override;
   void SelectAll(bool reversed) override {}
-  void SetFocus() override {}
+  void SetFocus(bool is_user_initiated) override {}
   void ApplyCaretVisibility() override {}
   void OnInlineAutocompleteTextCleared() override {}
   void OnRevertTemporaryText(const base::string16& display_text,