Make BrowserMessageFilter support dispatching messages on different threads.

chrome/browser/in_process_webkit/dom_storage_message_filter.cc

Index: chrome/browser/in_process_webkit/dom_storage_message_filter.cc
===================================================================
--- chrome/browser/in_process_webkit/dom_storage_message_filter.cc	(revision 68139)
+++ chrome/browser/in_process_webkit/dom_storage_message_filter.cc	(working copy)
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h"
+#include "chrome/browser/in_process_webkit/dom_storage_message_filter.h"
 
 #include "base/nullable_string16.h"
 #include "chrome/browser/browser_thread.h"
@@ -10,79 +10,68 @@
 #include "chrome/browser/in_process_webkit/dom_storage_context.h"
 #include "chrome/browser/in_process_webkit/dom_storage_namespace.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_host/browser_render_process_host.h"
 #include "chrome/browser/renderer_host/render_view_host_notification_task.h"
-#include "chrome/browser/renderer_host/resource_message_filter.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/render_messages_params.h"
+#include "chrome/common/url_constants.h"
 #include "googleurl/src/gurl.h"
 
 using WebKit::WebStorageArea;
 
-DOMStorageDispatcherHost* DOMStorageDispatcherHost::storage_event_host_ = NULL;
-const GURL* DOMStorageDispatcherHost::storage_event_url_ = NULL;
+DOMStorageMessageFilter* DOMStorageMessageFilter::storage_event_message_filter =
+    NULL;
+const GURL* DOMStorageMessageFilter::storage_event_url_ = NULL;
 
-DOMStorageDispatcherHost::
+DOMStorageMessageFilter::
 ScopedStorageEventContext::ScopedStorageEventContext(
-    DOMStorageDispatcherHost* dispatcher_host, const GURL* url) {
+    DOMStorageMessageFilter* dispatcher_message_filter, const GURL* url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
-  DCHECK(!storage_event_host_);
+  DCHECK(!storage_event_message_filter);
   DCHECK(!storage_event_url_);
-  storage_event_host_ = dispatcher_host;
+  storage_event_message_filter = dispatcher_message_filter;
   storage_event_url_ = url;
-  DCHECK(storage_event_host_);
+  DCHECK(storage_event_message_filter);
   DCHECK(storage_event_url_);
 }
 
-DOMStorageDispatcherHost::
+DOMStorageMessageFilter::
 ScopedStorageEventContext::~ScopedStorageEventContext() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
-  DCHECK(storage_event_host_);
+  DCHECK(storage_event_message_filter);
   DCHECK(storage_event_url_);
-  storage_event_host_ = NULL;
+  storage_event_message_filter = NULL;
   storage_event_url_ = NULL;
 }
 
-DOMStorageDispatcherHost::DOMStorageDispatcherHost(
-    ResourceMessageFilter* resource_message_filter,
-    WebKitContext* webkit_context)
-    : webkit_context_(webkit_context),
-      resource_message_filter_(resource_message_filter),
-      process_handle_(0),
-      process_id_(0) {
-  DCHECK(webkit_context_.get());
-  DCHECK(resource_message_filter_);
+DOMStorageMessageFilter::DOMStorageMessageFilter(int process_id,
+                                                 Profile* profile)
+    : webkit_context_(profile->GetWebKitContext()),
+      process_id_(process_id),
+      request_context_(profile->GetRequestContext()),
+      extensions_request_context_(profile->GetRequestContextForExtensions()) {
 }
 
-DOMStorageDispatcherHost::~DOMStorageDispatcherHost() {
+DOMStorageMessageFilter::~DOMStorageMessageFilter() {
+  // This is not always true during testing.
+  if (peer_handle())
+    Context()->UnregisterMessageFilter(this);
 }
 
-void DOMStorageDispatcherHost::Init(int process_id,
-                                    base::ProcessHandle process_handle) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(resource_message_filter_);  // Ensure Shutdown() has not been called.
-  DCHECK(!process_handle_);  // Make sure Init() has not yet been called.
-  DCHECK(process_handle);
-  Context()->RegisterDispatcherHost(this);
-  process_id_ = process_id;
-  process_handle_ = process_handle;
-}
+void DOMStorageMessageFilter::OnChannelConnected(int32 peer_pid) {
+  BrowserMessageFilter::OnChannelConnected(peer_pid);
 
-void DOMStorageDispatcherHost::Shutdown() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // This is not always true during testing.
-  if (process_handle_)
-    Context()->UnregisterDispatcherHost(this);
-  resource_message_filter_ = NULL;
+  Context()->RegisterMessageFilter(this);
 }
 
 /* static */
-void DOMStorageDispatcherHost::DispatchStorageEvent(const NullableString16& key,
+void DOMStorageMessageFilter::DispatchStorageEvent(const NullableString16& key,
     const NullableString16& old_value, const NullableString16& new_value,
     const string16& origin, const GURL& url, bool is_local_storage) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DCHECK(is_local_storage);  // Only LocalStorage is implemented right now.
-  DCHECK(storage_event_host_);
+  DCHECK(storage_event_message_filter);
   ViewMsg_DOMStorageEvent_Params params;
   params.key_ = key;
   params.old_value_ = old_value;
@@ -91,20 +80,17 @@
   params.url_ = *storage_event_url_;  // The url passed in is junk.
   params.storage_type_ = is_local_storage ? DOM_STORAGE_LOCAL
                                           : DOM_STORAGE_SESSION;
-  // The storage_event_host_ is the DOMStorageDispatcherHost that is up in the
-  // current call stack since it caused the storage event to fire.
+  // The storage_event_message_filter is the DOMStorageMessageFilter that is up
+  // in the current call stack since it caused the storage event to fire.
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-      NewRunnableMethod(storage_event_host_,
-          &DOMStorageDispatcherHost::OnStorageEvent, params));
+      NewRunnableMethod(storage_event_message_filter,
+          &DOMStorageMessageFilter::OnStorageEvent, params));
 }
 
-bool DOMStorageDispatcherHost::OnMessageReceived(const IPC::Message& message,
-                                                 bool* msg_is_ok) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(process_handle_);
-
+bool DOMStorageMessageFilter::OnMessageReceived(const IPC::Message& message,
+                                                bool* message_was_ok) {
   bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP_EX(DOMStorageDispatcherHost, message, *msg_is_ok)
+  IPC_BEGIN_MESSAGE_MAP_EX(DOMStorageMessageFilter, message, *message_was_ok)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageStorageAreaId,
                                     OnStorageAreaId)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageLength, OnLength)
@@ -116,56 +102,46 @@
     IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageClear, OnClear)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
+
   return handled;
 }
 
-int64 DOMStorageDispatcherHost::CloneSessionStorage(int64 original_id) {
-  return Context()->CloneSessionStorage(original_id);
-}
-
-void DOMStorageDispatcherHost::Send(IPC::Message* message) {
-  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    // TODO(jorlow): Even if we successfully post, I believe it's possible for
-    //               the task to never run (if the IO thread is already shutting
-    //               down).  We may want to handle this case, though
-    //               realistically it probably doesn't matter.
-    if (!BrowserThread::PostTask(
-            BrowserThread::IO, FROM_HERE, NewRunnableMethod(
-                this, &DOMStorageDispatcherHost::Send, message))) {
-      // The IO thread is dead.
-      delete message;
-    }
-    return;
+BrowserThread::ID DOMStorageMessageFilter::HandleMessageOnThread(
+    const IPC::Message& message) {
+  switch (message.type()) {
+    case ViewHostMsg_DOMStorageLength::ID:
+    case ViewHostMsg_DOMStorageKey::ID:
+    case ViewHostMsg_DOMStorageGetItem::ID:
+    case ViewHostMsg_DOMStorageSetItem::ID:
+    case ViewHostMsg_DOMStorageRemoveItem::ID:
+    case ViewHostMsg_DOMStorageClear::ID:
+      return BrowserThread::WEBKIT;
+    default:
+      break;
   }
-
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (!resource_message_filter_)
-    delete message;
-  else
-    resource_message_filter_->Send(message);
+  return BrowserMessageFilter::HandleMessageOnThread(message);

[darin (slow to review), 2010/12/06 05:14:58]

A better return value here might be some kind of "NONE" identifier.

It is also kind of unfortunate to have to repeat message IDs here and again in
OnMessageReceived.  This makes me want some kind of "group ID" that can be used
to select all DOMStorage* messages.

Maybe that's the answer.  Instead of using ViewHostMsg_, we should introduce
DOMStorageHostMsg_?

[jam, 2010/12/06 07:24:43]

yeah I wasn't sure how best to structure this, I also considered having a bool
rv that says whether this filter wants to handle the message or not, and if it's
true, then another output parameter would specify the thread.  I don't feel
strongly either way.
This breaks down when a filter wants to handle messages on different threads. 
For example, DatabaseDispatcherHost and DOMStorageMessageFilter (and maybe
others) want to process most of their messages on the FILE/WEBKIT threads, but
they each want to process one message on the IO thread.  That message is related
to the other messages.  If the grouping decides which thread a message is
dispatched on, then it seems awkward to have create multiple groupings for all
of a feature's messages.  Same with messages that are dispatched on different
threads depending on the platform.

 }
 
-void DOMStorageDispatcherHost::OnStorageAreaId(int64 namespace_id,
-                                               const string16& origin,
-                                               IPC::Message* reply_msg) {
+void DOMStorageMessageFilter::OnStorageAreaId(int64 namespace_id,
+                                              const string16& origin,
+                                              IPC::Message* reply_msg) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   ChromeURLRequestContext* url_request_context =
-      resource_message_filter_->GetRequestContextForURL(GURL(origin));
+      GetRequestContextForURL(GURL(origin));
   BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-      this, &DOMStorageDispatcherHost::OnStorageAreaIdWebKit, namespace_id,
+      this, &DOMStorageMessageFilter::OnStorageAreaIdWebKit, namespace_id,
       origin, reply_msg,
       make_scoped_refptr(url_request_context->host_content_settings_map())));
 }
 
-void DOMStorageDispatcherHost::OnStorageAreaIdWebKit(
+void DOMStorageMessageFilter::OnStorageAreaIdWebKit(
     int64 namespace_id, const string16& origin, IPC::Message* reply_msg,
     HostContentSettingsMap* host_content_settings_map) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DOMStorageNamespace* storage_namespace =
       Context()->GetStorageNamespace(namespace_id, true);
   if (!storage_namespace) {
-    BrowserRenderProcessHost::BadMessageTerminateProcess(
-        ViewHostMsg_DOMStorageStorageAreaId::ID, process_handle_);
+    BadMessageReceived(ViewHostMsg_DOMStorageStorageAreaId::ID);
     delete reply_msg;
     return;
   }
@@ -176,19 +152,12 @@
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnLength(int64 storage_area_id,
-                                        IPC::Message* reply_msg) {
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnLength, storage_area_id, reply_msg));
-    return;
-  }
-
+void DOMStorageMessageFilter::OnLength(int64 storage_area_id,
+                                       IPC::Message* reply_msg) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DOMStorageArea* storage_area = Context()->GetStorageArea(storage_area_id);
   if (!storage_area) {
-    BrowserRenderProcessHost::BadMessageTerminateProcess(
-        ViewHostMsg_DOMStorageLength::ID, process_handle_);
+    BadMessageReceived(ViewHostMsg_DOMStorageLength::ID);
     delete reply_msg;
     return;
   }
@@ -197,20 +166,12 @@
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnKey(int64 storage_area_id, unsigned index,
-                                     IPC::Message* reply_msg) {
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnKey, storage_area_id, index,
-        reply_msg));
-    return;
-  }
-
+void DOMStorageMessageFilter::OnKey(int64 storage_area_id, unsigned index,
+                                    IPC::Message* reply_msg) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DOMStorageArea* storage_area = Context()->GetStorageArea(storage_area_id);
   if (!storage_area) {
-    BrowserRenderProcessHost::BadMessageTerminateProcess(
-        ViewHostMsg_DOMStorageKey::ID, process_handle_);
+    BadMessageReceived(ViewHostMsg_DOMStorageKey::ID);
     delete reply_msg;
     return;
   }
@@ -219,21 +180,13 @@
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnGetItem(int64 storage_area_id,
-                                         const string16& key,
-                                         IPC::Message* reply_msg) {
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnGetItem, storage_area_id, key,
-        reply_msg));
-    return;
-  }
-
+void DOMStorageMessageFilter::OnGetItem(int64 storage_area_id,
+                                        const string16& key,
+                                        IPC::Message* reply_msg) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DOMStorageArea* storage_area = Context()->GetStorageArea(storage_area_id);
   if (!storage_area) {
-    BrowserRenderProcessHost::BadMessageTerminateProcess(
-        ViewHostMsg_DOMStorageGetItem::ID, process_handle_);
+    BadMessageReceived(ViewHostMsg_DOMStorageGetItem::ID);
     delete reply_msg;
     return;
   }
@@ -242,27 +195,20 @@
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnSetItem(
+void DOMStorageMessageFilter::OnSetItem(
     int64 storage_area_id, const string16& key, const string16& value,
     const GURL& url, IPC::Message* reply_msg) {
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnSetItem, storage_area_id, key, value,
-        url, reply_msg));
-    return;
-  }
-
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DOMStorageArea* storage_area = Context()->GetStorageArea(storage_area_id);
   if (!storage_area) {
-    BrowserRenderProcessHost::BadMessageTerminateProcess(
-        ViewHostMsg_DOMStorageSetItem::ID, process_handle_);
+    BadMessageReceived(ViewHostMsg_DOMStorageSetItem::ID);
+    delete reply_msg;
     return;
   }
 
   ScopedStorageEventContext scope(this, &url);
   WebStorageArea::Result result;
-  NullableString16 old_value = storage_area->SetItem(key, value, &result, this);
+  NullableString16 old_value = storage_area->SetItem(key, value, &result);
 
   // If content was blocked, tell the UI to display the blocked content icon.
   if (reply_msg->routing_id() == MSG_ROUTING_CONTROL) {
@@ -279,21 +225,14 @@
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnRemoveItem(
+void DOMStorageMessageFilter::OnRemoveItem(
     int64 storage_area_id, const string16& key, const GURL& url,
     IPC::Message* reply_msg) {
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key,
-        url, reply_msg));
-    return;
-  }
-
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DOMStorageArea* storage_area = Context()->GetStorageArea(storage_area_id);
   if (!storage_area) {
-    BrowserRenderProcessHost::BadMessageTerminateProcess(
-        ViewHostMsg_DOMStorageRemoveItem::ID, process_handle_);
+    BadMessageReceived(ViewHostMsg_DOMStorageRemoveItem::ID);
+    delete reply_msg;
     return;
   }
 
@@ -303,20 +242,13 @@
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnClear(int64 storage_area_id, const GURL& url,
-                                       IPC::Message* reply_msg) {
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnClear, storage_area_id, url,
-        reply_msg));
-    return;
-  }
-
+void DOMStorageMessageFilter::OnClear(int64 storage_area_id, const GURL& url,
+                                      IPC::Message* reply_msg) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   DOMStorageArea* storage_area = Context()->GetStorageArea(storage_area_id);
   if (!storage_area) {
-    BrowserRenderProcessHost::BadMessageTerminateProcess(
-        ViewHostMsg_DOMStorageClear::ID, process_handle_);
+    BadMessageReceived(ViewHostMsg_DOMStorageClear::ID);
+    delete reply_msg;
     return;
   }
 
@@ -326,12 +258,12 @@
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnStorageEvent(
+void DOMStorageMessageFilter::OnStorageEvent(
     const ViewMsg_DOMStorageEvent_Params& params) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  const DOMStorageContext::DispatcherHostSet* set =
-      Context()->GetDispatcherHostSet();
-  DOMStorageContext::DispatcherHostSet::const_iterator cur = set->begin();
+  const DOMStorageContext::MessageFilterSet* set =
+      Context()->GetMessageFilterSet();
+  DOMStorageContext::MessageFilterSet::const_iterator cur = set->begin();
   while (cur != set->end()) {
     // The renderer that generates the event handles it itself.
     if (*cur != this)
@@ -339,3 +271,15 @@
     ++cur;
   }
 }
+
+// This small function is unfortunately a duplicate of what's in
+// ResourceMessageFilter, but it's better than having a dependency on that large
+// class.
+ChromeURLRequestContext* DOMStorageMessageFilter::GetRequestContextForURL(
+    const GURL& url) {
+  URLRequestContextGetter* context_getter =
+      url.SchemeIs(chrome::kExtensionScheme) ?
+          extensions_request_context_ : request_context_;
+  return static_cast<ChromeURLRequestContext*>(
+      context_getter->GetURLRequestContext());
+}