Handle storage events locally when possible.

TEST=none
BUG=33108

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

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

8 files changed, 75 insertions, 53 deletions

diff --git a/chrome/browser/in_process_webkit/dom_storage_area.cc b/chrome/browser/in_process_webkit/dom_storage_area.cc
index d55adb9..32295a5 100644
--- a/chrome/browser/in_process_webkit/dom_storage_area.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_area.cc
@@ -11,6 +11,7 @@
 #include "third_party/WebKit/WebKit/chromium/public/WebURL.h"
 
 using WebKit::WebStorageArea;
+using WebKit::WebString;
 using WebKit::WebURL;
 
 DOMStorageArea::DOMStorageArea(const string16& origin,
@@ -40,20 +41,26 @@ NullableString16 DOMStorageArea::GetItem(const string16& key) {
   return storage_area_->getItem(key);
 }
 
-void DOMStorageArea::SetItem(const string16& key, const string16& value,
-                             bool* quota_exception) {
+NullableString16 DOMStorageArea::SetItem(
+    const string16& key, const string16& value, bool* quota_exception) {
   CreateWebStorageAreaIfNecessary();
-  storage_area_->setItem(key, value, WebURL(), *quota_exception);
+  WebString old_value;
+  storage_area_->setItem(key, value, WebURL(), *quota_exception, old_value);
+  return old_value;
 }
 
-void DOMStorageArea::RemoveItem(const string16& key) {
+NullableString16 DOMStorageArea::RemoveItem(const string16& key) {
   CreateWebStorageAreaIfNecessary();
-  storage_area_->removeItem(key, WebURL());
+  WebString old_value;
+  storage_area_->removeItem(key, WebURL(), old_value);
+  return old_value;
 }
 
-void DOMStorageArea::Clear() {
+bool DOMStorageArea::Clear() {
   CreateWebStorageAreaIfNecessary();
-  storage_area_->clear(WebURL());
+  bool somethingCleared;
+  storage_area_->clear(WebURL(), somethingCleared);
+  return somethingCleared;
 }
 
 void DOMStorageArea::PurgeMemory() {
diff --git a/chrome/browser/in_process_webkit/dom_storage_area.h b/chrome/browser/in_process_webkit/dom_storage_area.h
index 218aaf4..f53df58 100644
--- a/chrome/browser/in_process_webkit/dom_storage_area.h
+++ b/chrome/browser/in_process_webkit/dom_storage_area.h
@@ -25,10 +25,10 @@ class DOMStorageArea {
   unsigned Length();
   NullableString16 Key(unsigned index);
   NullableString16 GetItem(const string16& key);
-  void SetItem(const string16& key, const string16& value,
-               bool* quota_xception);
-  void RemoveItem(const string16& key);
-  void Clear();
+  NullableString16 SetItem(const string16& key, const string16& value,
+                           bool* quota_exception);
+  NullableString16 RemoveItem(const string16& key);
+  bool Clear();
   void PurgeMemory();
 
   int64 id() const { return id_; }
diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
index 31ed956..aab6a5b 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
@@ -118,8 +118,9 @@ bool DOMStorageDispatcherHost::OnMessageReceived(const IPC::Message& message,
     IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageKey, OnKey)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageGetItem, OnGetItem)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageSetItem, OnSetItem)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_DOMStorageRemoveItem, OnRemoveItem)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_DOMStorageClear, OnClear)
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageRemoveItem,
+                                    OnRemoveItem)
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_DOMStorageClear, OnClear)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -258,17 +259,20 @@ void DOMStorageDispatcherHost::OnSetItem(
   }
 
   ScopedStorageEventContext scope(this, &url);
-  storage_area->SetItem(key, value, &quota_exception);
-  ViewHostMsg_DOMStorageSetItem::WriteReplyParams(reply_msg, quota_exception);
+  NullableString16 old_value = storage_area->SetItem(key, value,
+                                                     &quota_exception);
+  ViewHostMsg_DOMStorageSetItem::WriteReplyParams(reply_msg, quota_exception,
+                                                  old_value);
   Send(reply_msg);
 }
 
 void DOMStorageDispatcherHost::OnRemoveItem(
-    int64 storage_area_id, const string16& key, const GURL& url) {
+    int64 storage_area_id, const string16& key, const GURL& url,
+    IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
     ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
         this, &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key,
-        url));
+        url, reply_msg));
     return;
   }
 
@@ -281,13 +285,17 @@ void DOMStorageDispatcherHost::OnRemoveItem(
   }
 
   ScopedStorageEventContext scope(this, &url);
-  storage_area->RemoveItem(key);
+  NullableString16 old_value = storage_area->RemoveItem(key);
+  ViewHostMsg_DOMStorageRemoveItem::WriteReplyParams(reply_msg, old_value);
+  Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnClear(int64 storage_area_id, const GURL& url) {
+void DOMStorageDispatcherHost::OnClear(int64 storage_area_id, const GURL& url,
+                                       IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
     ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnClear, storage_area_id, url));
+        this, &DOMStorageDispatcherHost::OnClear, storage_area_id, url,
+        reply_msg));
     return;
   }
 
@@ -300,7 +308,9 @@ void DOMStorageDispatcherHost::OnClear(int64 storage_area_id, const GURL& url) {
   }
 
   ScopedStorageEventContext scope(this, &url);
-  storage_area->Clear();
+  bool something_cleared = storage_area->Clear();
+  ViewHostMsg_DOMStorageClear::WriteReplyParams(reply_msg, something_cleared);
+  Send(reply_msg);
 }
 
 void DOMStorageDispatcherHost::OnStorageEvent(
@@ -310,7 +320,9 @@ void DOMStorageDispatcherHost::OnStorageEvent(
       Context()->GetDispatcherHostSet();
   DOMStorageContext::DispatcherHostSet::const_iterator cur = set->begin();
   while (cur != set->end()) {
-    (*cur)->Send(new ViewMsg_DOMStorageEvent(params));
+    // The renderer that generates the event handles it itself.
+    if (*cur != this)
+      (*cur)->Send(new ViewMsg_DOMStorageEvent(params));
     ++cur;
   }
 }
diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
index 55a87b5..fe4b3b7 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
@@ -67,8 +67,8 @@ class DOMStorageDispatcherHost
                  const string16& value, const GURL& url,
                  IPC::Message* reply_msg);
   void OnRemoveItem(int64 storage_area_id, const string16& key,
-                    const GURL& url);
-  void OnClear(int64 storage_area_id, const GURL& url);
+                    const GURL& url, IPC::Message* reply_msg);
+  void OnClear(int64 storage_area_id, const GURL& url, IPC::Message* reply_msg);
 
   // Only call on the IO thread.
   void OnStorageEvent(const ViewMsg_DOMStorageEvent_Params& params);
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index fcf448c..c46af28 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -1881,23 +1881,26 @@ IPC_BEGIN_MESSAGES(ViewHost)
                               NullableString16 /* value */)
 
   // Set a value that's associated with a key in a storage area.
-  IPC_SYNC_MESSAGE_CONTROL4_1(ViewHostMsg_DOMStorageSetItem,
+  IPC_SYNC_MESSAGE_CONTROL4_2(ViewHostMsg_DOMStorageSetItem,
                               int64 /* storage_area_id */,
                               string16 /* key */,
                               string16 /* value */,
                               GURL /* url */,
-                              bool /* quota_exception */)
+                              bool /* quota_exception */,
+                              NullableString16 /* old_value */)
 
   // Remove the value associated with a key in a storage area.
-  IPC_MESSAGE_CONTROL3(ViewHostMsg_DOMStorageRemoveItem,
-                       int64 /* storage_area_id */,
-                       string16 /* key */,
-                       GURL /* url */)
+  IPC_SYNC_MESSAGE_CONTROL3_1(ViewHostMsg_DOMStorageRemoveItem,
+                              int64 /* storage_area_id */,
+                              string16 /* key */,
+                              GURL /* url */,
+                              NullableString16 /* old_value */)
 
   // Clear the storage area.
-  IPC_MESSAGE_CONTROL2(ViewHostMsg_DOMStorageClear,
-                       int64 /* storage_area_id */,
-                       GURL /* url */)
+  IPC_SYNC_MESSAGE_CONTROL2_1(ViewHostMsg_DOMStorageClear,
+                              int64 /* storage_area_id */,
+                              GURL /* url */,
+                              bool /* something_cleared */)
 
   // Get file size in bytes. Set result to -1 if failed to get the file size.
   IPC_SYNC_MESSAGE_CONTROL1_1(ViewHostMsg_GetFileSize,
diff --git a/chrome/renderer/renderer_webstoragearea_impl.cc b/chrome/renderer/renderer_webstoragearea_impl.cc
index 95e688e..43caed5 100644
--- a/chrome/renderer/renderer_webstoragearea_impl.cc
+++ b/chrome/renderer/renderer_webstoragearea_impl.cc
@@ -44,19 +44,26 @@ WebString RendererWebStorageAreaImpl::getItem(const WebString& key) {
 
 void RendererWebStorageAreaImpl::setItem(
     const WebString& key, const WebString& value, const WebURL& url,
-    bool& quota_exception) {
+    bool& quota_exception, WebString& old_value_webkit) {
+  NullableString16 old_value;
   RenderThread::current()->Send(
       new ViewHostMsg_DOMStorageSetItem(storage_area_id_, key, value, url,
-                                        &quota_exception));
+                                        &quota_exception, &old_value));
+  old_value_webkit = old_value;
 }
 
-void RendererWebStorageAreaImpl::removeItem(const WebString& key,
-                                            const WebURL& url) {
+void RendererWebStorageAreaImpl::removeItem(
+    const WebString& key, const WebURL& url, WebString& old_value_webkit) {
+  NullableString16 old_value;
   RenderThread::current()->Send(
-      new ViewHostMsg_DOMStorageRemoveItem(storage_area_id_, key, url));
+      new ViewHostMsg_DOMStorageRemoveItem(storage_area_id_, key,
+                                           url, &old_value));
+  old_value_webkit = old_value;
 }
 
-void RendererWebStorageAreaImpl::clear(const WebURL& url) {
+void RendererWebStorageAreaImpl::clear(
+    const WebURL& url, bool& cleared_something) {
   RenderThread::current()->Send(
-      new ViewHostMsg_DOMStorageClear(storage_area_id_, url));
+      new ViewHostMsg_DOMStorageClear(storage_area_id_, url,
+                                      &cleared_something));
 }
diff --git a/chrome/renderer/renderer_webstoragearea_impl.h b/chrome/renderer/renderer_webstoragearea_impl.h
index 040c9db..5ce935f 100644
--- a/chrome/renderer/renderer_webstoragearea_impl.h
+++ b/chrome/renderer/renderer_webstoragearea_impl.h
@@ -22,10 +22,12 @@ class RendererWebStorageAreaImpl : public WebKit::WebStorageArea {
   virtual WebKit::WebString getItem(const WebKit::WebString& key);
   virtual void setItem(
       const WebKit::WebString& key, const WebKit::WebString& value,
-      const WebKit::WebURL& url, bool& quota_exception);
-  virtual void removeItem(const WebKit::WebString& key,
-                          const WebKit::WebURL& url);
-  virtual void clear(const WebKit::WebURL& url);
+      const WebKit::WebURL& url, bool& quota_exception,
+      WebKit::WebString& old_value);
+  virtual void removeItem(
+      const WebKit::WebString& key, const WebKit::WebURL& url,
+      WebKit::WebString& old_value);
+  virtual void clear(const WebKit::WebURL& url, bool& cleared_something);
 
  private:
   // The ID we use for all IPC.
diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index 0b553b9..b776f1f 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -2216,12 +2216,3 @@
    fun:find_objects_by_template
    fun:nssToken_FindCertificatesBySubject
 }
-{
-   bug_33108
-   Memcheck:Cond
-   fun:_ZN7WebCore16StorageAreaProxy5clearEPNS_5FrameE
-   fun:_ZN7WebCore7Storage5clearEv
-   fun:_ZN7WebCore15StorageInternal13clearCallbackERKN2v89ArgumentsE
-   fun:_ZN2v88internal21Builtin_HandleApiCallENS0_9ArgumentsE
-   obj:*
-}