BrowserInstantController: on default search engine change, post a task rather than reloading directly

BUG=536174

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

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

3 files changed, 48 insertions, 13 deletions

diff --git a/chrome/browser/ui/browser_instant_controller.cc b/chrome/browser/ui/browser_instant_controller.cc
index cd0f66e..8625b58 100644
--- a/chrome/browser/ui/browser_instant_controller.cc
+++ b/chrome/browser/ui/browser_instant_controller.cc
@@ -21,10 +21,12 @@
 #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"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_user_data.h"
 #include "content/public/common/referrer.h"
 #include "ui/base/page_transition_types.h"
 
@@ -39,8 +41,47 @@ InstantSearchPrerenderer* GetInstantSearchPrerenderer(Profile* profile) {
   return instant_service ? instant_service->instant_search_prerenderer() : NULL;
 }
 
+// Helper class for posting a task to reload a tab, to avoid doing a re-entrant
+// navigation, since it can be called when starting a navigation. This class
+// makes sure to only execute the reload if the WebContents still exists.
+class TabReloader : public content::WebContentsUserData<TabReloader> {
+ public:
+  static void Reload(content::WebContents* web_contents) {
+    TabReloader::CreateForWebContents(web_contents);
+  }
+
+ private:
+  friend class content::WebContentsUserData<TabReloader>;
+
+  explicit TabReloader(content::WebContents* web_contents)
+      : web_contents_(web_contents), weak_ptr_factory_(this) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&TabReloader::ReloadImpl, weak_ptr_factory_.GetWeakPtr()));
+  }
+  ~TabReloader() override {}
+
+  void ReloadImpl() {
+    web_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(web_contents_)->set_ignore_next_reload();
+
+    web_contents_->RemoveUserData(UserDataKey());
+  }
+
+  content::WebContents* web_contents_;
+  base::WeakPtrFactory<TabReloader> weak_ptr_factory_;
+};
+
 }  // namespace
 
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabReloader);
+
 
 // BrowserInstantController ---------------------------------------------------
 
@@ -174,14 +215,8 @@ void BrowserInstantController::DefaultSearchProviderChanged(
       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();
+      // non-privileged renderer.
+      TabReloader::Reload(contents);
     }
   }
 }
diff --git a/chrome/browser/ui/browser_instant_controller.h b/chrome/browser/ui/browser_instant_controller.h
index 6cd99cb..6c09621 100644
--- a/chrome/browser/ui/browser_instant_controller.h
+++ b/chrome/browser/ui/browser_instant_controller.h
@@ -5,8 +5,6 @@
 #ifndef CHROME_BROWSER_UI_BROWSER_INSTANT_CONTROLLER_H_
 #define CHROME_BROWSER_UI_BROWSER_INSTANT_CONTROLLER_H_
 
-#include <string>
-
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "chrome/browser/search/instant_service_observer.h"
@@ -14,7 +12,6 @@
 #include "chrome/browser/ui/search/search_model_observer.h"
 
 class Browser;
-struct InstantSuggestion;
 class Profile;
 
 namespace content {
diff --git a/chrome/browser/ui/browser_instant_controller_unittest.cc b/chrome/browser/ui/browser_instant_controller_unittest.cc
index f07ddbb..428d707 100644
--- a/chrome/browser/ui/browser_instant_controller_unittest.cc
+++ b/chrome/browser/ui/browser_instant_controller_unittest.cc
@@ -4,12 +4,11 @@
 
 #include <stddef.h>
 
-#include <string>
-
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/scoped_vector.h"
 #include "base/metrics/field_trial.h"
+#include "base/run_loop.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_observer.h"
@@ -151,6 +150,8 @@ TEST_F(BrowserInstantControllerTest, DefaultSearchProviderChanged) {
     }
 
     // Ensure only the expected tabs(contents) reloaded.
+    base::RunLoop loop;
+    loop.RunUntilIdle();
     EXPECT_EQ(test.should_reload ? 1 : 0, observer->num_reloads())
       << test.description;
 
@@ -193,6 +194,8 @@ TEST_F(BrowserInstantControllerTest, GoogleBaseURLUpdated) {
         << test.description;
 
     // Ensure only the expected tabs(contents) reloaded.
+    base::RunLoop loop;
+    loop.RunUntilIdle();
     EXPECT_EQ(test.should_reload ? 1 : 0, observer->num_reloads())
       << test.description;