Extracted inner class doing process dispatch.

chrome/browser/web_resource/web_resource_service.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/web_resource/web_resource_service.h"
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/prefs/pref_service.h"
-#include "chrome/common/chrome_switches.h"
-#include "chrome/common/chrome_utility_messages.h"
-#include "chrome/common/web_resource/web_resource_unpacker.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/resource_dispatcher_host.h"
-#include "content/public/browser/utility_process_host.h"
-#include "content/public/browser/utility_process_host_client.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
 
-using content::BrowserThread;
-using content::UtilityProcessHost;
-using content::UtilityProcessHostClient;
-
-// This class coordinates a web resource unpack and parse task which is run in
-// a separate process.  Results are sent back to this class and routed to
-// the WebResourceService.
-class WebResourceService::UnpackerClient : public UtilityProcessHostClient {
- public:
-  explicit UnpackerClient(WebResourceService* web_resource_service)
-    : web_resource_service_(web_resource_service),
-      got_response_(false) {
-  }
-
-  void Start(const std::string& json_data) {
-    AddRef();  // balanced in Cleanup.
-
-    // TODO(willchan): Look for a better signal of whether we're in a unit test
-    // or not. Using |ResourceDispatcherHost::Get()| for this is pretty lame.
-    // If we don't have a ResourceDispatcherHost, assume we're in a test and
-    // run the unpacker directly in-process.
-    bool use_utility_process =
-        content::ResourceDispatcherHost::Get() &&
-        !CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess);
-    if (use_utility_process) {
-      BrowserThread::ID thread_id;
-      CHECK(BrowserThread::GetCurrentThreadIdentifier(&thread_id));
-      BrowserThread::PostTask(
-          BrowserThread::IO, FROM_HERE,
-          base::Bind(&UnpackerClient::StartProcessOnIOThread,
-                     this, thread_id, json_data));
-    } else {
-      WebResourceUnpacker unpacker(json_data);
-      if (unpacker.Run()) {
-        OnUnpackWebResourceSucceeded(*unpacker.parsed_json());
-      } else {
-        OnUnpackWebResourceFailed(unpacker.error_message());
-      }
-    }
-  }
-
- private:
-  virtual ~UnpackerClient() {}
-
-  // UtilityProcessHostClient
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
-    bool handled = true;
-    IPC_BEGIN_MESSAGE_MAP(WebResourceService::UnpackerClient, message)
-      IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_UnpackWebResource_Succeeded,
-                          OnUnpackWebResourceSucceeded)
-      IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_UnpackWebResource_Failed,
-                          OnUnpackWebResourceFailed)
-      IPC_MESSAGE_UNHANDLED(handled = false)
-    IPC_END_MESSAGE_MAP()
-    return handled;
-  }
-
-  virtual void OnProcessCrashed(int exit_code) OVERRIDE {
-    if (got_response_)
-      return;
-
-    OnUnpackWebResourceFailed(
-        "Utility process crashed while trying to retrieve web resources.");
-  }
-
-  void OnUnpackWebResourceSucceeded(
-      const DictionaryValue& parsed_json) {
-    web_resource_service_->Unpack(parsed_json);
-    Cleanup();
-  }
-
-  void OnUnpackWebResourceFailed(const std::string& error_message) {
-    LOG(ERROR) << error_message;
-    Cleanup();
-  }
-
-  // Release reference and set got_response_.
-  void Cleanup() {
-    DCHECK(!got_response_);
-    got_response_ = true;
-
-    web_resource_service_->EndFetch();
-    Release();
-  }
-
-  void StartProcessOnIOThread(BrowserThread::ID thread_id,
-                              const std::string& json_data) {
-    UtilityProcessHost* host = UtilityProcessHost::Create(this, thread_id);
-    host->EnableZygote();
-    // TODO(mrc): get proper file path when we start using web resources
-    // that need to be unpacked.
-    host->Send(new ChromeUtilityMsg_UnpackWebResource(json_data));
-  }
-
-  scoped_refptr<WebResourceService> web_resource_service_;
-
-  // True if we got a response from the utility process and have cleaned up
-  // already.
-  bool got_response_;
-};
-
 WebResourceService::WebResourceService(
     PrefService* prefs,
     const GURL& web_resource_server,
     bool apply_locale_to_url,
     const char* last_update_time_pref_name,
     int start_fetch_delay_ms,
     int cache_update_delay_ms)
     : prefs_(prefs),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
+      json_unpacker_(NULL),
       in_fetch_(false),
       web_resource_server_(web_resource_server),
       apply_locale_to_url_(apply_locale_to_url),
       last_update_time_pref_name_(last_update_time_pref_name),
       start_fetch_delay_ms_(start_fetch_delay_ms),
       cache_update_delay_ms_(cache_update_delay_ms) {
   DCHECK(prefs);
 }
 
-WebResourceService::~WebResourceService() { }
+WebResourceService::~WebResourceService() {
+  if (in_fetch_)
+    EndFetch();
+}
+
+void WebResourceService::OnUnpackFinished(const DictionaryValue& parsed_json) {

[Miranda Callahan, 2012/07/09 14:24:30]

Having these in both the WRS and the derived PRS is seems awkward, as all these
are doing is wrapping a method we already have defined -- and they're not using
the parameters passed in. I think it makes more sense to make these pure virtual
methods that the derived class must define. Then the derived class sensibly
calls "EndFetch" at the end of their unpackFinished and unpackError methods,
rather than awkwardly calling up to the superclass, and passing information that
is never used. 

[noyau (Ping after 24h), 2012/07/09 15:23:57]

EndFetch IMHO should stay private to the class. It seem awkward to ask a
subclass to implement two virtual method and not forget to call a method on the
superclass.

Instead I turned this around: only the superclass implements the delegate and I
returned the virtual version of the Unpack method. 

+  EndFetch();
+}
+
+void WebResourceService::OnUnpackError(const std::string& error_message) {
+  EndFetch();
+}
 
 void WebResourceService::EndFetch() {
+  if (json_unpacker_) {
+    json_unpacker_->ClearDelegate();
+    json_unpacker_ = NULL;
+  }
   in_fetch_ = false;
 }
 
 void WebResourceService::StartAfterDelay() {
   int64 delay = start_fetch_delay_ms_;
   // 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(last_update_time_pref_name_)) {
     std::string last_update_pref =
         prefs_->GetString(last_update_time_pref_name_);
@@ skipping 60 matching lines @@
 
 void WebResourceService::OnURLFetchComplete(const net::URLFetcher* source) {
   // Delete the URLFetcher when this function exits.
   scoped_ptr<net::URLFetcher> clean_up_fetcher(url_fetcher_.release());
 
   if (source->GetStatus().is_success() && source->GetResponseCode() == 200) {
     std::string data;
     source->GetResponseAsString(&data);
 
     // UnpackerClient calls EndFetch and releases itself on completion.
-    UnpackerClient* client = new UnpackerClient(this);
-    client->Start(data);
+    json_unpacker_ = JSONAsynchronousUnpacker::Create(this);
+    json_unpacker_->Start(data);
   } else {
     // Don't parse data if attempt to download was unsuccessful.
     // Stop loading new web resource data, and silently exit.
     // We do not call UnpackerClient, so we need to call EndFetch ourselves.
     EndFetch();
   }
 
   Release();
 }