[Instant] Default Search Provider change redirects to local NTP in some cases

Previously, all instant renderers would reload if a search provider changed. With this patch, some of the renderers are redirected to the local NTP in the case that both the old and new search providers are google.<tld>.

BUG=456681
TEST=BrowserInstantControllerTest*, InstantService* unit_tests

Review URL: https://codereview.chromium.org/930853005

Cr-Commit-Position: refs/heads/master@{#317180}

chrome/browser/android/tab_android.cc

@@ -311,7 +311,8 @@ void TabAndroid::SwapTabContents(content::WebContents* old_contents,
       did_finish_load);
 }
 
-void TabAndroid::DefaultSearchProviderChanged() {
+void TabAndroid::DefaultSearchProviderChanged(
+    bool google_base_url_domain_changed) {
   // TODO(kmadhusu): Move this function definition to a common place and update
   // BrowserInstantController::DefaultSearchProviderChanged to use the same.
   if (!web_contents())

chrome/browser/android/tab_android.h

@@ -125,7 +125,8 @@ class TabAndroid : public CoreTabHelperDelegate,
                        bool did_finish_load) override;
 
   // Overridden from InstantServiceObserver:
-  void DefaultSearchProviderChanged() override;
+  void DefaultSearchProviderChanged(
+      bool google_base_url_domain_changed) override;
 
   // Overridden from SearchTabHelperDelegate:
   void OnWebContentsInstantSupportDisabled(

chrome/browser/search/instant_service.cc

@@ -407,18 +407,20 @@ void InstantService::OnTemplateURLServiceChanged() {
   // changed, the effective URLs might change if they reference the Google base
   // URL. The TemplateURLService will notify us when the effective URL changes
   // in this way but it's up to us to do the work to check both.
+  bool google_base_url_domain_changed = false;
   GURL google_base_url(UIThreadSearchTermsData(profile_).GoogleBaseURLValue());
   if (google_base_url != previous_google_base_url_) {
     previous_google_base_url_ = google_base_url;
     if (template_url && template_url->HasGoogleBaseURLs(
             UIThreadSearchTermsData(profile_)))
-      default_search_provider_changed = true;
+      google_base_url_domain_changed = true;
   }
 
-  if (default_search_provider_changed) {
+  if (default_search_provider_changed || google_base_url_domain_changed) {
     ResetInstantSearchPrerenderer();
-    FOR_EACH_OBSERVER(InstantServiceObserver, observers_,
-                      DefaultSearchProviderChanged());
+    FOR_EACH_OBSERVER(
+        InstantServiceObserver, observers_,
+        DefaultSearchProviderChanged(google_base_url_domain_changed));
   }
 }
 

chrome/browser/search/instant_service_observer.cc

@@ -11,7 +11,8 @@ void InstantServiceObserver::MostVisitedItemsChanged(
     const std::vector<InstantMostVisitedItem>&) {
 }
 
-void InstantServiceObserver::DefaultSearchProviderChanged() {
+void InstantServiceObserver::DefaultSearchProviderChanged(
+    bool google_base_url_domain_changed) {
 }
 
 void InstantServiceObserver::OmniboxStartMarginChanged(

chrome/browser/search/instant_service_observer.h

@@ -20,8 +20,11 @@ class InstantServiceObserver {
   virtual void MostVisitedItemsChanged(
       const std::vector<InstantMostVisitedItem>&);
 
-  // Indicates that the default search provider changed.
-  virtual void DefaultSearchProviderChanged();
+  // Indicates that the default search provider changed. Parameter indicates
+  // whether the Google base URL changed (such as when the user migrates from
+  // one google.<TLD> to another TLD).
+  virtual void DefaultSearchProviderChanged(
+      bool google_base_url_domain_changed);
 
   // Indicates that the omnibox start margin has changed.
   virtual void OmniboxStartMarginChanged(int omnibox_start_margin);

chrome/browser/search/instant_service_unittest.cc

@@ -28,7 +28,7 @@
 
 class MockInstantServiceObserver : public InstantServiceObserver {
  public:
-  MOCK_METHOD0(DefaultSearchProviderChanged, void());
+  MOCK_METHOD1(DefaultSearchProviderChanged, void(bool));
   MOCK_METHOD1(OmniboxStartMarginChanged, void(int));
 };
 
@@ -67,29 +67,29 @@ class InstantServiceEnabledTest : public InstantServiceTest {
 };
 
 TEST_F(InstantServiceEnabledTest, DispatchDefaultSearchProviderChanged) {
-  EXPECT_CALL(*instant_service_observer_.get(), DefaultSearchProviderChanged())
-      .Times(1);
+  EXPECT_CALL(*instant_service_observer_.get(),
+              DefaultSearchProviderChanged(false)).Times(1);
 
   const std::string new_base_url = "https://bar.com/";
   SetUserSelectedDefaultSearchProvider(new_base_url);
 }
 
 TEST_F(InstantServiceTest, DontDispatchGoogleURLUpdatedForNonGoogleURLs) {
-  EXPECT_CALL(*instant_service_observer_.get(), DefaultSearchProviderChanged())
-      .Times(1);
+  EXPECT_CALL(*instant_service_observer_.get(),
+              DefaultSearchProviderChanged(false)).Times(1);
   const std::string new_dsp_url = "https://bar.com/";
   SetUserSelectedDefaultSearchProvider(new_dsp_url);
   testing::Mock::VerifyAndClearExpectations(instant_service_observer_.get());
 
-  EXPECT_CALL(*instant_service_observer_.get(), DefaultSearchProviderChanged())
-      .Times(0);
+  EXPECT_CALL(*instant_service_observer_.get(),
+              DefaultSearchProviderChanged(false)).Times(0);
   const std::string new_base_url = "https://www.google.es/";
   NotifyGoogleBaseURLUpdate(new_base_url);
 }
 
 TEST_F(InstantServiceTest, DispatchGoogleURLUpdated) {
-  EXPECT_CALL(*instant_service_observer_.get(), DefaultSearchProviderChanged())
-      .Times(1);
+  EXPECT_CALL(*instant_service_observer_.get(),
+              DefaultSearchProviderChanged(true)).Times(1);
 
   const std::string new_base_url = "https://www.google.es/";
   NotifyGoogleBaseURLUpdate(new_base_url);
@@ -123,8 +123,8 @@ TEST_F(InstantServiceTest, InstantSearchEnabled) {
 
 TEST_F(InstantServiceEnabledTest,
        ResetInstantSearchPrerenderer_DefaultProviderChanged) {
-  EXPECT_CALL(*instant_service_observer_.get(), DefaultSearchProviderChanged())
-      .Times(2);
+  EXPECT_CALL(*instant_service_observer_.get(),
+              DefaultSearchProviderChanged(false)).Times(2);
 
   // Set a default search provider that doesn't support Instant.
   TemplateURLData data;
@@ -146,8 +146,8 @@ TEST_F(InstantServiceEnabledTest,
 
 TEST_F(InstantServiceEnabledTest,
        ResetInstantSearchPrerenderer_GoogleBaseURLUpdated) {
-  EXPECT_CALL(*instant_service_observer_.get(), DefaultSearchProviderChanged())
-      .Times(1);
+  EXPECT_CALL(*instant_service_observer_.get(),
+              DefaultSearchProviderChanged(true)).Times(1);
 
   InstantSearchPrerenderer* old_prerenderer = GetInstantSearchPrerenderer();
   EXPECT_TRUE(old_prerenderer != NULL);

chrome/browser/ui/browser_instant_controller.cc

@@ -21,10 +21,12 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/instant_types.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
-
+#include "content/public/common/referrer.h"
+#include "ui/base/page_transition_types.h"
 
 // Helpers --------------------------------------------------------------------
 
@@ -140,7 +142,8 @@ void BrowserInstantController::ModelChanged(
     instant_.InstantSupportChanged(new_state.instant_support);
 }
 
-void BrowserInstantController::DefaultSearchProviderChanged() {
+void BrowserInstantController::DefaultSearchProviderChanged(
+    bool google_base_url_domain_changed) {
   InstantService* instant_service =
       InstantServiceFactory::GetForProfile(profile());
   if (!instant_service)
@@ -157,17 +160,28 @@ void BrowserInstantController::DefaultSearchProviderChanged() {
     content::RenderProcessHost* rph = contents->GetRenderProcessHost();
     instant_service->SendSearchURLsToRenderer(rph);
 
-    // Reload the contents to ensure that it gets assigned to a non-priviledged
-    // renderer.
     if (!instant_service->IsInstantProcess(rph->GetID()))
       continue;
 
-    contents->GetController().Reload(false);
-
-    // As the reload was not triggered by the user we don't want to close any
-    // infobars. We have to tell the InfoBarService after the reload, otherwise
-    // it would ignore this call when
-    // WebContentsObserver::DidStartNavigationToPendingEntry is invoked.
-    InfoBarService::FromWebContents(contents)->set_ignore_next_reload();
+    if (google_base_url_domain_changed &&
+        SearchTabHelper::FromWebContents(contents)->model()->mode().is_ntp()) {
+      // Replace the server NTP with the local NTP.
+      content::NavigationController::LoadURLParams
+          params(chrome::GetLocalInstantURL(profile()));
+      params.should_replace_current_entry = true;
+      params.referrer = content::Referrer();
+      params.transition_type = ui::PAGE_TRANSITION_RELOAD;
+      contents->GetController().LoadURLWithParams(params);
+    } else {
+      // Reload the contents to ensure that it gets assigned to a
+      // non-priviledged renderer.
+      contents->GetController().Reload(false);
+
+      // As the reload was not triggered by the user we don't want to close any
+      // infobars. We have to tell the InfoBarService after the reload,
+      // otherwise it would ignore this call when
+      // WebContentsObserver::DidStartNavigationToPendingEntry is invoked.
+      InfoBarService::FromWebContents(contents)->set_ignore_next_reload();
+    }
   }
 }

chrome/browser/ui/browser_instant_controller.h

@@ -53,7 +53,8 @@ class BrowserInstantController : public SearchModelObserver,
                     const SearchModel::State& new_state) override;
 
   // InstantServiceObserver:
-  void DefaultSearchProviderChanged() override;
+  void DefaultSearchProviderChanged(
+      bool google_base_url_domain_changed) override;
 
   // Replaces the contents at tab |index| with |new_contents| and deletes the
   // existing contents.

chrome/browser/ui/browser_instant_controller_unittest.cc

@@ -36,23 +36,40 @@ class BrowserInstantControllerTest : public InstantUnitTestBase {
   friend class FakeWebContentsObserver;
 };
 
-const struct TabReloadTestCase {
+struct TabReloadTestCase {
   const char* description;
   const char* start_url;
   bool start_in_instant_process;
   bool should_reload;
+  bool end_in_local_ntp;
   bool end_in_instant_process;
-} kTabReloadTestCases[] = {
+};
+
+// Test cases for when Google is the initial, but not final provider.
+const TabReloadTestCase kTabReloadTestCasesFinalProviderNotGoogle[] = {
+    {"Local Embedded NTP", chrome::kChromeSearchLocalNtpUrl,
+     true, true, true, true},
+    {"Remote Embedded NTP", "https://www.google.com/newtab",
+     true, true, false, false},
+    {"Remote Embedded SERP", "https://www.google.com/url?strk&bar=search+terms",
+     true, true, false, false},
+    {"Other NTP", "https://bar.com/newtab",
+     false, false, false, false}
+};
+
+// Test cases for when Google is both the initial and final provider.
+const TabReloadTestCase kTabReloadTestCasesFinalProviderGoogle[] = {
     {"Local Embedded NTP", chrome::kChromeSearchLocalNtpUrl,
-     true, true, true},
-    {"Remote Embedded NTP", "https://www.google.com/instant?strk",
-     true, true, false},
+     true, true, true, true},
+    {"Remote Embedded NTP", "https://www.google.com/newtab",
+     true, false, true, true},
     {"Remote Embedded SERP", "https://www.google.com/url?strk&bar=search+terms",
-     true, true, false},
-    {"Other NTP", "https://bar.com/instant?strk",
-     false, false, false}
+     true, true, false, false},
+    {"Other NTP", "https://bar.com/newtab",
+     false, false, false, false}
 };
 
+
 class FakeWebContentsObserver : public content::WebContentsObserver {
  public:
   explicit FakeWebContentsObserver(content::WebContents* contents)
@@ -66,16 +83,25 @@ class FakeWebContentsObserver : public content::WebContentsObserver {
       content::NavigationController::ReloadType reload_type) override {
     if (url_ == url)
       num_reloads_++;
+    current_url_ = url;
   }
 
   const GURL url() const {
     return url_;
   }
 
+  const GURL current_url() const {
+    return contents_->GetURL();
+  }
+
   int num_reloads() const {
     return num_reloads_;
   }
 
+  bool can_go_back() const {
+    return contents_->GetController().CanGoBack();
+  }
+
  protected:
   friend class BrowserInstantControllerTest;
   FRIEND_TEST_ALL_PREFIXES(BrowserInstantControllerTest,
@@ -86,14 +112,16 @@ class FakeWebContentsObserver : public content::WebContentsObserver {
  private:
   content::WebContents* contents_;
   const GURL& url_;
+  GURL current_url_;
   int num_reloads_;
 };
 
 TEST_F(BrowserInstantControllerTest, DefaultSearchProviderChanged) {
-  size_t num_tests = arraysize(kTabReloadTestCases);
+  size_t num_tests = arraysize(kTabReloadTestCasesFinalProviderNotGoogle);
   ScopedVector<FakeWebContentsObserver> observers;
   for (size_t i = 0; i < num_tests; ++i) {
-    const TabReloadTestCase& test = kTabReloadTestCases[i];
+    const TabReloadTestCase& test =
+        kTabReloadTestCasesFinalProviderNotGoogle[i];
     AddTab(browser(), GURL(test.start_url));
     content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -112,27 +140,34 @@ TEST_F(BrowserInstantControllerTest, DefaultSearchProviderChanged) {
 
   for (size_t i = 0; i < num_tests; ++i) {
     FakeWebContentsObserver* observer = observers[i];
-    const TabReloadTestCase& test = kTabReloadTestCases[i];
+    const TabReloadTestCase& test =
+        kTabReloadTestCasesFinalProviderNotGoogle[i];
 
     if (test.should_reload) {
       // Validate final instant state.
       EXPECT_EQ(
           test.end_in_instant_process,
-          chrome::ShouldAssignURLToInstantRenderer(observer->url(), profile()))
+          chrome::ShouldAssignURLToInstantRenderer(
+              observer->current_url(), profile()))
         << test.description;
     }
 
     // Ensure only the expected tabs(contents) reloaded.
     EXPECT_EQ(test.should_reload ? 1 : 0, observer->num_reloads())
       << test.description;
+
+    if (test.end_in_local_ntp) {
+      EXPECT_EQ(GURL(chrome::kChromeSearchLocalNtpUrl), observer->current_url())
+          << test.description;
+    }
   }
 }
 
 TEST_F(BrowserInstantControllerTest, GoogleBaseURLUpdated) {
-  const size_t num_tests = arraysize(kTabReloadTestCases);
+  const size_t num_tests = arraysize(kTabReloadTestCasesFinalProviderGoogle);
   ScopedVector<FakeWebContentsObserver> observers;
   for (size_t i = 0; i < num_tests; ++i) {
-    const TabReloadTestCase& test = kTabReloadTestCases[i];
+    const TabReloadTestCase& test = kTabReloadTestCasesFinalProviderGoogle[i];
     AddTab(browser(), GURL(test.start_url));
     content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -150,20 +185,26 @@ TEST_F(BrowserInstantControllerTest, GoogleBaseURLUpdated) {
   NotifyGoogleBaseURLUpdate("https://www.google.es/");
 
   for (size_t i = 0; i < num_tests; ++i) {
-    const TabReloadTestCase& test = kTabReloadTestCases[i];
+    const TabReloadTestCase& test = kTabReloadTestCasesFinalProviderGoogle[i];
     FakeWebContentsObserver* observer = observers[i];
 
-    if (test.should_reload) {
-      // Validate final instant state.
-      EXPECT_EQ(
-          test.end_in_instant_process,
-          chrome::ShouldAssignURLToInstantRenderer(observer->url(), profile()))
-        << test.description;
-    }
+    // Validate final instant state.
+    EXPECT_EQ(
+        test.end_in_instant_process,
+        chrome::ShouldAssignURLToInstantRenderer(
+            observer->current_url(), profile()))
+      << test.description;
 
     // Ensure only the expected tabs(contents) reloaded.
     EXPECT_EQ(test.should_reload ? 1 : 0, observer->num_reloads())
       << test.description;
+
+    if (test.end_in_local_ntp) {
+      EXPECT_EQ(GURL(chrome::kChromeSearchLocalNtpUrl), observer->current_url())
+          << test.description;
+      // The navigation to Local NTP should be definitive i.e. can't go back.
+      EXPECT_FALSE(observer->can_go_back());
+    }
   }
 }