Switch to using one ResourceDispatcher per render process, and move it to ChildThread so that the same code is used by the plugin process (and soon, workers).

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

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

11 files changed, 81 insertions, 120 deletions

diff --git a/chrome/common/child_thread.cc b/chrome/common/child_thread.cc
index 7a6e39c..237e53c 100644
--- a/chrome/common/child_thread.cc
+++ b/chrome/common/child_thread.cc
@@ -62,6 +62,10 @@ void ChildThread::RemoveRoute(int32 routing_id) {
 }
 
 void ChildThread::OnMessageReceived(const IPC::Message& msg) {
+  // Resource responses are sent to the resource dispatcher.
+  if (resource_dispatcher_->OnMessageReceived(msg))
+    return;
+
   if (msg.routing_id() == MSG_ROUTING_CONTROL) {
     OnControlMessageReceived(msg);
   } else {
@@ -80,6 +84,8 @@ void ChildThread::Init() {
 #ifdef IPC_MESSAGE_LOG_ENABLED
   IPC::Logging::current()->SetIPCSender(this);
 #endif
+
+  resource_dispatcher_.reset(new ResourceDispatcher(this));
 }
 
 void ChildThread::CleanUp() {
@@ -89,4 +95,5 @@ void ChildThread::CleanUp() {
   // Need to destruct the SyncChannel to the browser before we go away because
   // it caches a pointer to this thread.
   channel_.reset();
+  resource_dispatcher_.reset();
 }
diff --git a/chrome/common/child_thread.h b/chrome/common/child_thread.h
index f97964c..c499cb5 100644
--- a/chrome/common/child_thread.h
+++ b/chrome/common/child_thread.h
@@ -8,6 +8,7 @@
 #include "base/thread.h"
 #include "chrome/common/ipc_sync_channel.h"
 #include "chrome/common/message_router.h"
+#include "chrome/common/resource_dispatcher.h"
 
 // Child processes's background thread should derive from this class.
 class ChildThread : public IPC::Channel::Listener,
@@ -27,6 +28,10 @@ class ChildThread : public IPC::Channel::Listener,
 
   MessageLoop* owner_loop() { return owner_loop_; }
 
+  ResourceDispatcher* resource_dispatcher() {
+    return resource_dispatcher_.get();
+  }
+
  protected:
   friend class ChildProcess;
 
@@ -68,6 +73,10 @@ class ChildThread : public IPC::Channel::Listener,
 
   Thread::Options options_;
 
+  // Handles resource loads for this process.
+  // NOTE: this object lives on the owner thread.
+  scoped_ptr<ResourceDispatcher> resource_dispatcher_;
+
   DISALLOW_EVIL_CONSTRUCTORS(ChildThread);
 };
 
diff --git a/chrome/common/resource_dispatcher.cc b/chrome/common/resource_dispatcher.cc
index 9b57396..61c5013 100644
--- a/chrome/common/resource_dispatcher.cc
+++ b/chrome/common/resource_dispatcher.cc
@@ -52,7 +52,8 @@ class IPCResourceLoaderBridge : public ResourceLoaderBridge {
                           int origin_pid,
                           ResourceType::Type resource_type,
                           bool mixed_content,
-                          uint32 request_context);
+                          uint32 request_context,
+                          int route_id);
   virtual ~IPCResourceLoaderBridge();
 
   // ResourceLoaderBridge
@@ -71,8 +72,9 @@ class IPCResourceLoaderBridge : public ResourceLoaderBridge {
  private:
   ResourceLoaderBridge::Peer* peer_;
 
-  // The resource dispatcher for this loader.
-  scoped_refptr<ResourceDispatcher> dispatcher_;
+  // The resource dispatcher for this loader.  The bridge doesn't own it, but
+  // it's guaranteed to outlive the bridge.
+  ResourceDispatcher* dispatcher_;
 
   // The request to send, created on initialization for modification and
   // appending data.
@@ -81,6 +83,9 @@ class IPCResourceLoaderBridge : public ResourceLoaderBridge {
   // ID for the request, valid once Start()ed, -1 if not valid yet.
   int request_id_;
 
+  // The routing id used when sending IPC messages.
+  int route_id_;
+
 #ifdef LOG_RESOURCE_REQUESTS
   // indicates the URL of this resource request for help debugging
   std::string url_;
@@ -98,10 +103,12 @@ IPCResourceLoaderBridge::IPCResourceLoaderBridge(
     int origin_pid,
     ResourceType::Type resource_type,
     bool mixed_content,
-    uint32 request_context)
+    uint32 request_context,
+    int route_id)
     : peer_(NULL),
       dispatcher_(dispatcher),
-      request_id_(-1) {
+      request_id_(-1),
+      route_id_(route_id) {
   DCHECK(dispatcher_) << "no resource dispatcher";
   request_.method = method;
   request_.url = url;
@@ -164,13 +171,8 @@ bool IPCResourceLoaderBridge::Start(Peer* peer) {
   request_id_ = dispatcher_->AddPendingRequest(peer_, request_.resource_type,
                                                request_.mixed_content);
 
-  IPC::Message::Sender* sender = dispatcher_->message_sender();
-  bool ret = false;
-  if (sender)
-    ret = sender->Send(new ViewHostMsg_RequestResource(MSG_ROUTING_NONE,
-                                                       request_id_,
-                                                       request_));
-  return ret;
+  return dispatcher_->message_sender()->Send(
+      new ViewHostMsg_RequestResource(route_id_, request_id_, request_));
 }
 
 void IPCResourceLoaderBridge::Cancel() {
@@ -181,9 +183,8 @@ void IPCResourceLoaderBridge::Cancel() {
 
   RESOURCE_LOG("Canceling request for " << url_);
 
-  IPC::Message::Sender* sender = dispatcher_->message_sender();
-  if (sender)
-    sender->Send(new ViewHostMsg_CancelRequest(MSG_ROUTING_NONE, request_id_));
+  dispatcher_->message_sender()->Send(
+      new ViewHostMsg_CancelRequest(route_id_, request_id_));
 
   // We can't remove the request ID from the resource dispatcher because more
   // data might be pending. Sending the cancel message may cause more data
@@ -211,15 +212,11 @@ void IPCResourceLoaderBridge::SyncLoad(SyncLoadResponse* response) {
   request_id_ = MakeRequestID();
 
   SyncLoadResult result;
-  IPC::Message::Sender* sender = dispatcher_->message_sender();
-
-  if (sender) {
-    IPC::Message* msg = new ViewHostMsg_SyncLoad(MSG_ROUTING_NONE, request_id_,
-                                                 request_, &result);
-    if (!sender->Send(msg)) {
-      response->status.set_status(URLRequestStatus::FAILED);
-      return;
-    }
+  IPC::Message* msg = new ViewHostMsg_SyncLoad(route_id_, request_id_,
+                                               request_, &result);
+  if (!dispatcher_->message_sender()->Send(msg)) {
+    response->status.set_status(URLRequestStatus::FAILED);
+    return;
   }
 
   response->status = result.status;
@@ -279,7 +276,7 @@ bool ResourceDispatcher::OnMessageReceived(const IPC::Message& message) {
 }
 
 void ResourceDispatcher::OnUploadProgress(
-    int request_id, int64 position, int64 size) {
+    const IPC::Message& message, int request_id, int64 position, int64 size) {
   PendingRequestList::iterator it = pending_requests_.find(request_id);
   if (it == pending_requests_.end()) {
     // this might happen for kill()ed requests on the webkit end, so perhaps
@@ -296,10 +293,8 @@ void ResourceDispatcher::OnUploadProgress(
   request_info.peer->OnUploadProgress(position, size);
 
   // Acknowlegde reciept
-  IPC::Message::Sender* sender = message_sender();
-  if (sender)
-    sender->Send(
-        new ViewHostMsg_UploadProgress_ACK(MSG_ROUTING_NONE, request_id));
+  message_sender()->Send(
+      new ViewHostMsg_UploadProgress_ACK(message.routing_id(), request_id));
 }
 
 void ResourceDispatcher::OnReceivedResponse(
@@ -333,14 +328,13 @@ void ResourceDispatcher::OnReceivedResponse(
   peer->OnReceivedResponse(response_head, false);
 }
 
-void ResourceDispatcher::OnReceivedData(int request_id,
+void ResourceDispatcher::OnReceivedData(const IPC::Message& message,
+                                        int request_id,
                                         base::SharedMemoryHandle shm_handle,
                                         int data_len) {
   // Acknowlegde the reception of this data.
-  IPC::Message::Sender* sender = message_sender();
-  if (sender)
-    sender->Send(
-        new ViewHostMsg_DataReceived_ACK(MSG_ROUTING_NONE, request_id));
+  message_sender()->Send(
+      new ViewHostMsg_DataReceived_ACK(message.routing_id(), request_id));
 
   const bool shm_valid = base::SharedMemory::IsHandleValid(shm_handle);
   DCHECK((shm_valid && data_len > 0) || (!shm_valid && !data_len));
@@ -495,12 +489,14 @@ webkit_glue::ResourceLoaderBridge* ResourceDispatcher::CreateBridge(
     int origin_pid,
     ResourceType::Type resource_type,
     bool mixed_content,
-    uint32 request_context) {
+    uint32 request_context,
+    int route_id) {
   return new webkit_glue::IPCResourceLoaderBridge(this, method, url, policy_url,
                                                   referrer, headers, flags,
                                                   origin_pid, resource_type,
                                                   mixed_content,
-                                                  request_context);
+                                                  request_context,
+                                                  route_id);
 }
 
 
diff --git a/chrome/common/resource_dispatcher.h b/chrome/common/resource_dispatcher.h
index b4e7fb1..3cdf1ee 100644
--- a/chrome/common/resource_dispatcher.h
+++ b/chrome/common/resource_dispatcher.h
@@ -20,15 +20,10 @@
 
 struct ResourceResponseHead;
 
-// Uncomment this to disable loading resources via the parent process.  This
-// may be useful for debugging purposes.
-//#define USING_SIMPLE_RESOURCE_LOADER_BRIDGE
-
 // This class serves as a communication interface between the
 // ResourceDispatcherHost in the browser process and the ResourceLoaderBridge in
-// the child process.  It can be used from either the renderer or plugin
-// processes.
-class ResourceDispatcher : public base::RefCounted<ResourceDispatcher> {
+// the child process.  It can be used from any child process.
+class ResourceDispatcher {
  public:
   explicit ResourceDispatcher(IPC::Message::Sender* sender);
   ~ResourceDispatcher();
@@ -49,7 +44,8 @@ class ResourceDispatcher : public base::RefCounted<ResourceDispatcher> {
     int origin_pid,
     ResourceType::Type resource_type,
     bool mixed_content,
-    uint32 request_context /* used for plugin->browser requests */);
+    uint32 request_context /* used for plugin->browser requests */,
+    int route_id);
 
   // Adds a request from the pending_requests_ list, returning the new
   // requests' ID
@@ -68,11 +64,6 @@ class ResourceDispatcher : public base::RefCounted<ResourceDispatcher> {
   // Toggles the is_deferred attribute for the specified request.
   void SetDefersLoading(int request_id, bool value);
 
-  // We can no longer use message sender
-  void ClearMessageSender() {
-    message_sender_ = NULL;
-  }
-
   // Returns true if the message passed in is a resource related
   // message.
   bool IsResourceMessage(const IPC::Message& message) const;
@@ -103,10 +94,15 @@ class ResourceDispatcher : public base::RefCounted<ResourceDispatcher> {
   typedef base::hash_map<int, PendingRequestInfo> PendingRequestList;
 
   // Message response handlers, called by the message handler for this process.
-  void OnUploadProgress(int request_id, int64 position, int64 size);
+  void OnUploadProgress(const IPC::Message& message,
+                        int request_id,
+                        int64 position,
+                        int64 size);
   void OnReceivedResponse(int request_id, const ResourceResponseHead&);
   void OnReceivedRedirect(int request_id, const GURL& new_url);
-  void OnReceivedData(int request_id, base::SharedMemoryHandle data,
+  void OnReceivedData(const IPC::Message& message,
+                      int request_id,
+                      base::SharedMemoryHandle data,
                       int data_len);
   void OnRequestComplete(int request_id,
                          const URLRequestStatus& status,
diff --git a/chrome/common/resource_dispatcher_unittest.cc b/chrome/common/resource_dispatcher_unittest.cc
index a3eea69..a53f0c3 100644
--- a/chrome/common/resource_dispatcher_unittest.cc
+++ b/chrome/common/resource_dispatcher_unittest.cc
@@ -6,7 +6,6 @@
 #include <vector>
 
 #include "base/process.h"
-#include "base/ref_counted.h"
 #include "chrome/common/filter_policy.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/resource_dispatcher.h"
@@ -86,13 +85,10 @@ class ResourceDispatcherTest : public testing::Test,
   // returning the hardcoded file contents.
   void ProcessMessages() {
     while (!message_queue_.empty()) {
-      void* iter = NULL;
-
       int request_id;
-      ASSERT_TRUE(IPC::ReadParam(&message_queue_[0], &iter, &request_id));
-
       ViewHostMsg_Resource_Request request;
-      ASSERT_TRUE(IPC::ReadParam(&message_queue_[0], &iter, &request));
+      ASSERT_TRUE(ViewHostMsg_RequestResource::Read(
+          &message_queue_[0], &request_id, &request));
 
       // check values
       EXPECT_EQ(test_page_url, request.url.spec());
@@ -117,15 +113,15 @@ class ResourceDispatcherTest : public testing::Test,
       base::SharedMemoryHandle dup_handle;
       EXPECT_TRUE(shared_mem.GiveToProcess(
           base::Process::Current().handle(), &dup_handle));
-      dispatcher_->OnReceivedData(request_id, dup_handle,
-                                  test_page_contents_len);
+      dispatcher_->OnReceivedData(
+          message_queue_[0], request_id, dup_handle, test_page_contents_len);
 
       message_queue_.erase(message_queue_.begin());
 
       // read the ack message.
-      iter = NULL;
       int request_ack = -1;
-      ASSERT_TRUE(IPC::ReadParam(&message_queue_[0], &iter, &request_ack));
+      ASSERT_TRUE(ViewHostMsg_DataReceived_ACK::Read(
+          &message_queue_[0], &request_ack));
 
       ASSERT_EQ(request_ack, request_id);
 
@@ -135,23 +131,23 @@ class ResourceDispatcherTest : public testing::Test,
 
  protected:
   static ResourceDispatcher* GetResourceDispatcher(WebFrame* unused) {
-    return dispatcher_;
+    return dispatcher_.get();
   }
 
   // testing::Test
   virtual void SetUp() {
-    dispatcher_ = new ResourceDispatcher(this);
+    dispatcher_.reset(new ResourceDispatcher(this));
   }
   virtual void TearDown() {
-    dispatcher_ = NULL;
+    dispatcher_.reset();
   }
 
   std::vector<IPC::Message> message_queue_;
-  static scoped_refptr<ResourceDispatcher> dispatcher_;
+  static scoped_ptr<ResourceDispatcher> dispatcher_;
 };
 
 /*static*/
-scoped_refptr<ResourceDispatcher> ResourceDispatcherTest::dispatcher_;
+scoped_ptr<ResourceDispatcher> ResourceDispatcherTest::dispatcher_;
 
 // Does a simple request and tests that the correct data is received.
 TEST_F(ResourceDispatcherTest, RoundTrip) {
@@ -159,7 +155,8 @@ TEST_F(ResourceDispatcherTest, RoundTrip) {
   ResourceLoaderBridge* bridge =
     dispatcher_->CreateBridge("GET", GURL(test_page_url), GURL(test_page_url),
                               GURL(), std::string(), 0, 0,
-                              ResourceType::SUB_RESOURCE, false, 0);
+                              ResourceType::SUB_RESOURCE, false, 0,
+                              MSG_ROUTING_CONTROL);
 
   bridge->Start(&callback);
 
diff --git a/chrome/plugin/chrome_plugin_host.cc b/chrome/plugin/chrome_plugin_host.cc
index bc5357a..0401987 100644
--- a/chrome/plugin/chrome_plugin_host.cc
+++ b/chrome/plugin/chrome_plugin_host.cc
@@ -149,7 +149,8 @@ class PluginRequestHandlerProxy
             GetCurrentProcessId(),
             ResourceType::OBJECT,
             false,  // TODO (jcampan): mixed-content?
-            cprequest_->context));
+            cprequest_->context,
+            MSG_ROUTING_CONTROL));
     if (!bridge_.get())
       return CPERR_FAILURE;
 
diff --git a/chrome/plugin/plugin_thread.cc b/chrome/plugin/plugin_thread.cc
index 1590250..4524335 100644
--- a/chrome/plugin/plugin_thread.cc
+++ b/chrome/plugin/plugin_thread.cc
@@ -38,10 +38,6 @@ PluginThread* PluginThread::current() {
 }
 
 void PluginThread::OnControlMessageReceived(const IPC::Message& msg) {
-  // Resource responses are sent to the resource dispatcher.
-  if (resource_dispatcher_->OnMessageReceived(msg))
-    return;
-
   IPC_BEGIN_MESSAGE_MAP(PluginThread, msg)
     IPC_MESSAGE_HANDLER(PluginProcessMsg_CreateChannel, OnCreateChannel)
     IPC_MESSAGE_HANDLER(PluginProcessMsg_ShutdownResponse, OnShutdownResponse)
@@ -55,7 +51,6 @@ void PluginThread::Init() {
   PatchNPNFunctions();
   CoInitialize(NULL);
   notification_service_.reset(new NotificationService);
-  resource_dispatcher_ = new ResourceDispatcher(this);
 
   // Preload the library to avoid loading, unloading then reloading
   preloaded_plugin_module_ = NPAPI::PluginLib::LoadNativeLibrary(plugin_path_);
@@ -83,7 +78,6 @@ void PluginThread::CleanUp() {
   NPAPI::PluginLib::UnloadAllPlugins();
   ChromePluginLib::UnloadAllPlugins();
   notification_service_.reset();
-  resource_dispatcher_ = NULL;
   CoUninitialize();
 
   if (webkit_glue::ShouldForcefullyTerminatePluginProcess())
diff --git a/chrome/plugin/plugin_thread.h b/chrome/plugin/plugin_thread.h
index 1be1c7a..bcf926f 100644
--- a/chrome/plugin/plugin_thread.h
+++ b/chrome/plugin/plugin_thread.h
@@ -7,7 +7,6 @@
 
 #include "base/file_path.h"
 #include "chrome/common/child_thread.h"
-#include "chrome/common/resource_dispatcher.h"
 #include "chrome/plugin/plugin_channel.h"
 
 class NotificationService;
@@ -23,11 +22,6 @@ class PluginThread : public ChildThread {
   // Returns the one plugin thread.
   static PluginThread* current();
 
-  // Returns the one true dispatcher.
-  ResourceDispatcher* resource_dispatcher() {
-    return resource_dispatcher_.get();
-  }
-
  private:
   virtual void OnControlMessageReceived(const IPC::Message& msg);
 
@@ -42,10 +36,6 @@ class PluginThread : public ChildThread {
 
   scoped_ptr<NotificationService> notification_service_;
 
-  // Handles resource loads for this view.
-  // NOTE: this object lives on the owner thread.
-  scoped_refptr<ResourceDispatcher> resource_dispatcher_;
-
   // The plugin module which is preloaded in Init
   HMODULE preloaded_plugin_module_;
 
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index e098199..eb0ccf7 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -191,7 +191,6 @@ RenderView::RenderView(RenderThreadBase* render_thread)
       form_field_autofill_request_id_(0),
       popup_notification_visible_(false),
       delay_seconds_for_form_state_sync_(kDefaultDelaySecondsForFormStateSync) {
-  resource_dispatcher_ = new ResourceDispatcher(this);
 #ifdef CHROME_PERSONALIZATION
   personalization_ = Personalization::CreateRendererPersonalization();
 #endif
@@ -201,7 +200,6 @@ RenderView::~RenderView() {
   if (decrement_shared_popup_at_destruction_)
     shared_popup_counter_->data--;
 
-  resource_dispatcher_->ClearMessageSender();
   // Clear any back-pointers that might still be held by plugins.
   PluginDelegateList::iterator it = plugin_delegates_.begin();
   while (it != plugin_delegates_.end()) {
@@ -342,10 +340,6 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
       main_frame ? main_frame->GetURL() : GURL());
 #endif
 
-  // Let the resource dispatcher intercept resource messages first.
-  if (resource_dispatcher_->OnMessageReceived(message))
-    return;
-
   // If this is developer tools renderer intercept tools messages first.
   if (dev_tools_client_.get() && dev_tools_client_->OnMessageReceived(message))
     return;
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 9a0ec22..29c262d7 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -16,7 +16,6 @@
 #include "base/timer.h"
 #include "base/values.h"
 #include "build/build_config.h"
-#include "chrome/common/resource_dispatcher.h"
 #ifdef CHROME_PERSONALIZATION
 #include "chrome/personalization/personalization.h"
 #endif
@@ -117,11 +116,6 @@ class RenderView : public RenderWidget,
   // Sets the "next page id" counter.
   static void SetNextPageID(int32 next_page_id);
 
-  // The resource dispatcher used to fetch resources for this view.
-  ResourceDispatcher* resource_dispatcher() {
-    return resource_dispatcher_;
-  }
-
   // May return NULL when the view is closing.
   WebView* webview() const {
     return static_cast<WebView*>(webwidget());
@@ -619,9 +613,6 @@ class RenderView : public RenderWidget,
   // A helper method used by WasOpenedByUserGesture.
   bool WasOpenedByUserGestureHelper() const;
 
-  // Handles resource loads for this view.
-  scoped_refptr<ResourceDispatcher> resource_dispatcher_;
-
   // Bitwise-ORed set of extra bindings that have been enabled.  See
   // BindingsPolicy for details.
   int enabled_bindings_;
diff --git a/chrome/renderer/renderer_glue.cc b/chrome/renderer/renderer_glue.cc
index 6cace23..117f625 100644
--- a/chrome/renderer/renderer_glue.cc
+++ b/chrome/renderer/renderer_glue.cc
@@ -236,18 +236,6 @@ webkit_glue::ScreenInfo GetScreenInfo(gfx::NativeViewId window) {
   return results;
 }
 
-#ifndef USING_SIMPLE_RESOURCE_LOADER_BRIDGE
-
-// Each RenderView has a ResourceDispatcher.  In unit tests, this function may
-// not work properly since there may be a ResourceDispatcher w/o a RenderView.
-// The WebView's delegate may be null, which typically happens as a WebView is
-// being closed (but it is also possible that it could be null at other times
-// since WebView has a SetDelegate method).
-static ResourceDispatcher* GetResourceDispatcher(WebFrame* frame) {
-  WebViewDelegate* d = frame->GetView()->GetDelegate();
-  return d ? static_cast<RenderView*>(d)->resource_dispatcher() : NULL;
-}
-
 // static factory function
 ResourceLoaderBridge* ResourceLoaderBridge::Create(
     WebFrame* webframe,
@@ -268,14 +256,13 @@ ResourceLoaderBridge* ResourceLoaderBridge::Create(
     NOTREACHED() << "no webframe";
     return NULL;
   }
-  ResourceDispatcher* dispatcher = GetResourceDispatcher(webframe);
-  if (!dispatcher) {
-    DLOG(WARNING) << "no resource dispatcher";
-    return NULL;
-  }
-  return dispatcher->CreateBridge(method, url, policy_url, referrer, headers,
-                                  load_flags, origin_pid, resource_type,
-                                  mixed_content, 0);
+
+  RenderView* rv = static_cast<RenderView*>(webframe->GetView()->GetDelegate());
+  int route_id = rv->routing_id();
+  ResourceDispatcher* dispatch = RenderThread::current()->resource_dispatcher();
+  return dispatch->CreateBridge(method, url, policy_url, referrer, headers,
+                                load_flags, origin_pid, resource_type,
+                                mixed_content, 0, route_id);
 }
 
 void NotifyCacheStats() {
@@ -286,6 +273,5 @@ void NotifyCacheStats() {
     RenderThread::current()->InformHostOfCacheStatsLater();
 }
 
-#endif  // !USING_SIMPLE_RESOURCE_LOADER_BRIDGE
 
 }  // namespace webkit_glue