Cleanup: Remove SearchTabHelperDelegate

Its one remaining member, GetOmniboxView, can be replaced easily by
telling the SearchTabHelper when its tab gets attached to a new window
(i.e. Browser::TabInsertedAt), at which point it can grab the
OmniboxView* itself. This also correctly handles the case where a tab
is dragged between windows.

BUG=627747

Review-Url: https://codereview.chromium.org/2739943005
Cr-Commit-Position: refs/heads/master@{#456046}

chrome/browser/ui/BUILD.gn

@@ -149,8 +149,6 @@ split_static_library("ui") {
     "search/search_model_observer.h",
     "search/search_tab_helper.cc",
     "search/search_tab_helper.h",
-    "search/search_tab_helper_delegate.cc",
-    "search/search_tab_helper_delegate.h",
     "search_engines/edit_search_engine_controller.cc",
     "search_engines/edit_search_engine_controller.h",
     "search_engines/keyword_editor_controller.cc",

chrome/browser/ui/browser.cc

@@ -962,9 +962,9 @@ void Browser::TabInsertedAt(TabStripModel* tab_strip_model,
                             bool foreground) {
   SetAsDelegate(contents, true);
 
-  SessionTabHelper* session_tab_helper =
-      SessionTabHelper::FromWebContents(contents);
-  session_tab_helper->SetWindowID(session_id());
+  SessionTabHelper::FromWebContents(contents)->SetWindowID(session_id());
+
+  SearchTabHelper::FromWebContents(contents)->OnTabAttachedToWindow(window_);
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_TAB_PARENTED,
@@ -1982,13 +1982,6 @@ bool Browser::CanSaveContents(content::WebContents* web_contents) const {
   return chrome::CanSavePage(this);
 }
 
-///////////////////////////////////////////////////////////////////////////////
-// Browser, SearchTabHelperDelegate implementation:
-
-OmniboxView* Browser::GetOmniboxView() {
-  return window_->GetLocationBar()->GetOmniboxView();
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, web_modal::WebContentsModalDialogManagerDelegate implementation:
 
@@ -2429,7 +2422,6 @@ void Browser::SetAsDelegate(WebContents* web_contents, bool set_delegate) {
   WebContentsModalDialogManager::FromWebContents(web_contents)->
       SetDelegate(delegate);
   CoreTabHelper::FromWebContents(web_contents)->set_delegate(delegate);
-  SearchTabHelper::FromWebContents(web_contents)->set_delegate(delegate);
   translate::ContentTranslateDriver& content_translate_driver =
       ChromeTranslateClient::FromWebContents(web_contents)->translate_driver();
   if (delegate) {

chrome/browser/ui/browser.h

@@ -28,7 +28,6 @@
 #include "chrome/browser/ui/chrome_web_modal_dialog_manager_delegate.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h"
 #include "chrome/browser/ui/profile_chooser_constants.h"
-#include "chrome/browser/ui/search/search_tab_helper_delegate.h"
 #include "chrome/browser/ui/signin_view_controller.h"
 #include "chrome/browser/ui/tab_contents/core_tab_helper_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
@@ -106,7 +105,6 @@ class WebContentsModalDialogHost;
 class Browser : public TabStripModelObserver,
                 public content::WebContentsDelegate,
                 public CoreTabHelperDelegate,
-                public SearchTabHelperDelegate,
                 public ChromeWebModalDialogManagerDelegate,
                 public BookmarkTabHelperDelegate,
                 public zoom::ZoomObserver,
@@ -719,9 +717,6 @@ class Browser : public TabStripModelObserver,
   bool CanReloadContents(content::WebContents* web_contents) const override;
   bool CanSaveContents(content::WebContents* web_contents) const override;
 
-  // Overridden from SearchTabHelperDelegate:
-  OmniboxView* GetOmniboxView() override;
-
   // Overridden from WebContentsModalDialogManagerDelegate:
   void SetWebContentsBlocked(content::WebContents* web_contents,
                              bool blocked) override;

chrome/browser/ui/search/search_tab_helper.cc

@@ -23,7 +23,6 @@
 #include "chrome/browser/ui/omnibox/clipboard_utils.h"
 #include "chrome/browser/ui/search/instant_tab.h"
 #include "chrome/browser/ui/search/search_ipc_router_policy_impl.h"
-#include "chrome/browser/ui/search/search_tab_helper_delegate.h"
 #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/browser/ui/webui/ntp/ntp_user_data_logger.h"
 #include "chrome/common/url_constants.h"
@@ -141,8 +140,8 @@ SearchTabHelper::SearchTabHelper(content::WebContents* web_contents)
           web_contents,
           this,
           base::WrapUnique(new SearchIPCRouterPolicyImpl(web_contents))),
-      instant_service_(NULL),
-      delegate_(NULL) {
+      instant_service_(nullptr),
+      omnibox_view_(nullptr) {
   if (!is_search_enabled_)
     return;
 
@@ -177,7 +176,7 @@ void SearchTabHelper::OmniboxFocusChanged(OmniboxFocusState state,
   // Don't send oninputstart/oninputend updates in response to focus changes
   // if there's a navigation in progress. This prevents Chrome from sending
   // a spurious oninputend when the user accepts a match in the omnibox.
-  if (web_contents_->GetController().GetPendingEntry() == NULL)
+  if (web_contents_->GetController().GetPendingEntry() == nullptr)
     ipc_router_.SetInputInProgress(IsInputInProgress());
 }
 
@@ -198,6 +197,10 @@ void SearchTabHelper::Submit(const base::string16& text,
   ipc_router_.Submit(text, params);
 }
 
+void SearchTabHelper::OnTabAttachedToWindow(BrowserWindow* window) {
+  omnibox_view_ = window->GetLocationBar()->GetOmniboxView();
+}
+
 void SearchTabHelper::OnTabActivated() {
   ipc_router_.OnTabActivated();
 
@@ -341,8 +344,7 @@ void SearchTabHelper::MostVisitedItemsChanged(
 void SearchTabHelper::FocusOmnibox(OmniboxFocusState state) {
 // TODO(kmadhusu): Move platform specific code from here and get rid of #ifdef.
 #if !defined(OS_ANDROID)
-  OmniboxView* omnibox = GetOmniboxView();
-  if (!omnibox)
+  if (!omnibox_view_)
     return;
 
   // Do not add a default case in the switch block for the following reasons:
@@ -354,25 +356,25 @@ void SearchTabHelper::FocusOmnibox(OmniboxFocusState state) {
   // doing nothing instead of crashing the browser process (intentional no-op).
   switch (state) {
     case OMNIBOX_FOCUS_VISIBLE:
-      omnibox->SetFocus();
-      omnibox->model()->SetCaretVisibility(true);
+      omnibox_view_->SetFocus();
+      omnibox_view_->model()->SetCaretVisibility(true);
       break;
     case OMNIBOX_FOCUS_INVISIBLE:
-      omnibox->SetFocus();
-      omnibox->model()->SetCaretVisibility(false);
+      omnibox_view_->SetFocus();
+      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
       // text is a convenient way to accomplish this. It also gives a slight
       // visual cue to users who really understand selection state about what
       // will happen if they start typing.
-      omnibox->SelectAll(false);
-      omnibox->ShowImeIfNeeded();
+      omnibox_view_->SelectAll(false);
+      omnibox_view_->ShowImeIfNeeded();
       break;
     case OMNIBOX_FOCUS_NONE:
       // Remove focus only if the popup is closed. This will prevent someone
       // from changing the omnibox value and closing the popup without user
       // interaction.
-      if (!omnibox->model()->popup_model()->IsOpen())
+      if (!omnibox_view_->model()->popup_model()->IsOpen())
         web_contents()->Focus();
       break;
   }
@@ -428,26 +430,26 @@ void SearchTabHelper::OnLogMostVisitedNavigation(
 void SearchTabHelper::PasteIntoOmnibox(const base::string16& text) {
 // TODO(kmadhusu): Move platform specific code from here and get rid of #ifdef.
 #if !defined(OS_ANDROID)
-  OmniboxView* omnibox = GetOmniboxView();
-  if (!omnibox)
+  if (!omnibox_view_)
     return;
   // The first case is for right click to paste, where the text is retrieved
   // from the clipboard already sanitized. The second case is needed to handle
   // drag-and-drop value and it has to be sanitazed before setting it into the
   // omnibox.
   base::string16 text_to_paste =
-      text.empty() ? GetClipboardText() : omnibox->SanitizeTextForPaste(text);
+      text.empty() ? GetClipboardText()
+                   : omnibox_view_->SanitizeTextForPaste(text);
 
   if (text_to_paste.empty())
     return;
 
-  if (!omnibox->model()->has_focus())
-    omnibox->SetFocus();
+  if (!omnibox_view_->model()->has_focus())
+    omnibox_view_->SetFocus();
 
-  omnibox->OnBeforePossibleChange();
-  omnibox->model()->OnPaste();
-  omnibox->SetUserText(text_to_paste);
-  omnibox->OnAfterPossibleChange(true);
+  omnibox_view_->OnBeforePossibleChange();
+  omnibox_view_->model()->OnPaste();
+  omnibox_view_->SetUserText(text_to_paste);
+  omnibox_view_->OnAfterPossibleChange(true);
 #endif
 }
 
@@ -487,8 +489,7 @@ void SearchTabHelper::UpdateMode(bool update_origin) {
   if (!update_origin)
     origin = model_.mode().origin;
 
-  OmniboxView* omnibox = GetOmniboxView();
-  if (omnibox && omnibox->model()->user_input_in_progress())
+  if (omnibox_view_ && omnibox_view_->model()->user_input_in_progress())
     type = SearchMode::MODE_SEARCH_SUGGESTIONS;
 
   SearchMode old_mode(model_.mode());
@@ -518,11 +519,6 @@ Profile* SearchTabHelper::profile() const {
 }
 
 bool SearchTabHelper::IsInputInProgress() const {
-  OmniboxView* omnibox = GetOmniboxView();
-  return !model_.mode().is_ntp() && omnibox &&
-      omnibox->model()->focus_state() == OMNIBOX_FOCUS_VISIBLE;
-}
-
-OmniboxView* SearchTabHelper::GetOmniboxView() const {
-  return delegate_ ? delegate_->GetOmniboxView() : NULL;
+  return !model_.mode().is_ntp() && omnibox_view_ &&
+         omnibox_view_->model()->focus_state() == OMNIBOX_FOCUS_VISIBLE;
 }

chrome/browser/ui/search/search_tab_helper.h

@@ -27,13 +27,13 @@ class WebContents;
 struct LoadCommittedDetails;
 }
 
+class BrowserWindow;
 class GURL;
 class InstantService;
 class InstantTabTest;
 class OmniboxView;
 class Profile;
 class SearchIPCRouterTest;
-class SearchTabHelperDelegate;
 
 // Per-tab search "helper".  Acts as the owner and controller of the tab's
 // search UI model.
@@ -75,14 +75,16 @@ class SearchTabHelper : public content::WebContentsObserver,
   void Submit(const base::string16& text,
               const EmbeddedSearchRequestParams& params);
 
+  // Called when the tab corresponding to |this| instance is attached to a
+  // browser window.
+  void OnTabAttachedToWindow(BrowserWindow* window);
+
   // Called when the tab corresponding to |this| instance is activated.
   void OnTabActivated();
 
   // Called when the tab corresponding to |this| instance is deactivated.
   void OnTabDeactivated();
 
-  void set_delegate(SearchTabHelperDelegate* delegate) { delegate_ = delegate; }
-
   SearchIPCRouter& ipc_router_for_testing() { return ipc_router_; }
 
  private:
@@ -176,9 +178,6 @@ class SearchTabHelper : public content::WebContentsObserver,
   // active tab is in mode SEARCH_SUGGESTIONS.
   bool IsInputInProgress() const;
 
-  // Returns the OmniboxView for |web_contents_| or NULL if not available.
-  OmniboxView* GetOmniboxView() const;
-
   const bool is_search_enabled_;
 
   // Model object for UI that cares about search state.
@@ -190,10 +189,7 @@ class SearchTabHelper : public content::WebContentsObserver,
 
   InstantService* instant_service_;
 
-  // Delegate for notifying our owner about the SearchTabHelper state. Not owned
-  // by us.
-  // NULL on iOS and Android because they don't use the Instant framework.
-  SearchTabHelperDelegate* delegate_;
+  OmniboxView* omnibox_view_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchTabHelper);
 };

chrome/browser/ui/search/search_tab_helper_delegate.cc

-// Copyright 2014 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/search/search_tab_helper_delegate.h"
-
-OmniboxView* SearchTabHelperDelegate::GetOmniboxView() {
-  return nullptr;
-}
-
-SearchTabHelperDelegate::~SearchTabHelperDelegate() {
-}

chrome/browser/ui/search/search_tab_helper_delegate.h

-// Copyright 2014 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.
-
-#ifndef CHROME_BROWSER_UI_SEARCH_SEARCH_TAB_HELPER_DELEGATE_H_
-#define CHROME_BROWSER_UI_SEARCH_SEARCH_TAB_HELPER_DELEGATE_H_
-
-class OmniboxView;
-
-// Objects implement this interface to provide necessary functionality to
-// SearchTabHelper.
-class SearchTabHelperDelegate {
- public:
-  // Returns the OmniboxView or NULL if not available.
-  virtual OmniboxView* GetOmniboxView();
-
- protected:
-  virtual ~SearchTabHelperDelegate();
-};
-
-#endif  // CHROME_BROWSER_UI_SEARCH_SEARCH_TAB_HELPER_DELEGATE_H_