Make BrowserMessageFilter support dispatching messages on different threads.

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

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

9 files changed, 171 insertions, 104 deletions

diff --git a/chrome/browser/appcache/appcache_dispatcher_host.cc b/chrome/browser/appcache/appcache_dispatcher_host.cc
index 720b71b..ecea93c 100644
--- a/chrome/browser/appcache/appcache_dispatcher_host.cc
+++ b/chrome/browser/appcache/appcache_dispatcher_host.cc
@@ -31,7 +31,7 @@ AppCacheDispatcherHost::AppCacheDispatcherHost(
 AppCacheDispatcherHost::~AppCacheDispatcherHost() {}
 
 void AppCacheDispatcherHost::OnChannelConnected(int32 peer_pid) {
-  BrowserIOMessageFilter::OnChannelConnected(peer_pid);
+  BrowserMessageFilter::OnChannelConnected(peer_pid);
 
   DCHECK(request_context_.get() || request_context_getter_.get());
 
@@ -56,11 +56,11 @@ void AppCacheDispatcherHost::OnChannelConnected(int32 peer_pid) {
   }
 }
 
-bool AppCacheDispatcherHost::OnMessageReceived(const IPC::Message& message) {
+bool AppCacheDispatcherHost::OnMessageReceived(const IPC::Message& message,
+                                               bool* message_was_ok) {
   bool handled = true;
-  bool message_was_ok = true;
 
-  IPC_BEGIN_MESSAGE_MAP_EX(AppCacheDispatcherHost, message, message_was_ok)
+  IPC_BEGIN_MESSAGE_MAP_EX(AppCacheDispatcherHost, message, *message_was_ok)
     IPC_MESSAGE_HANDLER(AppCacheMsg_RegisterHost, OnRegisterHost);
     IPC_MESSAGE_HANDLER(AppCacheMsg_UnregisterHost, OnUnregisterHost);
     IPC_MESSAGE_HANDLER(AppCacheMsg_GetResourceList, OnGetResourceList);
@@ -76,9 +76,6 @@ bool AppCacheDispatcherHost::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP_EX()
 
-  if (!message_was_ok)
-    BadMessageReceived(message.type());
-
   return handled;
 }
 
diff --git a/chrome/browser/appcache/appcache_dispatcher_host.h b/chrome/browser/appcache/appcache_dispatcher_host.h
index a9e6d20..425fdd2 100644
--- a/chrome/browser/appcache/appcache_dispatcher_host.h
+++ b/chrome/browser/appcache/appcache_dispatcher_host.h
@@ -12,7 +12,7 @@
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/appcache/appcache_frontend_proxy.h"
-#include "chrome/browser/browser_io_message_filter.h"
+#include "chrome/browser/browser_message_filter.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "ipc/ipc_message.h"
 #include "webkit/appcache/appcache_backend_impl.h"
@@ -25,7 +25,7 @@ class URLRequestContextGetter;
 // its child processes. There is a distinct host for each child process.
 // Messages are handled on the IO thread. The ResourceMessageFilter and
 // WorkerProcessHost create an instance and delegates calls to it.
-class AppCacheDispatcherHost : public BrowserIOMessageFilter {
+class AppCacheDispatcherHost : public BrowserMessageFilter {
  public:
   // Constructor for use on the IO thread.
   AppCacheDispatcherHost(URLRequestContext* request_context,
@@ -38,8 +38,9 @@ class AppCacheDispatcherHost : public BrowserIOMessageFilter {
   ~AppCacheDispatcherHost();
 
   // BrowserIOMessageFilter implementation
-  virtual bool OnMessageReceived(const IPC::Message& message);
   virtual void OnChannelConnected(int32 peer_pid);
+  virtual bool OnMessageReceived(const IPC::Message& message,
+                                 bool* message_was_ok);
 
  private:
   // IPC message handlers
diff --git a/chrome/browser/browser_io_message_filter.cc b/chrome/browser/browser_io_message_filter.cc
deleted file mode 100644
index bd1fd500d..0000000
--- a/chrome/browser/browser_io_message_filter.cc
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright (c) 2010 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/browser_io_message_filter.h"
-
-#include "base/logging.h"
-#include "base/process_util.h"
-#include "chrome/browser/renderer_host/browser_render_process_host.h"
-
-BrowserIOMessageFilter::BrowserIOMessageFilter() : channel_(NULL) {
-}
-
-BrowserIOMessageFilter::~BrowserIOMessageFilter() {
-}
-
-void BrowserIOMessageFilter::OnFilterAdded(IPC::Channel* channel) {
-  channel_ = channel;
-}
-
-void BrowserIOMessageFilter::OnChannelClosing() {
-  channel_ = NULL;
-}
-
-void BrowserIOMessageFilter::OnChannelConnected(int32 peer_pid) {
-  if (!base::OpenProcessHandle(peer_pid, &peer_handle_)) {
-    NOTREACHED();
-  }
-}
-
-bool BrowserIOMessageFilter::Send(IPC::Message* msg) {
-  if (channel_)
-    return channel_->Send(msg);
-
-  delete msg;
-  return false;
-}
-
-void BrowserIOMessageFilter::BadMessageReceived(uint32 msg_type) {
-  BrowserRenderProcessHost::BadMessageTerminateProcess(msg_type, peer_handle());
-}
diff --git a/chrome/browser/browser_io_message_filter.h b/chrome/browser/browser_io_message_filter.h
deleted file mode 100644
index 2f19850..0000000
--- a/chrome/browser/browser_io_message_filter.h
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef CHROME_BROWSER_BROWSER_IO_MESSAGE_FILTER_H_
-#define CHROME_BROWSER_BROWSER_IO_MESSAGE_FILTER_H_
-#pragma once
-
-#include "base/process.h"
-#include "ipc/ipc_channel_proxy.h"
-
-// Base class for message filters in the browser process that reside on the IO
-// thread.
-class BrowserIOMessageFilter : public IPC::ChannelProxy::MessageFilter,
-                               public IPC::Message::Sender {
- public:
-  BrowserIOMessageFilter();
-  virtual ~BrowserIOMessageFilter();
-
-  // IPC::ChannelProxy::MessageFilter methods.  If you override them, make sure
-  // to call them as well.
-  virtual void OnFilterAdded(IPC::Channel* channel);
-  virtual void OnChannelClosing();
-  virtual void OnChannelConnected(int32 peer_pid);
-
-  // IPC::Message::Sender implementation:
-  virtual bool Send(IPC::Message* msg);
-
- protected:
-  base::ProcessHandle peer_handle() { return peer_handle_; }
-
-  // Call this if a message couldn't be deserialized.  This kills the renderer.
-  void BadMessageReceived(uint32 msg_type);
-
- private:
-  IPC::Channel* channel_;
-  base::ProcessHandle peer_handle_;
-};
-
-#endif  // CHROME_BROWSER_BROWSER_IO_MESSAGE_FILTER_H_
diff --git a/chrome/browser/browser_message_filter.cc b/chrome/browser/browser_message_filter.cc
new file mode 100644
index 0000000..c4ee1d6
--- /dev/null
+++ b/chrome/browser/browser_message_filter.cc
@@ -0,0 +1,88 @@
+// Copyright (c) 2010 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/browser_message_filter.h"
+
+#include "base/logging.h"
+#include "base/process.h"
+#include "base/process_util.h"
+#include "chrome/browser/renderer_host/browser_render_process_host.h"
+
+BrowserMessageFilter::BrowserMessageFilter()
+    : channel_(NULL), peer_handle_(base::kNullProcessHandle) {
+}
+
+BrowserMessageFilter::~BrowserMessageFilter() {
+}
+
+void BrowserMessageFilter::OnFilterAdded(IPC::Channel* channel) {
+  channel_ = channel;
+}
+
+void BrowserMessageFilter::OnChannelClosing() {
+  channel_ = NULL;
+}
+
+void BrowserMessageFilter::OnChannelConnected(int32 peer_pid) {
+  if (!base::OpenProcessHandle(peer_pid, &peer_handle_)) {
+    NOTREACHED();
+  }
+}
+
+bool BrowserMessageFilter::Send(IPC::Message* message) {
+  if (message->is_sync()) {
+    // We don't support sending synchronous messages from the browser.  If we
+    // really needed it, we can make this class derive from SyncMessageFilter
+    // but it seems better to not allow sending synchronous messages from the
+    // browser, since it might allow a corrupt/malicious renderer to hang us.
+    NOTREACHED() << "Can't send sync message through BrowserMessageFilter!";
+    return false;
+  }
+
+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        NewRunnableMethod(this, &BrowserMessageFilter::Send, message));
+    return true;
+  }
+
+  if (channel_)
+    return channel_->Send(message);
+
+  delete message;
+  return false;
+}
+
+void BrowserMessageFilter::OverrideThreadForMessage(const IPC::Message& message,
+                                                    BrowserThread::ID* thread) {
+}
+
+bool BrowserMessageFilter::OnMessageReceived(const IPC::Message& message) {
+  BrowserThread::ID thread = BrowserThread::IO;
+  OverrideThreadForMessage(message, &thread);
+  if (thread == BrowserThread::IO)
+    return DispatchMessage(message);
+
+  BrowserThread::PostTask(
+      thread, FROM_HERE,
+      NewRunnableMethod(
+          this, &BrowserMessageFilter::DispatchMessage, message));
+  return true;
+}
+
+bool BrowserMessageFilter::DispatchMessage(const IPC::Message& message) {
+  bool message_was_ok = true;
+  bool rv = OnMessageReceived(message, &message_was_ok);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) || rv) <<
+      "Must handle messages that were dispatched to another thread!";
+  if (!message_was_ok)
+    BadMessageReceived(message.type());
+
+  return rv;
+}
+
+void BrowserMessageFilter::BadMessageReceived(uint32 msg_type) {
+  BrowserRenderProcessHost::BadMessageTerminateProcess(msg_type, peer_handle());
+}
diff --git a/chrome/browser/browser_message_filter.h b/chrome/browser/browser_message_filter.h
new file mode 100644
index 0000000..c5b12ba
--- /dev/null
+++ b/chrome/browser/browser_message_filter.h
@@ -0,0 +1,64 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_BROWSER_MESSAGE_FILTER_H_
+#define CHROME_BROWSER_BROWSER_MESSAGE_FILTER_H_
+#pragma once
+
+#include "base/process.h"
+#include "chrome/browser/browser_thread.h"
+#include "ipc/ipc_channel_proxy.h"
+
+// Base class for message filters in the browser process.  You can receive and
+// send messages on any thread.
+class BrowserMessageFilter : public IPC::ChannelProxy::MessageFilter,
+                             public IPC::Message::Sender {
+ public:
+  BrowserMessageFilter();
+  virtual ~BrowserMessageFilter();
+
+  // IPC::ChannelProxy::MessageFilter methods.  If you override them, make sure
+  // to call them as well.  These are always called on the IO thread.
+  virtual void OnFilterAdded(IPC::Channel* channel);
+  virtual void OnChannelClosing();
+  virtual void OnChannelConnected(int32 peer_pid);
+  // DON'T OVERRIDE THIS!  Override the other version below.
+  virtual bool OnMessageReceived(const IPC::Message& message);
+
+  // IPC::Message::Sender implementation.  Can be called on any thread.  Can't
+  // send sync messages (since we don't want to block the browser on any other
+  // process).
+  virtual bool Send(IPC::Message* message);
+
+  // If you want the given message to be dispatched to your OnMessageReceived on
+  // a different thread, change |thread| to the id of the target thread.
+  // If you don't handle this message, or want to keep it on the IO thread, do
+  // nothing.
+  virtual void OverrideThreadForMessage(const IPC::Message& message,
+                                        BrowserThread::ID* thread);
+
+  // Override this to receive messages.
+  // Your function will normally be called on the IO thread.  However, if your
+  // OverrideThreadForMessage modifies the thread used to dispatch the message,
+  // your function will be called on the requested thread.
+  virtual bool OnMessageReceived(const IPC::Message& message,
+                                 bool* message_was_ok) = 0;
+
+ protected:
+  // Can be called on any thread, after OnChannelConnected is called.
+  base::ProcessHandle peer_handle() { return peer_handle_; }
+
+  // Call this if a message couldn't be deserialized.  This kills the renderer.
+  // Can be called on any thread.
+  void BadMessageReceived(uint32 msg_type);
+
+ private:
+  // Dispatches a message to the derived class.
+  bool DispatchMessage(const IPC::Message& message);
+
+  IPC::Channel* channel_;
+  base::ProcessHandle peer_handle_;
+};
+
+#endif  // CHROME_BROWSER_BROWSER_MESSAGE_FILTER_H_
diff --git a/chrome/browser/renderer_host/audio_renderer_host.cc b/chrome/browser/renderer_host/audio_renderer_host.cc
index fc8c84b..1b5ce0a 100644
--- a/chrome/browser/renderer_host/audio_renderer_host.cc
+++ b/chrome/browser/renderer_host/audio_renderer_host.cc
@@ -56,7 +56,7 @@ AudioRendererHost::~AudioRendererHost() {
 }
 
 void AudioRendererHost::OnChannelClosing() {
-  BrowserIOMessageFilter::OnChannelClosing();
+  BrowserMessageFilter::OnChannelClosing();
 
   // Since the IPC channel is gone, close all requested audio streams.
   DeleteEntries();
@@ -233,10 +233,10 @@ void AudioRendererHost::DoHandleError(media::AudioOutputController* controller,
 
 ///////////////////////////////////////////////////////////////////////////////
 // IPC Messages handler
-bool AudioRendererHost::OnMessageReceived(const IPC::Message& message) {
+bool AudioRendererHost::OnMessageReceived(const IPC::Message& message,
+                                          bool* message_was_ok) {
   bool handled = true;
-  bool message_was_ok = true;
-  IPC_BEGIN_MESSAGE_MAP_EX(AudioRendererHost, message, message_was_ok)
+  IPC_BEGIN_MESSAGE_MAP_EX(AudioRendererHost, message, *message_was_ok)
     IPC_MESSAGE_HANDLER(ViewHostMsg_CreateAudioStream, OnCreateStream)
     IPC_MESSAGE_HANDLER(ViewHostMsg_PlayAudioStream, OnPlayStream)
     IPC_MESSAGE_HANDLER(ViewHostMsg_PauseAudioStream, OnPauseStream)
@@ -248,9 +248,6 @@ bool AudioRendererHost::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP_EX()
 
-  if (!message_was_ok)
-    BadMessageReceived(message.type());
-
   return handled;
 }
 
diff --git a/chrome/browser/renderer_host/audio_renderer_host.h b/chrome/browser/renderer_host/audio_renderer_host.h
index 3248205..b91be77 100644
--- a/chrome/browser/renderer_host/audio_renderer_host.h
+++ b/chrome/browser/renderer_host/audio_renderer_host.h
@@ -60,7 +60,7 @@
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/shared_memory.h"
-#include "chrome/browser/browser_io_message_filter.h"
+#include "chrome/browser/browser_message_filter.h"
 #include "chrome/browser/browser_thread.h"
 #include "ipc/ipc_message.h"
 #include "media/audio/audio_io.h"
@@ -70,7 +70,7 @@
 class AudioManager;
 struct ViewHostMsg_Audio_CreateStream_Params;
 
-class AudioRendererHost : public BrowserIOMessageFilter,
+class AudioRendererHost : public BrowserMessageFilter,
                           public media::AudioOutputController::EventHandler {
  public:
   typedef std::pair<int32, int> AudioEntryId;
@@ -107,10 +107,11 @@ class AudioRendererHost : public BrowserIOMessageFilter,
   AudioRendererHost();
 
 
-  // BrowserIOMessageFilter implementation
-  virtual bool OnMessageReceived(const IPC::Message& message);
+  // BrowserMessageFilter implementation.
   virtual void OnChannelClosing();
   virtual void OnDestruct() const;
+  virtual bool OnMessageReceived(const IPC::Message& message,
+                                 bool* message_was_ok);
 
   /////////////////////////////////////////////////////////////////////////////
   // AudioOutputController::EventHandler implementations.
diff --git a/chrome/browser/worker_host/worker_process_host.h b/chrome/browser/worker_host/worker_process_host.h
index df83a97..4c983e0 100644
--- a/chrome/browser/worker_host/worker_process_host.h
+++ b/chrome/browser/worker_host/worker_process_host.h
@@ -14,7 +14,7 @@
 #include "base/file_path.h"
 #include "base/ref_counted.h"
 #include "chrome/browser/browser_child_process_host.h"
-#include "chrome/browser/browser_io_message_filter.h"
+#include "chrome/browser/browser_message_filter.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/worker_host/worker_document_set.h"
 #include "googleurl/src/gurl.h"
@@ -223,7 +223,7 @@ class WorkerProcessHost : public BrowserChildProcessHost {
   // Holds all the IPC message filters.  Since the worker process host is on the
   // IO thread, we don't have a IPC::ChannelProxy and so we manage filters
   // manually.
-  std::vector<scoped_refptr<BrowserIOMessageFilter> > filters_;
+  std::vector<scoped_refptr<BrowserMessageFilter> > filters_;
 
   // A callback to create a routing id for the associated worker process.
   scoped_ptr<CallbackWithReturnValue<int>::Type> next_route_id_callback_;