Instant cleanup: Remove SearchModel and SearchDelegate

This stuff was a LOT of plumbing, all to eventually determine "is the
current tab an NTP?". That can be had MUCH simpler :)

Before:
SearchTabHelper (per-tab) has a SearchModel and updates that with the
"isNTP" state. The state for the active tab gets propagated via
SearchDelegate to BrowserInstantController's SearchModel instance.
InstantController listens for changes to that, and on non-NTP->NTP
transitions sends the "Instant info" (i.e. MV tiles and theme) to the
Instant process.

After:
InstantController watches for relevant navigations itself and just
checks the "isNTP" state directly. Several kilometers of plumbing are
removed, and there is much rejoicing.

Manually tested the following cases:
- Navigating regular page <-> remote NTP
- Navigating regular page <-> local NTP
- Navigating local NTP <-> remote NTP
All these for regular and forward/back navigations, and for Google and
third-party DSE.

Bug: 627747
Change-Id: Id55b1f28550b4aeee1e5472ac0e8eebd0ee0d65c
Reviewed-on: https://chromium-review.googlesource.com/678719Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504342}

chrome/browser/search/search.h

@@ -10,7 +10,6 @@
 #include <vector>
 
 #include "base/strings/string16.h"
-#include "chrome/browser/ui/search/search_model.h"
 
 class GURL;
 class Profile;

chrome/browser/ui/BUILD.gn

@@ -165,9 +165,6 @@ split_static_library("ui") {
     "search/search_ipc_router.h",
     "search/search_ipc_router_policy_impl.cc",
     "search/search_ipc_router_policy_impl.h",
-    "search/search_model.cc",
-    "search/search_model.h",
-    "search/search_model_observer.h",
     "search/search_tab_helper.cc",
     "search/search_tab_helper.h",
     "search_engines/edit_search_engine_controller.cc",
@@ -979,8 +976,6 @@ split_static_library("ui") {
       "scoped_tabbed_browser_displayer.h",
       "search/instant_controller.cc",
       "search/instant_controller.h",
-      "search/search_delegate.cc",
-      "search/search_delegate.h",
       "settings_window_manager_chromeos.cc",
       "settings_window_manager_chromeos.h",
       "settings_window_manager_observer_chromeos.h",

chrome/browser/ui/browser_instant_controller.cc

@@ -13,13 +13,8 @@
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/search/search_model.h"
-#include "chrome/browser/ui/search/search_tab_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/common/search/instant_types.h"
 #include "chrome/common/url_constants.h"
-#include "components/omnibox/browser/omnibox_popup_model.h"
-#include "components/omnibox/browser/omnibox_view.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/render_process_host.h"
@@ -78,7 +73,7 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabReloader);
 // BrowserInstantController ---------------------------------------------------
 
 BrowserInstantController::BrowserInstantController(Browser* browser)
-    : browser_(browser), search_delegate_(&search_model_), instant_(this) {
+    : browser_(browser), instant_(this) {
   TemplateURLService* template_url_service =
       TemplateURLServiceFactory::GetForProfile(profile());
   // TemplateURLService can be null in tests.
@@ -98,25 +93,19 @@ Profile* BrowserInstantController::profile() const {
   return browser_->profile();
 }
 
-content::WebContents* BrowserInstantController::GetActiveWebContents() const {
-  return browser_->tab_strip_model()->GetActiveWebContents();
-}
-
 void BrowserInstantController::OnTabActivated(
     content::WebContents* web_contents) {
-  // Note: Order matters, |search_delegate_| must be notified first.
-  search_delegate_.OnTabActivated(web_contents);
-  instant_.ActiveTabChanged();
+  instant_.OnTabActivated(web_contents);
 }
 
 void BrowserInstantController::OnTabDeactivated(
     content::WebContents* web_contents) {
-  search_delegate_.OnTabDeactivated(web_contents);
+  instant_.OnTabDeactivated(web_contents);
 }
 
 void BrowserInstantController::OnTabDetached(
     content::WebContents* web_contents) {
-  search_delegate_.OnTabDetached(web_contents);
+  instant_.OnTabDetached(web_contents);
 }
 
 void BrowserInstantController::OnSearchEngineBaseURLChanged(
@@ -143,11 +132,9 @@ void BrowserInstantController::OnSearchEngineBaseURLChanged(
     bool google_base_url_domain_changed =
         change_reason ==
         SearchEngineBaseURLTracker::ChangeReason::GOOGLE_BASE_URL;
-    SearchModel* model = SearchTabHelper::FromWebContents(contents)->model();
-    if (google_base_url_domain_changed &&
-        model->origin() == SearchModel::Origin::NTP) {
+    if (google_base_url_domain_changed) {
       GURL local_ntp_url(chrome::kChromeSearchLocalNtpUrl);
-      // Replace the server NTP with the local NTP.
+      // Replace the server NTP with the local NTP (or reload the local NTP).
       content::NavigationController::LoadURLParams params(local_ntp_url);
       params.should_replace_current_entry = true;
       params.referrer = content::Referrer();

chrome/browser/ui/browser_instant_controller.h

@@ -11,8 +11,6 @@
 #include "base/macros.h"
 #include "chrome/browser/search/search_engine_base_url_tracker.h"
 #include "chrome/browser/ui/search/instant_controller.h"
-#include "chrome/browser/ui/search/search_delegate.h"
-#include "chrome/browser/ui/search/search_model.h"
 
 class Browser;
 class Profile;
@@ -36,11 +34,6 @@ class BrowserInstantController {
 
   InstantController* instant() { return &instant_; }
 
-  SearchModel* search_model() { return &search_model_; }
-
-  // Invoked by |instant_| to get the currently active tab.
-  content::WebContents* GetActiveWebContents() const;
-
   // Invoked by |browser_| when the active tab changes.
   // TODO(treib): Implement TabStripModelObserver instead of relying on custom
   // callbacks from Browser.
@@ -54,17 +47,6 @@ class BrowserInstantController {
 
   Browser* const browser_;
 
-  // The model for the "active" search state.  There are per-tab search models
-  // as well.  When a tab is active its model is kept in sync with this one.
-  // When a new tab is activated its model state is propagated to this active
-  // model.  This way, observers only have to attach to this single model for
-  // updates, and don't have to worry about active tab changes directly.
-  SearchModel search_model_;
-
-  // A delegate that handles the details of updating the "active"
-  // |search_model_| state with the tab's state.
-  SearchDelegate search_delegate_;
-
   InstantController instant_;
 
   std::unique_ptr<SearchEngineBaseURLTracker> search_engine_base_url_tracker_;

chrome/browser/ui/browser_instant_controller_unittest.cc

@@ -27,13 +27,6 @@ namespace chrome {
 namespace {
 
 class BrowserInstantControllerTest : public InstantUnitTestBase {
- public:
-  void SetUp() override {
-    ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial(
-        "EmbeddedSearch", "Group1 use_cacheable_ntp:1"));
-    InstantUnitTestBase::SetUp();
-  }
-
  protected:
   friend class FakeWebContentsObserver;
 };

chrome/browser/ui/search/instant_controller.cc

@@ -9,13 +9,13 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/memory/ptr_util.h"
-#include "base/strings/stringprintf.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
+#include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
-#include "chrome/browser/ui/search/search_tab_helper.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -28,10 +28,20 @@ class InstantController::TabObserver : public content::WebContentsObserver {
 
  private:
   // Overridden from content::WebContentsObserver:
+  void NavigationEntryCommitted(
+      const content::LoadCommittedDetails& load_details) override {
+    if (load_details.is_main_frame && search::IsInstantNTP(web_contents())) {
+      callback_.Run();
+    }
+  }
+
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override {
+    // TODO(treib): Verify if this is necessary - NavigationEntryCommitted
+    // should already cover all cases.
     if (navigation_handle->HasCommitted() &&
-        navigation_handle->IsInMainFrame()) {
+        navigation_handle->IsInMainFrame() &&
+        search::IsInstantNTP(web_contents())) {
       callback_.Run();
     }
   }
@@ -43,30 +53,30 @@ class InstantController::TabObserver : public content::WebContentsObserver {
 
 InstantController::InstantController(
     BrowserInstantController* browser_instant_controller)
-    : browser_instant_controller_(browser_instant_controller) {
-  browser_instant_controller_->search_model()->AddObserver(this);
-}
+    : browser_instant_controller_(browser_instant_controller) {}
 
-InstantController::~InstantController() {
-  browser_instant_controller_->search_model()->RemoveObserver(this);
+InstantController::~InstantController() = default;
+
+void InstantController::OnTabActivated(content::WebContents* web_contents) {
+  if (!tab_observer_ || tab_observer_->web_contents() != web_contents) {
+    tab_observer_ = std::make_unique<TabObserver>(
+        web_contents, base::Bind(&InstantController::UpdateInfoForInstantTab,
+                                 base::Unretained(this)));
+    // If this tab is an NTP, immediately send it the required info.
+    if (search::IsInstantNTP(web_contents)) {
+      UpdateInfoForInstantTab();
+    }
+  }
 }
 
-void InstantController::ModelChanged(SearchModel::Origin old_origin,
-                                     SearchModel::Origin new_origin) {
-  // The search mode in the active tab has changed. Bind |instant_tab_observer_|
-  // if the |new_origin| reflects an Instant NTP.
-  // Note: This can be called either because the SearchMode changed within the
-  // current tab, or because the active tab changed. In the latter case, this
-  // gets called before ActiveTabChanged().
-  LogDebugEvent(
-      base::StringPrintf("ModelChanged: %d to %d", old_origin, new_origin));
-
-  ResetInstantTab();
+void InstantController::OnTabDeactivated(content::WebContents* web_contents) {
+  if (tab_observer_ && tab_observer_->web_contents() == web_contents) {
+    tab_observer_ = nullptr;
+  }
 }
 
-void InstantController::ActiveTabChanged() {
-  LogDebugEvent("ActiveTabChanged");
-  ResetInstantTab();
+void InstantController::OnTabDetached(content::WebContents* web_contents) {
+  OnTabDeactivated(web_contents);
 }
 
 void InstantController::LogDebugEvent(const std::string& info) const {
@@ -83,40 +93,7 @@ void InstantController::ClearDebugEvents() {
   debug_events_.clear();
 }
 
-void InstantController::InstantTabAboutToNavigateMainFrame() {
-  DCHECK(instant_tab_observer_);
-  // The Instant tab navigated (which means it had instant support both before
-  // and after the navigation). This may cause it to be assigned to a new
-  // renderer process, which doesn't have the most visited/theme data yet, so
-  // send it now.
-  // TODO(treib): This seems unnecessarily convoluted and fragile. Can't we just
-  // send this when the Instant process is created?
-  UpdateInfoForInstantTab();
-}
-
-void InstantController::ResetInstantTab() {
-  content::WebContents* active_tab =
-      browser_instant_controller_->GetActiveWebContents();
-  if (active_tab &&
-      SearchTabHelper::FromWebContents(active_tab)->model()->origin() ==
-          SearchModel::Origin::NTP) {
-    // The active tab is an NTP. If we're not already tracking it, do so and
-    // also update the required info.
-    if (!instant_tab_observer_ ||
-        active_tab != instant_tab_observer_->web_contents()) {
-      instant_tab_observer_ = base::MakeUnique<TabObserver>(
-          active_tab,
-          base::Bind(&InstantController::InstantTabAboutToNavigateMainFrame,
-                     base::Unretained(this)));
-      UpdateInfoForInstantTab();
-    }
-  } else {
-    instant_tab_observer_ = nullptr;
-  }
-}
-
 void InstantController::UpdateInfoForInstantTab() {
-  DCHECK(instant_tab_observer_);
   InstantService* instant_service = InstantServiceFactory::GetForProfile(
       browser_instant_controller_->profile());
   if (instant_service) {

chrome/browser/ui/search/instant_controller.h

@@ -14,29 +14,28 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
-#include "chrome/browser/ui/search/search_model_observer.h"
 
 class BrowserInstantController;
 
+namespace content {
+class WebContents;
+}  // namespace content
+
 // InstantController is responsible for updating the theme and most visited info
 // of the current tab when
 // * the user switches to an existing NTP tab, or
 // * an open tab navigates to an NTP.
 //
 // InstantController is owned by Browser via BrowserInstantController.
-class InstantController : public SearchModelObserver {
+class InstantController {
  public:
   explicit InstantController(
       BrowserInstantController* browser_instant_controller);
-  ~InstantController() override;
-
-  // SearchModelObserver:
-  void ModelChanged(SearchModel::Origin old_origin,
-                    SearchModel::Origin new_origin) override;
+  ~InstantController();
 
-  // The user switched tabs. Bind |instant_tab_observer_| if the newly active
-  // tab is an Instant NTP.
-  void ActiveTabChanged();
+  void OnTabActivated(content::WebContents* web_contents);
+  void OnTabDeactivated(content::WebContents* web_contents);
+  void OnTabDetached(content::WebContents* web_contents);
 
   // Resets list of debug events.
   void ClearDebugEvents();
@@ -56,23 +55,17 @@ class InstantController : public SearchModelObserver {
   FRIEND_TEST_ALL_PREFIXES(InstantExtendedTest,
                            DispatchMVChangeEventWhileNavigatingBackToNTP);
 
-  void InstantTabAboutToNavigateMainFrame();
-
   // Adds a new event to |debug_events_| and also DVLOG's it. Ensures that
   // |debug_events_| doesn't get too large.
   void LogDebugEvent(const std::string& info) const;
 
-  // If the active tab is an Instant NTP, sets |instant_tab_| to point to it.
-  // Else, deletes any existing |instant_tab_|.
-  void ResetInstantTab();
-
   // Sends theme info and most visited items to the Instant renderer process.
   void UpdateInfoForInstantTab();
 
   BrowserInstantController* const browser_instant_controller_;
 
-  // Only non-null if the current tab is an Instant tab, i.e. an NTP.
-  std::unique_ptr<TabObserver> instant_tab_observer_;
+  // Observes the currently active tab, and calls us back if it becomes an NTP.
+  std::unique_ptr<TabObserver> tab_observer_;
 
   // List of events and their timestamps, useful in debugging Instant behaviour.
   mutable std::list<std::pair<int64_t, std::string>> debug_events_;

chrome/browser/ui/search/search_delegate.cc

-// Copyright (c) 2012 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_delegate.h"
-
-#include "chrome/browser/ui/search/search_model.h"
-#include "chrome/browser/ui/search/search_tab_helper.h"
-
-SearchDelegate::SearchDelegate(SearchModel* browser_search_model)
-    : browser_model_(browser_search_model), tab_model_(nullptr) {}
-
-SearchDelegate::~SearchDelegate() {
-  DCHECK(!tab_model_) << "All tabs should have been deactivated or closed.";
-}
-
-void SearchDelegate::ModelChanged(SearchModel::Origin old_origin,
-                                  SearchModel::Origin new_origin) {
-  browser_model_->SetOrigin(new_origin);
-}
-
-void SearchDelegate::OnTabActivated(content::WebContents* web_contents) {
-  if (tab_model_)
-    tab_model_->RemoveObserver(this);
-  tab_model_ = SearchTabHelper::FromWebContents(web_contents)->model();
-  browser_model_->SetOrigin(tab_model_->origin());
-  tab_model_->AddObserver(this);
-}
-
-void SearchDelegate::OnTabDeactivated(content::WebContents* web_contents) {
-  StopObservingTab(web_contents);
-}
-
-void SearchDelegate::OnTabDetached(content::WebContents* web_contents) {
-  StopObservingTab(web_contents);
-}
-
-void SearchDelegate::StopObservingTab(content::WebContents* web_contents) {
-  SearchTabHelper* search_tab_helper =
-      SearchTabHelper::FromWebContents(web_contents);
-  if (search_tab_helper->model() == tab_model_) {
-    tab_model_->RemoveObserver(this);
-    tab_model_ = NULL;
-  }
-}

chrome/browser/ui/search/search_delegate.h

-// Copyright (c) 2012 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_DELEGATE_H_
-#define CHROME_BROWSER_UI_SEARCH_SEARCH_DELEGATE_H_
-
-#include "base/compiler_specific.h"
-#include "base/macros.h"
-#include "chrome/browser/ui/search/search_model_observer.h"
-
-namespace content {
-class WebContents;
-}
-
-class SearchModel;
-
-// The SearchDelegate class acts as a helper to the Browser class.
-// It is responsible for routing the changes from the active tab's
-// SearchModel through to the toolbar, tabstrip and other UI
-// observers.
-// Changes are propagated from the active tab's model via this class to the
-// Browser-level model.
-class SearchDelegate : public SearchModelObserver {
- public:
-  explicit SearchDelegate(SearchModel* browser_search_model);
-  ~SearchDelegate() override;
-
-  // Overrides for SearchModelObserver:
-  void ModelChanged(SearchModel::Origin old_origin,
-                    SearchModel::Origin new_origin) override;
-
-  // When the active tab is changed, the model state of this new active tab is
-  // propagated to the browser.
-  void OnTabActivated(content::WebContents* web_contents);
-
-  // When a tab is deactivated, this class no longer observes changes to the
-  // tab's model.
-  void OnTabDeactivated(content::WebContents* web_contents);
-
-  // When a tab is detached, this class no longer observes changes to the
-  // tab's model.
-  void OnTabDetached(content::WebContents* web_contents);
-
- private:
-  // Stop observing tab.
-  void StopObservingTab(content::WebContents* web_contents);
-
-  // Weak.  The Browser class owns this.  The active |tab_model_| state is
-  // propagated to the |browser_model_|.
-  SearchModel* browser_model_;
-
-  // Weak.  The WebContents owns this.  It is the model of the active
-  // tab.  Changes to this model are propagated through to the |browser_model_|.
-  SearchModel* tab_model_;
-
-  DISALLOW_COPY_AND_ASSIGN(SearchDelegate);
-};
-
-#endif  // CHROME_BROWSER_UI_SEARCH_SEARCH_DELEGATE_H_

chrome/browser/ui/search/search_delegate_unittest.cc

-// Copyright (c) 2012 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/browser.h"
-#include "chrome/browser/ui/browser_instant_controller.h"
-#include "chrome/browser/ui/search/search_model.h"
-#include "chrome/browser/ui/search/search_tab_helper.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/test/base/browser_with_test_window_test.h"
-
-typedef BrowserWithTestWindowTest SearchDelegateTest;
-
-// Test the propagation of search "mode" changes from the tab's search model to
-// the browser's search model.
-TEST_F(SearchDelegateTest, SearchModel) {
-  const SearchModel& browser_search_model =
-      *browser()->instant_controller()->search_model();
-
-  // Initial state.
-  EXPECT_EQ(SearchModel::Origin::DEFAULT, browser_search_model.origin());
-
-  // Propagate change from tab's search model to browser's search model.
-  AddTab(browser(), GURL("http://foo/0"));
-  content::WebContents* first_tab =
-      browser()->tab_strip_model()->GetWebContentsAt(0);
-  SearchTabHelper::FromWebContents(first_tab)->model()->SetOrigin(
-      SearchModel::Origin::NTP);
-  EXPECT_EQ(SearchModel::Origin::NTP, browser_search_model.origin());
-
-  // Add second tab (it gets inserted at index 0), make it active, and make sure
-  // its mode changes propagate to the browser's search model.
-  AddTab(browser(), GURL("http://foo/1"));
-  content::WebContents* second_tab =
-      browser()->tab_strip_model()->GetWebContentsAt(0);
-  ASSERT_NE(first_tab, second_tab);
-  browser()->tab_strip_model()->ActivateTabAt(0, true);
-  EXPECT_EQ(SearchModel::Origin::DEFAULT, browser_search_model.origin());
-  SearchTabHelper::FromWebContents(second_tab)
-      ->model()
-      ->SetOrigin(SearchModel::Origin::NTP);
-  EXPECT_EQ(SearchModel::Origin::NTP, browser_search_model.origin());
-
-  // The first tab is not active so changes should not propagate.
-  SearchTabHelper::FromWebContents(first_tab)->model()->SetOrigin(
-      SearchModel::Origin::DEFAULT);
-  EXPECT_EQ(SearchModel::Origin::NTP, browser_search_model.origin());
-}

chrome/browser/ui/search/search_model.cc

-// Copyright (c) 2012 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_model.h"
-
-#include "chrome/browser/ui/search/search_model_observer.h"
-#include "components/search/search.h"
-
-SearchModel::SearchModel() : origin_(Origin::DEFAULT) {}
-
-SearchModel::~SearchModel() = default;
-
-void SearchModel::SetOrigin(Origin origin) {
-  DCHECK(search::IsInstantExtendedAPIEnabled())
-      << "Please do not try to set the SearchModel mode without first "
-      << "checking if Search is enabled.";
-
-  if (origin_ == origin)
-    return;
-
-  const Origin old_origin = origin_;
-  origin_ = origin;
-
-  for (SearchModelObserver& observer : observers_)
-    observer.ModelChanged(old_origin, origin_);
-}
-
-void SearchModel::AddObserver(SearchModelObserver* observer) {
-  observers_.AddObserver(observer);
-}
-
-void SearchModel::RemoveObserver(SearchModelObserver* observer) {
-  observers_.RemoveObserver(observer);
-}

chrome/browser/ui/search/search_model.h

-// Copyright (c) 2012 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_MODEL_H_
-#define CHROME_BROWSER_UI_SEARCH_SEARCH_MODEL_H_
-
-#include "base/macros.h"
-#include "base/observer_list.h"
-
-class SearchModelObserver;
-
-// An observable model for UI components that care about search model state
-// changes.
-class SearchModel {
- public:
-  enum class Origin {
-    // The user is on some page other than the NTP.
-    DEFAULT = 0,
-
-    // The user is on the NTP.
-    NTP,
-  };
-
-  SearchModel();
-  ~SearchModel();
-
-  // Change the origin.  Change notifications are sent to observers.
-  void SetOrigin(Origin origin);
-
-  // Get the active origin.
-  Origin origin() const { return origin_; }
-
-  // Add and remove observers.
-  void AddObserver(SearchModelObserver* observer);
-  void RemoveObserver(SearchModelObserver* observer);
-
- private:
-  Origin origin_;
-
-  base::ObserverList<SearchModelObserver> observers_;
-
-  DISALLOW_COPY_AND_ASSIGN(SearchModel);
-};
-
-#endif  // CHROME_BROWSER_UI_SEARCH_SEARCH_MODEL_H_

chrome/browser/ui/search/search_model_observer.h

-// Copyright (c) 2012 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_MODEL_OBSERVER_H_
-#define CHROME_BROWSER_UI_SEARCH_SEARCH_MODEL_OBSERVER_H_
-
-#include "chrome/browser/ui/search/search_model.h"
-
-// This class defines the observer interface for the |SearchModel|.
-class SearchModelObserver {
- public:
-  // Informs the observer that the model's state has changed.
-  virtual void ModelChanged(SearchModel::Origin old_origin,
-                            SearchModel::Origin new_origin) = 0;
-
- protected:
-  virtual ~SearchModelObserver() {}
-};
-
-#endif  // CHROME_BROWSER_UI_SEARCH_SEARCH_MODEL_OBSERVER_H_

chrome/browser/ui/search/search_model_unittest.cc

-// Copyright 2013 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_model.h"
-
-#include "base/command_line.h"
-#include "base/macros.h"
-#include "chrome/browser/ui/search/search_model_observer.h"
-#include "chrome/browser/ui/search/search_tab_helper.h"
-#include "chrome/test/base/chrome_render_view_host_test_harness.h"
-
-namespace {
-
-class MockSearchModelObserver : public SearchModelObserver {
- public:
-  MockSearchModelObserver();
-  ~MockSearchModelObserver() override;
-
-  void ModelChanged(SearchModel::Origin old_origin,
-                    SearchModel::Origin new_origin) override;
-
-  void VerifySearchModelStates(SearchModel::Origin expected_old_origin,
-                               SearchModel::Origin expected_new_origin);
-
-  void VerifyNotificationCount(int expected_count);
-
- private:
-  // How many times we've seen search model changed notifications.
-  int modelchanged_notification_count_;
-
-  SearchModel::Origin actual_old_origin_;
-  SearchModel::Origin actual_new_origin_;
-
-  DISALLOW_COPY_AND_ASSIGN(MockSearchModelObserver);
-};
-
-MockSearchModelObserver::MockSearchModelObserver()
-    : modelchanged_notification_count_(0) {
-}
-
-MockSearchModelObserver::~MockSearchModelObserver() {
-}
-
-void MockSearchModelObserver::ModelChanged(SearchModel::Origin old_origin,
-                                           SearchModel::Origin new_origin) {
-  actual_old_origin_ = old_origin;
-  actual_new_origin_ = new_origin;
-  modelchanged_notification_count_++;
-}
-
-void MockSearchModelObserver::VerifySearchModelStates(
-    SearchModel::Origin expected_old_origin,
-    SearchModel::Origin expected_new_origin) {
-  EXPECT_TRUE(actual_old_origin_ == expected_old_origin);
-  EXPECT_TRUE(actual_new_origin_ == expected_new_origin);
-}
-
-void MockSearchModelObserver::VerifyNotificationCount(int expected_count) {
-  EXPECT_EQ(modelchanged_notification_count_, expected_count);
-}
-
-}  // namespace
-
-class SearchModelTest : public ChromeRenderViewHostTestHarness {
- public:
-  void SetUp() override;
-  void TearDown() override;
-
-  MockSearchModelObserver mock_observer;
-  SearchModel* model;
-};
-
-void SearchModelTest::SetUp() {
-  ChromeRenderViewHostTestHarness::SetUp();
-  SearchTabHelper::CreateForWebContents(web_contents());
-  SearchTabHelper* search_tab_helper =
-      SearchTabHelper::FromWebContents(web_contents());
-  ASSERT_TRUE(search_tab_helper != NULL);
-  model = search_tab_helper->model();
-  model->AddObserver(&mock_observer);
-}
-
-void SearchModelTest::TearDown() {
-  model->RemoveObserver(&mock_observer);
-  ChromeRenderViewHostTestHarness::TearDown();
-}
-
-TEST_F(SearchModelTest, UpdateSearchModelOrigin) {
-  mock_observer.VerifyNotificationCount(0);
-  SearchModel::Origin origin = SearchModel::Origin::NTP;
-  SearchModel::Origin expected_old_origin = model->origin();
-  SearchModel::Origin expected_new_origin = origin;
-  model->SetOrigin(origin);
-  mock_observer.VerifySearchModelStates(expected_old_origin,
-                                        expected_new_origin);
-  mock_observer.VerifyNotificationCount(1);
-
-  origin = SearchModel::Origin::DEFAULT;
-  expected_old_origin = expected_new_origin;
-  expected_new_origin = origin;
-  model->SetOrigin(origin);
-  mock_observer.VerifySearchModelStates(expected_old_origin,
-                                        expected_new_origin);
-  mock_observer.VerifyNotificationCount(2);
-  EXPECT_EQ(expected_new_origin, model->origin());
-}

chrome/browser/ui/search/search_tab_helper.cc

@@ -58,17 +58,6 @@ bool IsCacheableNTP(const content::WebContents* contents) {
          entry->GetURL() != chrome::kChromeSearchLocalNtpUrl;
 }
 
-bool IsNTP(const content::WebContents* contents) {
-  // We can't use WebContents::GetURL() because that uses the active entry,
-  // whereas we want the visible entry.
-  const content::NavigationEntry* entry =
-      contents->GetController().GetVisibleEntry();
-  if (entry && entry->GetVirtualURL() == chrome::kChromeUINewTabURL)
-    return true;
-
-  return search::IsInstantNTP(contents);
-}
-
 // Returns true if |contents| are rendered inside an Instant process.
 bool InInstantProcess(Profile* profile,
                       const content::WebContents* contents) {
@@ -278,7 +267,8 @@ void SearchTabHelper::NavigationEntryCommitted(
   if (!load_details.is_main_frame)
     return;
 
-  UpdateMode();
+  if (search::IsInstantNTP(web_contents_))
+    ipc_router_.SetInputInProgress(IsInputInProgress());
 
   if (InInstantProcess(profile(), web_contents_))
     ipc_router_.OnNavigationEntryCommitted();
@@ -425,16 +415,6 @@ void SearchTabHelper::OnHistorySyncCheck() {
   ipc_router_.SendHistorySyncCheckResult(IsHistorySyncEnabled(profile()));
 }
 
-void SearchTabHelper::UpdateMode() {
-  bool is_ntp = IsNTP(web_contents_);
-
-  model_.SetOrigin(is_ntp ? SearchModel::Origin::NTP
-                          : SearchModel::Origin::DEFAULT);
-
-  if (is_ntp)
-    ipc_router_.SetInputInProgress(IsInputInProgress());
-}
-
 const OmniboxView* SearchTabHelper::GetOmniboxView() const {
 #if defined(OS_ANDROID)
   return nullptr;

chrome/browser/ui/search/search_tab_helper.h

@@ -13,7 +13,6 @@
 #include "base/time/time.h"
 #include "chrome/browser/search/instant_service_observer.h"
 #include "chrome/browser/ui/search/search_ipc_router.h"
-#include "chrome/browser/ui/search/search_model.h"
 #include "chrome/common/search/instant_types.h"
 #include "chrome/common/search/ntp_logging_events.h"
 #include "components/ntp_tiles/tile_source.h"
@@ -47,10 +46,6 @@ class SearchTabHelper : public content::WebContentsObserver,
  public:
   ~SearchTabHelper() override;
 
-  SearchModel* model() {
-    return &model_;
-  }
-
   // Invoked when the omnibox input state is changed in some way that might
   // affect the search mode.
   void OmniboxInputStateChanged();
@@ -123,9 +118,6 @@ class SearchTabHelper : public content::WebContentsObserver,
   void MostVisitedItemsChanged(
       const std::vector<InstantMostVisitedItem>& items) override;
 
-  // Sets the mode of the model based on the current URL of web_contents().
-  void UpdateMode();
-
   OmniboxView* GetOmniboxView();
   const OmniboxView* GetOmniboxView() const;
 
@@ -137,9 +129,6 @@ class SearchTabHelper : public content::WebContentsObserver,
 
   const bool is_search_enabled_;
 
-  // Model object for UI that cares about search state.
-  SearchModel model_;
-
   content::WebContents* web_contents_;
 
   SearchIPCRouter ipc_router_;

chrome/test/BUILD.gn

@@ -3754,10 +3754,8 @@ test("unit_tests") {
       "../browser/ui/passwords/manage_passwords_bubble_model_unittest.cc",
       "../browser/ui/passwords/password_dialog_controller_impl_unittest.cc",
       "../browser/ui/search/ntp_user_data_logger_unittest.cc",
-      "../browser/ui/search/search_delegate_unittest.cc",
       "../browser/ui/search/search_ipc_router_policy_unittest.cc",
       "../browser/ui/search/search_ipc_router_unittest.cc",
-      "../browser/ui/search/search_model_unittest.cc",
       "../browser/ui/search/search_tab_helper_unittest.cc",
       "../browser/ui/tab_contents/tab_contents_iterator_unittest.cc",
       "../browser/ui/tabs/pinned_tab_codec_unittest.cc",