Fixup the DomStorage IPC messsages.

- make OpenStorageArea async
- use GURL to represent origins instead of strings
- consistently use 'connection_id'
- send DomStorage event messages to the originating renderer too
  (these are dropped on the floor on that side pending additional
   webkit/webcore changes)
- include the src connection_id for DomStorage event message that
  originated in the destination renderer
Review URL: https://chromiumcodereview.appspot.com/10116007

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

6 files changed, 106 insertions, 59 deletions

diff --git a/content/browser/dom_storage/dom_storage_message_filter.cc b/content/browser/dom_storage/dom_storage_message_filter.cc
index 1560a23..e61d98f 100644
--- a/content/browser/dom_storage/dom_storage_message_filter.cc
+++ b/content/browser/dom_storage/dom_storage_message_filter.cc
@@ -9,6 +9,7 @@
 #include "base/nullable_string16.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/utf_string_conversions.h"
+#include "content/public/browser/user_metrics.h"
 #include "content/browser/dom_storage/dom_storage_context_impl.h"
 #include "content/common/dom_storage_messages.h"
 #include "googleurl/src/gurl.h"
@@ -17,6 +18,7 @@
 #include "webkit/dom_storage/dom_storage_task_runner.h"
 
 using content::BrowserThread;
+using content::UserMetricsAction;
 using dom_storage::DomStorageTaskRunner;
 using WebKit::WebStorageArea;
 
@@ -24,7 +26,7 @@ DOMStorageMessageFilter::DOMStorageMessageFilter(
     int unused,
     DOMStorageContextImpl* context)
     : context_(context->context()),
-      is_dispatching_message_(false) {
+      connection_dispatching_message_for_(0) {
 }
 
 DOMStorageMessageFilter::~DOMStorageMessageFilter() {
@@ -74,7 +76,12 @@ bool DOMStorageMessageFilter::OnMessageReceived(const IPC::Message& message,
     return false;
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(host_.get());
-  AutoReset<bool> auto_reset(&is_dispatching_message_, true);
+  DCHECK_EQ(0, connection_dispatching_message_for_);
+
+  // The connection_id is always the first param.
+  int connection_id = IPC::MessageIterator(message).NextInt();
+  AutoReset<int> auto_reset(&connection_dispatching_message_for_,
+                            connection_id);
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_EX(DOMStorageMessageFilter, message, *message_was_ok)
     IPC_MESSAGE_HANDLER(DOMStorageHostMsg_OpenStorageArea, OnOpenStorageArea)
@@ -90,31 +97,34 @@ bool DOMStorageMessageFilter::OnMessageReceived(const IPC::Message& message,
   return handled;
 }
 
-void DOMStorageMessageFilter::OnOpenStorageArea(int64 namespace_id,
-                                                const string16& origin,
-                                                int64* connection_id) {
+void DOMStorageMessageFilter::OnOpenStorageArea(int connection_id,
+                                                int64 namespace_id,
+                                                const GURL& origin) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
-  *connection_id = host_->OpenStorageArea(namespace_id, GURL(origin));
+  if (!host_->OpenStorageArea(connection_id, namespace_id, origin)) {
+    content::RecordAction(UserMetricsAction("BadMessageTerminate_DSMF"));
+    BadMessageReceived();
+  }
 }
 
-void DOMStorageMessageFilter::OnCloseStorageArea(int64 connection_id) {
+void DOMStorageMessageFilter::OnCloseStorageArea(int connection_id) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
   host_->CloseStorageArea(connection_id);
 }
 
-void DOMStorageMessageFilter::OnLength(int64 connection_id,
+void DOMStorageMessageFilter::OnLength(int connection_id,
                                        unsigned* length) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
   *length = host_->GetAreaLength(connection_id);
 }
 
-void DOMStorageMessageFilter::OnKey(int64 connection_id, unsigned index,
+void DOMStorageMessageFilter::OnKey(int connection_id, unsigned index,
                                     NullableString16* key) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
   *key = host_->GetAreaKey(connection_id, index);
 }
 
-void DOMStorageMessageFilter::OnGetItem(int64 connection_id,
+void DOMStorageMessageFilter::OnGetItem(int connection_id,
                                         const string16& key,
                                         NullableString16* value) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -122,7 +132,7 @@ void DOMStorageMessageFilter::OnGetItem(int64 connection_id,
 }
 
 void DOMStorageMessageFilter::OnSetItem(
-    int64 connection_id, const string16& key,
+    int connection_id, const string16& key,
     const string16& value, const GURL& page_url,
     WebKit::WebStorageArea::Result* result, NullableString16* old_value) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -134,7 +144,7 @@ void DOMStorageMessageFilter::OnSetItem(
 }
 
 void DOMStorageMessageFilter::OnRemoveItem(
-    int64 connection_id, const string16& key, const GURL& pageUrl,
+    int connection_id, const string16& key, const GURL& pageUrl,
     NullableString16* old_value) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
   string16 old_string_value;
@@ -144,7 +154,7 @@ void DOMStorageMessageFilter::OnRemoveItem(
     *old_value = NullableString16(true);
 }
 
-void DOMStorageMessageFilter::OnClear(int64 connection_id, const GURL& page_url,
+void DOMStorageMessageFilter::OnClear(int connection_id, const GURL& page_url,
                                       bool* something_cleared) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
   *something_cleared = host_->ClearArea(connection_id, page_url);
@@ -189,14 +199,10 @@ void DOMStorageMessageFilter::SendDomStorageEvent(
     const NullableString16& new_value,
     const NullableString16& old_value) {
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // Only send if this renderer is not the source of the event.
-  if (is_dispatching_message_)
-    return;
   DOMStorageMsg_Event_Params params;
-  // TODO(michaeln): change the origin type to be GURL in ipc params.
-  params.origin = UTF8ToUTF16(area->origin().spec());
+  params.origin = area->origin();
   params.page_url = page_url;
-  params.namespace_id = dom_storage::kLocalStorageNamespaceId;
+  params.connection_id = connection_dispatching_message_for_;
   params.key = key;
   params.new_value = new_value;
   params.old_value = old_value;
diff --git a/content/browser/dom_storage/dom_storage_message_filter.h b/content/browser/dom_storage/dom_storage_message_filter.h
index c1e720b..1426faa 100644
--- a/content/browser/dom_storage/dom_storage_message_filter.h
+++ b/content/browser/dom_storage/dom_storage_message_filter.h
@@ -46,20 +46,20 @@ class DOMStorageMessageFilter
                                  bool* message_was_ok) OVERRIDE;
 
   // Message Handlers.
-  void OnOpenStorageArea(int64 namespace_id, const string16& origin,
-                         int64* storage_area_id);
-  void OnCloseStorageArea(int64 storage_area_id);
-  void OnLength(int64 storage_area_id, unsigned* length);
-  void OnKey(int64 storage_area_id, unsigned index, NullableString16* key);
-  void OnGetItem(int64 storage_area_id, const string16& key,
+  void OnOpenStorageArea(int connection_id, int64 namespace_id,
+                         const GURL& origin);
+  void OnCloseStorageArea(int connection_id);
+  void OnLength(int connection_id, unsigned* length);
+  void OnKey(int connection_id, unsigned index, NullableString16* key);
+  void OnGetItem(int connection_id, const string16& key,
                  NullableString16* value);
-  void OnSetItem(int64 storage_area_id, const string16& key,
+  void OnSetItem(int connection_id, const string16& key,
                  const string16& value, const GURL& url,
                  WebKit::WebStorageArea::Result* result,
                  NullableString16* old_value);
-  void OnRemoveItem(int64 storage_area_id, const string16& key,
+  void OnRemoveItem(int connection_id, const string16& key,
                     const GURL& url, NullableString16* old_value);
-  void OnClear(int64 storage_area_id, const GURL& url, bool* something_cleared);
+  void OnClear(int connection_id, const GURL& url, bool* something_cleared);
 
   // DomStorageContext::EventObserver implementation which
   // sends events back to our renderer process.
@@ -87,7 +87,7 @@ class DOMStorageMessageFilter
 
   scoped_refptr<dom_storage::DomStorageContext> context_;
   scoped_ptr<dom_storage::DomStorageHost> host_;
-  bool is_dispatching_message_;
+  int connection_dispatching_message_for_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(DOMStorageMessageFilter);
 };
diff --git a/content/common/dom_storage_messages.h b/content/common/dom_storage_messages.h
index 9ecc853..6b95faf 100644
--- a/content/common/dom_storage_messages.h
+++ b/content/common/dom_storage_messages.h
@@ -12,7 +12,7 @@
 
 #define IPC_MESSAGE_START DOMStorageMsgStart
 
-// Signals a storage event.
+// Signals a local storage event.
 IPC_STRUCT_BEGIN(DOMStorageMsg_Event_Params)
   // The key that generated the storage event.  Null if clear() was called.
   IPC_STRUCT_MEMBER(NullableString16, key)
@@ -24,13 +24,14 @@ IPC_STRUCT_BEGIN(DOMStorageMsg_Event_Params)
   IPC_STRUCT_MEMBER(NullableString16, new_value)
 
   // The origin this is associated with.
-  IPC_STRUCT_MEMBER(string16, origin)
+  IPC_STRUCT_MEMBER(GURL, origin)
 
   // The URL of the page that caused the storage event.
   IPC_STRUCT_MEMBER(GURL, page_url)
 
-  // The namespace_id this is associated with.
-  IPC_STRUCT_MEMBER(int64, namespace_id)
+  // The non-zero connection_id which caused the event or 0 if the event
+  // was not caused by the target renderer process.
+  IPC_STRUCT_MEMBER(int, connection_id)
 IPC_STRUCT_END()
 
 IPC_ENUM_TRAITS(WebKit::WebStorageArea::Result)
@@ -43,38 +44,38 @@ IPC_MESSAGE_CONTROL1(DOMStorageMsg_Event,
 
 
 // DOM Storage messages sent from the renderer to the browser.
+// Note: The 'connection_id' must be the first parameter in these message.
 
 // Open the storage area for a particular origin within a namespace.
-// TODO(michaeln): make this async and have the renderer send the connection_id
-IPC_SYNC_MESSAGE_CONTROL2_1(DOMStorageHostMsg_OpenStorageArea,
+IPC_MESSAGE_CONTROL3(DOMStorageHostMsg_OpenStorageArea,
+                            int /* connection_id */,
                             int64 /* namespace_id */,
-                            string16 /* origin */,
-                            int64 /* connection_id */)
+                            GURL /* origin */)
 
 // Close a previously opened storage area.
 IPC_MESSAGE_CONTROL1(DOMStorageHostMsg_CloseStorageArea,
-                     int64 /* connection_id */)
+                     int /* connection_id */)
 
 // Get the length of a storage area.
 IPC_SYNC_MESSAGE_CONTROL1_1(DOMStorageHostMsg_Length,
-                            int64 /* connection_id */,
+                            int /* connection_id */,
                             unsigned /* length */)
 
 // Get a the ith key within a storage area.
 IPC_SYNC_MESSAGE_CONTROL2_1(DOMStorageHostMsg_Key,
-                            int64 /* connection_id */,
+                            int /* connection_id */,
                             unsigned /* index */,
                             NullableString16 /* key */)
 
 // Get a value based on a key from a storage area.
 IPC_SYNC_MESSAGE_CONTROL2_1(DOMStorageHostMsg_GetItem,
-                            int64 /* connection_id */,
+                            int /* connection_id */,
                             string16 /* key */,
                             NullableString16 /* value */)
 
 // Set a value that's associated with a key in a storage area.
 IPC_SYNC_MESSAGE_CONTROL4_2(DOMStorageHostMsg_SetItem,
-                            int64 /* connection_id */,
+                            int /* connection_id */,
                             string16 /* key */,
                             string16 /* value */,
                             GURL /* page_url */,
@@ -83,14 +84,14 @@ IPC_SYNC_MESSAGE_CONTROL4_2(DOMStorageHostMsg_SetItem,
 
 // Remove the value associated with a key in a storage area.
 IPC_SYNC_MESSAGE_CONTROL3_1(DOMStorageHostMsg_RemoveItem,
-                            int64 /* connection_id */,
+                            int /* connection_id */,
                             string16 /* key */,
                             GURL /* page_url */,
                             NullableString16 /* old_value */)
 
 // Clear the storage area.
 IPC_SYNC_MESSAGE_CONTROL2_1(DOMStorageHostMsg_Clear,
-                            int64 /* connection_id */,
+                            int /* connection_id */,
                             GURL /* page_url */,
                             bool /* something_cleared */)
 
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index f61b5e8..1edd2ca 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -56,6 +56,7 @@
 #include "content/renderer/render_process_impl.h"
 #include "content/renderer/render_view_impl.h"
 #include "content/renderer/renderer_webkitplatformsupport_impl.h"
+#include "content/renderer/renderer_webstoragearea_impl.h"
 #include "grit/content_resources.h"
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_platform_file.h"
@@ -816,9 +817,28 @@ void RenderThreadImpl::OnDOMStorageEvent(
     EnsureWebKitInitialized();
     dom_storage_event_dispatcher_.reset(WebStorageEventDispatcher::create());
   }
-  dom_storage_event_dispatcher_->dispatchStorageEvent(params.key,
-      params.old_value, params.new_value, params.origin, params.page_url,
-      params.namespace_id == dom_storage::kLocalStorageNamespaceId);
+
+  // TODO(michaeln): Return early until webkit/webcore is modified to not
+  // raise LocalStorage events internally. Looks like i have some multi-side
+  // patch engineering in front of me todo.
+  if (params.connection_id)
+    return;
+
+  // TODO(michaeln): fix the webkit api so we don't need to convert from
+  // string types to url types and to pass in the 'area' which
+  // caused this event to occur and to get rid of the is_local param.
+  // RendererWebStorageAreaImpl* originating_area =
+  //     RendererWebStorageAreaImpl::FromConnectionId(params.connection_id);
+
+  // SessionStorage events are always raised internally by webkit/webcore.
+  const bool kIsLocalStorage = true;
+  dom_storage_event_dispatcher_->dispatchStorageEvent(
+      params.key,
+      params.old_value,
+      params.new_value,
+      UTF8ToUTF16(params.origin.spec()),
+      params.page_url,
+      kIsLocalStorage);
 }
 
 bool RenderThreadImpl::OnControlMessageReceived(const IPC::Message& msg) {
diff --git a/content/renderer/renderer_webstoragearea_impl.cc b/content/renderer/renderer_webstoragearea_impl.cc
index 457ce11..a7cbfb6 100644
--- a/content/renderer/renderer_webstoragearea_impl.cc
+++ b/content/renderer/renderer_webstoragearea_impl.cc
@@ -4,8 +4,10 @@
 
 #include "content/renderer/renderer_webstoragearea_impl.h"
 
+#include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "base/time.h"
+#include "base/utf_string_conversions.h"
 #include "content/common/dom_storage_messages.h"
 #include "content/renderer/render_thread_impl.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebURL.h"
@@ -14,16 +16,33 @@
 using WebKit::WebString;
 using WebKit::WebURL;
 
+typedef IDMap<RendererWebStorageAreaImpl> AreaImplMap;
+
+static base::LazyInstance<AreaImplMap>::Leaky
+    g_all_areas_map = LAZY_INSTANCE_INITIALIZER;
+
+// static
+RendererWebStorageAreaImpl* RendererWebStorageAreaImpl::FromConnectionId(
+    int id) {
+  return g_all_areas_map.Pointer()->Lookup(id);
+}
+
 RendererWebStorageAreaImpl::RendererWebStorageAreaImpl(
-    int64 namespace_id, const WebString& origin) {
+    int64 namespace_id, const WebString& origin)
+    : ALLOW_THIS_IN_INITIALIZER_LIST(
+          connection_id_(g_all_areas_map.Pointer()->Add(this))) {
+  // TODO(michaeln): fix the webkit api to have the 'origin' input
+  // be a URL instead of a string.
+  DCHECK(connection_id_);
   RenderThreadImpl::current()->Send(
-      new DOMStorageHostMsg_OpenStorageArea(namespace_id, origin,
-                                            &storage_area_id_));
+      new DOMStorageHostMsg_OpenStorageArea(
+          connection_id_, namespace_id, GURL(origin)));
 }
 
 RendererWebStorageAreaImpl::~RendererWebStorageAreaImpl() {
+  g_all_areas_map.Pointer()->Remove(connection_id_);
   RenderThreadImpl::current()->Send(
-      new DOMStorageHostMsg_CloseStorageArea(storage_area_id_));
+      new DOMStorageHostMsg_CloseStorageArea(connection_id_));
 }
 
 // In November 2011 stats were recorded about performance of each of the
@@ -43,21 +62,21 @@ RendererWebStorageAreaImpl::~RendererWebStorageAreaImpl() {
 unsigned RendererWebStorageAreaImpl::length() {
   unsigned length;
   RenderThreadImpl::current()->Send(
-      new DOMStorageHostMsg_Length(storage_area_id_, &length));
+      new DOMStorageHostMsg_Length(connection_id_, &length));
   return length;
 }
 
 WebString RendererWebStorageAreaImpl::key(unsigned index) {
   NullableString16 key;
   RenderThreadImpl::current()->Send(
-      new DOMStorageHostMsg_Key(storage_area_id_, index, &key));
+      new DOMStorageHostMsg_Key(connection_id_, index, &key));
   return key;
 }
 
 WebString RendererWebStorageAreaImpl::getItem(const WebString& key) {
   NullableString16 value;
   RenderThreadImpl::current()->Send(
-      new DOMStorageHostMsg_GetItem(storage_area_id_, key, &value));
+      new DOMStorageHostMsg_GetItem(connection_id_, key, &value));
   return value;
 }
 
@@ -70,7 +89,7 @@ void RendererWebStorageAreaImpl::setItem(
   }
   NullableString16 old_value;
   RenderThreadImpl::current()->Send(new DOMStorageHostMsg_SetItem(
-      storage_area_id_, key, value, url, &result, &old_value));
+      connection_id_, key, value, url, &result, &old_value));
   old_value_webkit = old_value;
 }
 
@@ -78,12 +97,12 @@ void RendererWebStorageAreaImpl::removeItem(
     const WebString& key, const WebURL& url, WebString& old_value_webkit) {
   NullableString16 old_value;
   RenderThreadImpl::current()->Send(
-      new DOMStorageHostMsg_RemoveItem(storage_area_id_, key, url, &old_value));
+      new DOMStorageHostMsg_RemoveItem(connection_id_, key, url, &old_value));
   old_value_webkit = old_value;
 }
 
 void RendererWebStorageAreaImpl::clear(
     const WebURL& url, bool& cleared_something) {
   RenderThreadImpl::current()->Send(
-      new DOMStorageHostMsg_Clear(storage_area_id_, url, &cleared_something));
+      new DOMStorageHostMsg_Clear(connection_id_, url, &cleared_something));
 }
diff --git a/content/renderer/renderer_webstoragearea_impl.h b/content/renderer/renderer_webstoragearea_impl.h
index d19c78d..9206d1c 100644
--- a/content/renderer/renderer_webstoragearea_impl.h
+++ b/content/renderer/renderer_webstoragearea_impl.h
@@ -12,6 +12,8 @@
 
 class RendererWebStorageAreaImpl : public WebKit::WebStorageArea {
  public:
+  static RendererWebStorageAreaImpl* FromConnectionId(int id);
+
   RendererWebStorageAreaImpl(int64 namespace_id,
                              const WebKit::WebString& origin);
   virtual ~RendererWebStorageAreaImpl();
@@ -30,8 +32,7 @@ class RendererWebStorageAreaImpl : public WebKit::WebStorageArea {
   virtual void clear(const WebKit::WebURL& url, bool& cleared_something);
 
  private:
-  // The ID we use for all IPC.
-  int64 storage_area_id_;
+  int connection_id_;
 };
 
 #endif  // CONTENT_RENDERER_RENDERER_WEBSTORAGEAREA_IMPL_H_