Turn backend tip service on by default.

BUG= none.
TEST= New NTP should come up with a default title in Tips and Suggestions box, then be loaded with actual data from popgadget.


Review URL: http://codereview.chromium.org/149255

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20466 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 62 insertions, 23 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index bf11fb4..4c742a2 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -751,7 +751,7 @@ int BrowserMain(const MainFunctionParams& parameters) {
   profile->InitExtensions();
   // Start up the web resource service.  This starts loading data after a
   // short delay so as not to interfere with startup time.
-  if (parsed_command_line.HasSwitch(switches::kWebResources))
+  if (!parsed_command_line.HasSwitch(switches::kDisableWebResources))
     profile->InitWebResources();
 
   int result_code = ResultCodes::NORMAL_EXIT;
diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc
index 2f6d724..8525e08 100644
--- a/chrome/browser/dom_ui/new_tab_ui.cc
+++ b/chrome/browser/dom_ui/new_tab_ui.cc
@@ -1426,7 +1426,7 @@ NewTabUI::NewTabUI(TabContents* contents)
     AddMessageHandler((new MostVisitedHandler())->Attach(this));
     AddMessageHandler((new RecentlyClosedTabsHandler())->Attach(this));
     AddMessageHandler((new MetricsHandler())->Attach(this));
-    if (EnableWebResources())
+    if (!WebResourcesDisabled())
       AddMessageHandler((new TipsHandler())->Attach(this));
 
     if (UseOldNewTabPage()) {
@@ -1484,7 +1484,7 @@ void NewTabUI::Observe(NotificationType type,
 void NewTabUI::RegisterUserPrefs(PrefService* prefs) {
   MostVisitedHandler::RegisterUserPrefs(prefs);
   ShownSectionsHandler::RegisterUserPrefs(prefs);
-  if (NewTabUI::EnableWebResources())
+  if (!NewTabUI::WebResourcesDisabled())
     TipsHandler::RegisterUserPrefs(prefs);
 }
 
@@ -1495,7 +1495,7 @@ bool NewTabUI::UseOldNewTabPage() {
 }
 
 // static
-bool NewTabUI::EnableWebResources() {
+bool NewTabUI::WebResourcesDisabled() {
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  return command_line->HasSwitch(switches::kWebResources);
+  return command_line->HasSwitch(switches::kDisableWebResources);
 }
diff --git a/chrome/browser/dom_ui/new_tab_ui.h b/chrome/browser/dom_ui/new_tab_ui.h
index b387115..4149a46 100644
--- a/chrome/browser/dom_ui/new_tab_ui.h
+++ b/chrome/browser/dom_ui/new_tab_ui.h
@@ -28,8 +28,8 @@ class NewTabUI : public DOMUI,
   // Whether we should use the old new tab page.
   static bool UseOldNewTabPage();
 
-  // Whether we should enable the web resources backend service
-  static bool EnableWebResources();
+  // Whether we should disable the web resources backend service
+  static bool WebResourcesDisabled();
 
  private:
   void Observe(NotificationType type,
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index 8654509..46ba216 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -566,6 +566,9 @@ void ProfileImpl::InitExtensions() {
 }
 
 void ProfileImpl::InitWebResources() {
+  if (web_resource_service_)
+    return;  // Already initialized.
+
   web_resource_service_ = new WebResourceService(
       this,
       g_browser_process->file_thread()->message_loop());
diff --git a/chrome/browser/utility_process_host.cc b/chrome/browser/utility_process_host.cc
index 84f506a..ae82e70 100644
--- a/chrome/browser/utility_process_host.cc
+++ b/chrome/browser/utility_process_host.cc
@@ -9,6 +9,7 @@
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
+#include "base/string_util.h"
 #include "base/task.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/render_messages.h"
@@ -59,6 +60,10 @@ std::wstring UtilityProcessHost::GetUtilityProcessCmd() {
 }
 
 bool UtilityProcessHost::StartProcess(const FilePath& exposed_dir) {
+  // Name must be set or metrics_service will crash in any test which
+  // launches a UtilityProcessHost.
+  set_name(L"utility_process_host_" + ASCIIToWide(channel_id()));
+
   if (!CreateChannel())
     return false;
 
diff --git a/chrome/browser/web_resource/web_resource_service.cc b/chrome/browser/web_resource/web_resource_service.cc
index 4ee5dff..5692fdb 100644
--- a/chrome/browser/web_resource/web_resource_service.cc
+++ b/chrome/browser/web_resource/web_resource_service.cc
@@ -44,6 +44,8 @@ class WebResourceService::WebResourceFetcher
     // If we are still fetching data, exit.
     if (web_resource_service_->in_fetch_)
       return;
+    else
+      web_resource_service_->in_fetch_ = true;
 
     url_fetcher_.reset(new URLFetcher(GURL(
         WideToUTF8(web_resource_service_->web_resource_server_)),
@@ -95,7 +97,7 @@ class WebResourceService::UnpackerClient
   UnpackerClient(WebResourceService* web_resource_service,
                  const std::string& json_data)
     : web_resource_service_(web_resource_service),
-      json_data_(json_data) {
+      json_data_(json_data), got_response_(false) {
   }
 
   void Start() {
@@ -107,18 +109,43 @@ class WebResourceService::UnpackerClient
                             web_resource_service_->resource_dispatcher_host_,
                             MessageLoop::current()));
     } else {
-      // TODO(mrc): unit tests here.
+      // If we don't have a resource_dispatcher_host_, assume we're in
+      // a test and run the unpacker directly in-process.
+      WebResourceUnpacker unpacker(json_data_);
+      if (unpacker.Run()) {
+        OnUnpackWebResourceSucceeded(*unpacker.parsed_json());
+      } else {
+        OnUnpackWebResourceFailed(unpacker.error_message());
+      }
     }
   }
 
  private:
+  // UtilityProcessHost::Client
+  virtual void OnProcessCrashed() {
+    if (got_response_)
+      return;
+
+    OnUnpackWebResourceFailed(
+        "Chrome crashed while trying to retrieve web resources.");
+  }
+
   virtual void OnUnpackWebResourceSucceeded(const ListValue& parsed_json) {
     web_resource_service_->OnWebResourceUnpacked(parsed_json);
-    Release();
+    Cleanup();
   }
 
   virtual void OnUnpackWebResourceFailed(const std::string& error_message) {
     web_resource_service_->EndFetch();
+    Cleanup();
+  }
+
+  // Release reference and set got_response_.
+  void Cleanup() {
+    if (got_response_)
+      return;
+
+    got_response_ = true;
     Release();
   }
 
@@ -134,6 +161,10 @@ class WebResourceService::UnpackerClient
 
   // Holds raw JSON string.
   const std::string& json_data_;
+
+  // True if we got a response from the utility process and have cleaned up
+  // already.
+  bool got_response_;
 };
 
 // TODO(mrc): make into a changeable preference.
@@ -222,24 +253,22 @@ void WebResourceService::OnWebResourceUnpacked(const ListValue& parsed_json) {
 }
 
 void WebResourceService::StartAfterDelay() {
-  int64 delay = kStartResourceFetchDelay;
+  int delay = kStartResourceFetchDelay;
   // Check whether we have ever put a value in the web resource cache;
   // if so, pull it out and see if it's time to update again.
   if (prefs_->HasPrefPath(prefs::kNTPTipsCacheUpdate)) {
     std::wstring last_update_pref =
       prefs_->GetString(prefs::kNTPTipsCacheUpdate);
-    int64 ms_since_update =
-        (base::Time::Now() - base::Time::FromDoubleT(
-        StringToDouble(WideToASCII(last_update_pref)))).InMilliseconds();
+    int ms_until_update = kCacheUpdateDelay -
+        static_cast<int>((base::Time::Now() - base::Time::FromDoubleT(
+        StringToDouble(WideToASCII(last_update_pref)))).InMilliseconds());
 
-    delay = kStartResourceFetchDelay +
-        (ms_since_update > kCacheUpdateDelay ?
-         0 : kCacheUpdateDelay - ms_since_update);
+    delay = ms_until_update > kCacheUpdateDelay ?
+            kCacheUpdateDelay : (ms_until_update < kStartResourceFetchDelay ?
+                                 kStartResourceFetchDelay : ms_until_update);
   }
 
   // Start fetch and wait for UpdateResourceCache.
-  DCHECK(delay >= kStartResourceFetchDelay &&
-         delay <= kCacheUpdateDelay);
   web_resource_fetcher_->StartAfterDelay(static_cast<int>(delay));
 }
 
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index cef4dad..c38e0b1 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -495,9 +495,9 @@ const wchar_t kNewTabPage[]                    = L"new-tab-page";
 // Switches back to the old new tab page.
 const wchar_t kOldNewTabPage[]                 = L"old-new-tab-page";
 
-// Enables the prototype of the backend service for web resources, used in the
-// new new tab page for loading tips and recommendations from a JSON feed.
-const wchar_t kWebResources[]                  = L"enable-web-resources";
+// Disables the backend service for web resources, used in the new tab page for
+// loading tips and recommendations from a JSON feed.
+const wchar_t kDisableWebResources[]           = L"disable-web-resources";
 
 // Disables the default browser check. Useful for UI/browser tests where we want
 // to avoid having the default browser info-bar displayed.
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index ef46c6c..3588b46 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -184,7 +184,7 @@ extern const wchar_t kForceFieldTestNameAndValue[];
 extern const wchar_t kNewTabPage[];
 extern const wchar_t kOldNewTabPage[];
 
-extern const wchar_t kWebResources[];
+extern const wchar_t kDisableWebResources[];
 
 extern const wchar_t kEnableBenchmarking[];
 
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 2295781..447464f 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -377,6 +377,8 @@ void UITest::LaunchBrowser(const CommandLine& arguments, bool clear_profile) {
   if (!homepage_.empty())
     command_line.AppendSwitchWithValue(switches::kHomePage,
                                        homepage_);
+  // Don't try to fetch web resources during UI testing.
+  command_line.AppendSwitch(switches::kDisableWebResources);
 #if defined(OS_POSIX)
   const char* alternative_userdir = getenv("CHROME_UI_TESTS_USER_DATA_DIR");
 #else