Handle TemplateURLService load failure better, and make some test correctness fixes that will be ne…

chrome/browser/ui/browser_instant_controller.cc

Index: chrome/browser/ui/browser_instant_controller.cc
diff --git a/chrome/browser/ui/browser_instant_controller.cc b/chrome/browser/ui/browser_instant_controller.cc
index d1d8e92bacc7cbaceafe2dd5b7ac894afa35f811..294b95907af3742e6a1acceb51e45b61bc349de5 100644
--- a/chrome/browser/ui/browser_instant_controller.cc
+++ b/chrome/browser/ui/browser_instant_controller.cc
@@ -26,7 +26,8 @@
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 
-using base::UserMetricsAction;
+
+// Helpers --------------------------------------------------------------------
 
 namespace {
 
@@ -39,8 +40,8 @@ InstantSearchPrerenderer* GetInstantSearchPrerenderer(Profile* profile) {
 
 }  // namespace
 
-////////////////////////////////////////////////////////////////////////////////
-// BrowserInstantController, public:
+
+// BrowserInstantController ---------------------------------------------------
 
 BrowserInstantController::BrowserInstantController(Browser* browser)
     : browser_(browser),
@@ -119,9 +120,6 @@ void BrowserInstantController::TabDeactivated(content::WebContents* contents) {
     prerenderer->Cancel();
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// BrowserInstantController, SearchModelObserver implementation:
-
 void BrowserInstantController::ModelChanged(
     const SearchModel::State& old_state,
     const SearchModel::State& new_state) {
@@ -132,9 +130,9 @@ void BrowserInstantController::ModelChanged(
     // the full story, it's necessary to look at other UMA actions as well,
     // such as tab switches.
     if (new_mode.is_search_results())
-      content::RecordAction(UserMetricsAction("InstantExtended.ShowSRP"));
+      content::RecordAction(base::UserMetricsAction("InstantExtended.ShowSRP"));
     else if (new_mode.is_ntp())
-      content::RecordAction(UserMetricsAction("InstantExtended.ShowNTP"));
+      content::RecordAction(base::UserMetricsAction("InstantExtended.ShowNTP"));
 
     instant_.SearchModeChanged(old_state.mode, new_mode);
   }
@@ -143,18 +141,9 @@ void BrowserInstantController::ModelChanged(
     instant_.InstantSupportChanged(new_state.instant_support);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// BrowserInstantController, InstantServiceObserver implementation:
-
+// Reloads the tabs in instant process to ensure that their privileged status
+// is still valid.

[Peter Kasting, 2014/05/08 21:04:41]

Nit: Instead of putting this here, I'd try to just improve the almost-identical
comment within the method as necessary.

Not only will that be less comment duplication, but since this method clearly
does more than just this sentence, putting such a comment here is somewhat
misleading.

[erikwright (departed), 2014/05/08 21:07:24]

jered: Can you suggest a way that the comments in this method could be reworked
to reflect its intent to someone with less intimate knowledge of
TemplateURLService?

[Jered, 2014/05/08 21:12:38]

I missed the comment below. Given that comment, I agree this is redundant, and
vote we just delete the one above. Diving into this method more feels outside
the scope of this CL.

[erikwright (departed), 2014/05/09 00:46:43]

Done.

 void BrowserInstantController::DefaultSearchProviderChanged() {
-  ReloadTabsInInstantProcess();
-}
-
-void BrowserInstantController::GoogleURLUpdated() {
-  ReloadTabsInInstantProcess();
-}
-
-void BrowserInstantController::ReloadTabsInInstantProcess() {
   InstantService* instant_service =
       InstantServiceFactory::GetForProfile(profile());
   if (!instant_service)