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

5 files changed, 72 insertions, 67 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);