[chromium] DomStorage event handling mods.
authormichaeln@google.com <michaeln@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Apr 2012 04:19:50 +0000 (04:19 +0000)
committermichaeln@google.com <michaeln@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Apr 2012 04:19:50 +0000 (04:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=84387

Source/WebCore:

Add a few simple inline getters to allow the source Document of a storage
event to be identified given a pointer to the source StorageArea so those
Documents can be excluded by the event dispatching logic.

Reviewed by Dimitri Glazkov.

No new tests. No new functionality or change in behavior.

* page/DOMWindow.h:
(WebCore::DOMWindow::optionalSessionStorage): simple inline getter to avoid construction where possible
(WebCore::DOMWindow::optionalLocalStorage): ditto
* storage/Storage.h:
(WebCore::Storage::area): simple inline getter

Source/WebKit/chromium:

Events are currently handled inconsistently. The dispatch of some events are
initiated from within webkit/webcore, the dispatch of other events are initiated
from the outside via the WebKit::WebStorageEventDispatcher interface. The existing
WebStorageEventDispatcher is not expressive enough to handle initiation all
events from the outside. There's a chunk of nearly replicated code in there
that shouldn't be. The existing code has several FIXMEs related to making
this better.

The current state of things in webkit/webcore is also blocking development of some
overall performance improvements to chromium's implemention of this feature (getting
rid of sync ipcs for each access and adding a renderer-side caching layer).

To facilitate the perf improvements, this patch adds WebKit API to allow all
DomStorage events to be initiated from the outside. This is needed because
there will be an async latency between setting an item and receiving the
'oldValue' from the main browser process which is required to raise the
mutation event.

This is the first of a multi-sided sequence of patches to make this transition.
wkpatch 1: add the new wider WebKit API and impl
crpatch 2: start using the new wider WebKit API
wkpatch 3: cleanup, delete the old WebKit API and impl
xxxxxxx 4/5: cleanup, remove extra params from new API whose only purpose was to allow the transition

Reviewed by Dimitri Glazkov.

* public/WebStorageEventDispatcher.h: Add new API to dispatch events.
* public/WebStorageNamespace.h: Add new API to test for namespace equality.
* src/StorageAreaProxy.cpp:
(WebCore::StorageAreaProxy::dispatchLocalStorageEvent): implementation of the new API
(WebCore::StorageAreaProxy::dispatchSessionStorageEvent):  implementation of the new API
(WebCore::StorageAreaProxy::IsEventSource): a helper method
(WebCore::StorageAreaProxy::FindPageWithSessionStorageNamespace): a helper method
* src/StorageAreaProxy.h:
* src/StorageEventDispatcherImpl.cpp: added a FIXME comment to delete soon
* src/StorageNamespaceProxy.cpp:
(WebCore::StorageNamespaceProxy::IsSameNamespace): plumbing to call the new API
* src/StorageNamespaceProxy.h:
* src/WebStorageEventDispatcherImpl.cpp:
(WebKit::WebStorageEventDispatcher::dispatchLocalStorageEvent): plumbing to call the new impl
(WebKit::WebStorageEventDispatcher::dispatchSessionStorageEvent): plumbing to call the new impl
* src/WebStorageEventDispatcherImpl.h: added a FIXME comment to delete soon

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@114993 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
Source/WebCore/ChangeLog
Source/WebCore/page/DOMWindow.h
Source/WebCore/storage/Storage.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/public/WebStorageEventDispatcher.h
Source/WebKit/chromium/public/WebStorageNamespace.h
Source/WebKit/chromium/src/StorageAreaProxy.cpp
Source/WebKit/chromium/src/StorageAreaProxy.h
Source/WebKit/chromium/src/StorageEventDispatcherImpl.cpp
Source/WebKit/chromium/src/StorageNamespaceProxy.cpp
Source/WebKit/chromium/src/StorageNamespaceProxy.h
Source/WebKit/chromium/src/WebStorageEventDispatcherImpl.cpp
Source/WebKit/chromium/src/WebStorageEventDispatcherImpl.h

index 38fe6a0..b55983e 100644 (file)
@@ -1,3 +1,22 @@
+2012-04-23  Michael Nordman  <michaeln@google.com>
+
+        [chromium] DomStorage event handling mods.
+        https://bugs.webkit.org/show_bug.cgi?id=84387
+
+        Add a few simple inline getters to allow the source Document of a storage
+        event to be identified given a pointer to the source StorageArea so those
+        Documents can be excluded by the event dispatching logic.
+
+        Reviewed by Dimitri Glazkov.
+
+        No new tests. No new functionality or change in behavior.
+
+        * page/DOMWindow.h:
+        (WebCore::DOMWindow::optionalSessionStorage): simple inline getter to avoid construction where possible
+        (WebCore::DOMWindow::optionalLocalStorage): ditto
+        * storage/Storage.h:
+        (WebCore::Storage::area): simple inline getter
+
 2012-04-23  Kenneth Russell  <kbr@google.com>
 
         Change ImageData to reference Uint8ClampedArray rather than CanvasPixelArray
index 1d3ff60..4d5897d 100644 (file)
@@ -354,6 +354,8 @@ namespace WebCore {
         // HTML 5 key/value storage
         Storage* sessionStorage(ExceptionCode&) const;
         Storage* localStorage(ExceptionCode&) const;
+        Storage* optionalSessionStorage() const { return m_sessionStorage.get(); }
+        Storage* optionalLocalStorage() const { return m_localStorage.get(); }
 
 #if ENABLE(QUOTA)
         StorageInfo* webkitStorageInfo() const;
index 98f041e..5eabf0f 100644 (file)
@@ -51,6 +51,8 @@ namespace WebCore {
 
         bool contains(const String& key) const;
 
+        StorageArea* area() const { return m_storageArea.get(); }
+
     private:
         Storage(Frame*, PassRefPtr<StorageArea>);
 
index 7cb9afc..15cfb10 100644 (file)
@@ -1,3 +1,51 @@
+2012-04-23  Michael Nordman  <michaeln@google.com>
+
+        [chromium] DomStorage event handling mods.
+        https://bugs.webkit.org/show_bug.cgi?id=84387
+
+        Events are currently handled inconsistently. The dispatch of some events are
+        initiated from within webkit/webcore, the dispatch of other events are initiated
+        from the outside via the WebKit::WebStorageEventDispatcher interface. The existing
+        WebStorageEventDispatcher is not expressive enough to handle initiation all 
+        events from the outside. There's a chunk of nearly replicated code in there
+        that shouldn't be. The existing code has several FIXMEs related to making
+        this better.
+
+        The current state of things in webkit/webcore is also blocking development of some
+        overall performance improvements to chromium's implemention of this feature (getting
+        rid of sync ipcs for each access and adding a renderer-side caching layer).
+
+        To facilitate the perf improvements, this patch adds WebKit API to allow all
+        DomStorage events to be initiated from the outside. This is needed because
+        there will be an async latency between setting an item and receiving the
+        'oldValue' from the main browser process which is required to raise the
+        mutation event.
+
+        This is the first of a multi-sided sequence of patches to make this transition.
+        wkpatch 1: add the new wider WebKit API and impl
+        crpatch 2: start using the new wider WebKit API
+        wkpatch 3: cleanup, delete the old WebKit API and impl
+        xxxxxxx 4/5: cleanup, remove extra params from new API whose only purpose was to allow the transition
+
+        Reviewed by Dimitri Glazkov.
+
+        * public/WebStorageEventDispatcher.h: Add new API to dispatch events.
+        * public/WebStorageNamespace.h: Add new API to test for namespace equality.
+        * src/StorageAreaProxy.cpp:
+        (WebCore::StorageAreaProxy::dispatchLocalStorageEvent): implementation of the new API
+        (WebCore::StorageAreaProxy::dispatchSessionStorageEvent):  implementation of the new API
+        (WebCore::StorageAreaProxy::IsEventSource): a helper method
+        (WebCore::StorageAreaProxy::FindPageWithSessionStorageNamespace): a helper method
+        * src/StorageAreaProxy.h:
+        * src/StorageEventDispatcherImpl.cpp: added a FIXME comment to delete soon
+        * src/StorageNamespaceProxy.cpp:
+        (WebCore::StorageNamespaceProxy::IsSameNamespace): plumbing to call the new API
+        * src/StorageNamespaceProxy.h:
+        * src/WebStorageEventDispatcherImpl.cpp:
+        (WebKit::WebStorageEventDispatcher::dispatchLocalStorageEvent): plumbing to call the new impl
+        (WebKit::WebStorageEventDispatcher::dispatchSessionStorageEvent): plumbing to call the new impl
+        * src/WebStorageEventDispatcherImpl.h: added a FIXME comment to delete soon
+
 2012-04-23  Kenneth Russell  <kbr@google.com>
 
         Change ImageData to reference Uint8ClampedArray rather than CanvasPixelArray
index 88d333a..4fbc596 100644 (file)
 
 namespace WebKit {
 
+class WebStorageArea;
+class WebStorageNamespace;
 class WebURL;
 
-// This is used to dispatch storage events to all pages.
-// FIXME: Make this (or something) work for SessionStorage!
 class WebStorageEventDispatcher {
 public:
-    WEBKIT_EXPORT static WebStorageEventDispatcher* create();
+    // Dispatch a local storage event to appropiate documents.
+    WEBKIT_EXPORT static void dispatchLocalStorageEvent(
+            const WebString& key, const WebString& oldValue,
+            const WebString& newValue, const WebURL& origin,
+            const WebURL& pageUrl, WebStorageArea* sourceAreaInstance,
+            bool originatedInProcess);
 
-    virtual ~WebStorageEventDispatcher() { }
+    // Dispatch a session storage event to appropiate documents.
+    WEBKIT_EXPORT static void dispatchSessionStorageEvent(
+            const WebString& key, const WebString& oldValue,
+            const WebString& newValue, const WebURL& origin,
+            const WebURL& pageUrl, const WebStorageNamespace&,
+            WebStorageArea* sourceAreaInstance, bool originatedInProcess);
 
-    // Dispatch the actual event.  Doesn't yet work for SessionStorage.
+    // DEPRECATED - The instance methods are going away soon in favor
+    // of the two static dispatch methods above.
+    WEBKIT_EXPORT static WebStorageEventDispatcher* create();
+    virtual ~WebStorageEventDispatcher() { }
     virtual void dispatchStorageEvent(const WebString& key, const WebString& oldValue,
                                       const WebString& newValue, const WebString& origin,
                                       const WebURL& url, bool isLocalStorage) = 0;
index 25f73ef..8943307 100644 (file)
@@ -53,6 +53,9 @@ public:
     // Copy a StorageNamespace. This only makes sense in the case of SessionStorage.
     virtual WebStorageNamespace* copy() = 0;
 
+    // Returns true of the two instances represent the same storage namespace.
+    virtual bool isSameNamespace(const WebStorageNamespace&) const { return false; }
+
     // DEPRECATED
     virtual void close() { }
 };
index 6b927f1..11eb7ce 100644 (file)
@@ -35,7 +35,9 @@
 #include "Page.h"
 #include "PageGroup.h"
 #include "SecurityOrigin.h"
+#include "Storage.h"
 #include "StorageEvent.h"
+#include "StorageNamespaceProxy.h"
 
 #include "WebFrameImpl.h"
 #include "WebPermissionClient.h"
@@ -121,6 +123,7 @@ bool StorageAreaProxy::contains(const String& key, Frame* frame) const
     return !getItem(key, frame).isNull();
 }
 
+// FIXME: remove this method and the calls to it from our setters after multi-side patch landing is done.
 // Copied from WebCore/storage/StorageEventDispatcher.cpp out of necessity.  It's probably best to keep it current.
 void StorageAreaProxy::storageEvent(const String& key, const String& oldValue, const String& newValue, StorageType storageType, SecurityOrigin* securityOrigin, Frame* sourceFrame)
 {
@@ -177,4 +180,76 @@ bool StorageAreaProxy::canAccessStorage(Frame* frame) const
     return !webView->permissionClient() || webView->permissionClient()->allowStorage(webFrame, m_storageType == LocalStorage);
 }
 
+void StorageAreaProxy::dispatchLocalStorageEvent(const String& pageGroupName, const String& key, const String& oldValue, const String& newValue,
+                                                 SecurityOrigin* securityOrigin, const KURL& pageURL, WebKit::WebStorageArea* sourceAreaInstance, bool originatedInProcess)
+{
+    // FIXME: Multi-sided patch engineering alert !
+    // step 1: this method gets defined and implemented in webkit/webcore with the early return.
+    // step 2: this method starts getting called by chromium still with the early return.
+    // step 3: This class's setters are modified to no longer raise SessionStorage
+    //         events for inprocess changes and this early return is removed.
+    if (originatedInProcess)
+        return;
+
+    const HashSet<Page*>& pages = PageGroup::pageGroup(pageGroupName)->pages();
+    for (HashSet<Page*>::const_iterator it = pages.begin(); it != pages.end(); ++it) {
+        for (Frame* frame = (*it)->mainFrame(); frame; frame = frame->tree()->traverseNext()) {
+            if (frame->document()->securityOrigin()->equal(securityOrigin) && !isEventSource(frame->domWindow()->optionalLocalStorage(), sourceAreaInstance)) {
+                // FIXME: maybe only raise if the window has an onstorage listener attached to avoid creating the Storage instance.
+                ExceptionCode ec = 0;
+                Storage* storage = frame->domWindow()->localStorage(ec);
+                if (!ec)
+                    frame->document()->enqueueWindowEvent(StorageEvent::create(eventNames().storageEvent, key, oldValue, newValue, pageURL, storage));
+            }
+        }
+    }
+}
+
+static Page* findPageWithSessionStorageNamespace(const String& pageGroupName, const WebKit::WebStorageNamespace& sessionNamespace)
+{
+    const HashSet<Page*>& pages = PageGroup::pageGroup(pageGroupName)->pages();
+    for (HashSet<Page*>::const_iterator it = pages.begin(); it != pages.end(); ++it) {
+        const bool createIfNeeded = true;
+        StorageNamespaceProxy* proxy = static_cast<StorageNamespaceProxy*>((*it)->sessionStorage(createIfNeeded));
+        if (proxy->isSameNamespace(sessionNamespace))
+            return *it;
+    }
+    return 0;
+}
+
+void StorageAreaProxy::dispatchSessionStorageEvent(const String& pageGroupName, const String& key, const String& oldValue, const String& newValue,
+                                                   SecurityOrigin* securityOrigin, const KURL& pageURL, const WebKit::WebStorageNamespace& sessionNamespace,
+                                                   WebKit::WebStorageArea* sourceAreaInstance, bool originatedInProcess)
+{
+    // FIXME: Multi-sided patch engineering alert !
+    // step 1: this method gets defined and implemented in webkit/webcore with the early return.
+    // step 2: this method starts getting called by chromium still with the early return.
+    // step 3: This class's setters are modified to no longer raise SessionStorage
+    //         events for inprocess changes and this early return is removed.
+    if (originatedInProcess)
+        return;
+
+    Page* page = findPageWithSessionStorageNamespace(pageGroupName, sessionNamespace);
+    if (!page)
+        return;
+
+    for (Frame* frame = page->mainFrame(); frame; frame = frame->tree()->traverseNext()) {
+        if (frame->document()->securityOrigin()->equal(securityOrigin) && !isEventSource(frame->domWindow()->optionalSessionStorage(), sourceAreaInstance)) {
+            // FIXME: maybe only raise if the window has an onstorage listener attached to avoid creating the Storage instance.
+            ExceptionCode ec = 0;
+            Storage* storage = frame->domWindow()->sessionStorage(ec);
+            if (!ec)
+                frame->document()->enqueueWindowEvent(StorageEvent::create(eventNames().storageEvent, key, oldValue, newValue, pageURL, storage));
+        }
+    }
+}
+
+bool StorageAreaProxy::isEventSource(Storage* storage, WebKit::WebStorageArea* sourceAreaInstance)
+{
+    if (!storage)
+        return false;
+    StorageAreaProxy* areaProxy = static_cast<StorageAreaProxy*>(storage->area());
+    return areaProxy->m_storageArea == sourceAreaInstance;
+}
+
 } // namespace WebCore
index 2949a52..ab23574 100644 (file)
 
 #include "StorageArea.h"
 
-namespace WebKit { class WebStorageArea; }
+namespace WebKit {
+class WebStorageArea;
+class WebStorageNamespace;
+}
 
 namespace WebCore {
 
 class Frame;
+class KURL;
+class Page;
 class SecurityOrigin;
+class Storage;
 
 class StorageAreaProxy : public StorageArea {
 public:
@@ -51,10 +57,20 @@ public:
 
     virtual bool disabledByPrivateBrowsingInFrame(const Frame*) const { return false; }
 
+    static void dispatchLocalStorageEvent(
+            const String& pageGroupName, const String& key, const String& oldValue, const String& newValue,
+            SecurityOrigin*, const KURL& pageURL, WebKit::WebStorageArea* sourceAreaInstance, bool originatedInProcess);
+    static void dispatchSessionStorageEvent(
+            const String& pageGroupName, const String& key, const String& oldValue, const String& newValue,
+            SecurityOrigin*, const KURL& pageURL, const WebKit::WebStorageNamespace&,
+            WebKit::WebStorageArea* sourceAreaInstance, bool originatedInProcess);
+
 private:
     void storageEvent(const String& key, const String& oldValue, const String& newValue, StorageType, SecurityOrigin*, Frame* sourceFrame);
     bool canAccessStorage(Frame*) const;
 
+    static bool isEventSource(Storage*, WebKit::WebStorageArea* sourceAreaInstance);
+
     OwnPtr<WebKit::WebStorageArea> m_storageArea;
     StorageType m_storageType;
 };
index 94317c4..e948b59 100644 (file)
@@ -41,6 +41,8 @@
 #include "SecurityOrigin.h"
 #include "StorageEvent.h"
 
+// FIXME: delete this almost obsolete file soon
+
 namespace WebCore {
 
 StorageEventDispatcherImpl::StorageEventDispatcherImpl(const String& groupName)
@@ -49,7 +51,6 @@ StorageEventDispatcherImpl::StorageEventDispatcherImpl(const String& groupName)
     ASSERT(m_pageGroup);
 }
 
-// FIXME: add a sourceStorageArea parameter to this
 void StorageEventDispatcherImpl::dispatchStorageEvent(const String& key, const String& oldValue,
                                                       const String& newValue, SecurityOrigin* securityOrigin,
                                                       const KURL& url, StorageType storageType)
@@ -66,7 +67,6 @@ void StorageEventDispatcherImpl::dispatchStorageEvent(const String& key, const S
     HashSet<Page*>::const_iterator end = pages.end();
     for (HashSet<Page*>::const_iterator it = pages.begin(); it != end; ++it) {
         for (Frame* frame = (*it)->mainFrame(); frame; frame = frame->tree()->traverseNext()) {
-            // FIXME: identify the srcFrame while in this loop too and exclude it from 'frames'.
             if (frame->document()->securityOrigin()->equal(securityOrigin))
                 frames.append(frame);
         }
index d061136..5c37e39 100644 (file)
@@ -100,4 +100,9 @@ void StorageNamespaceProxy::sync()
     ASSERT_NOT_REACHED();
 }
 
+bool StorageNamespaceProxy::isSameNamespace(const WebKit::WebStorageNamespace& sessionNamespace)
+{
+    return m_storageNamespace->isSameNamespace(sessionNamespace);
+}
+
 } // namespace WebCore
index d3d7596..f1b45fc 100644 (file)
@@ -45,6 +45,8 @@ public:
     virtual void clearAllOriginsForDeletion();
     virtual void sync();
 
+    bool isSameNamespace(const WebKit::WebStorageNamespace&);
+
 private:
     OwnPtr<WebKit::WebStorageNamespace> m_storageNamespace;
     StorageType m_storageType;
index c95e855..c8036aa 100644 (file)
@@ -33,6 +33,7 @@
 
 #include "KURL.h"
 #include "SecurityOrigin.h"
+#include "StorageAreaProxy.h"
 
 #include "platform/WebURL.h"
 #include <wtf/PassOwnPtr.h>
@@ -41,6 +42,33 @@ namespace WebKit {
 
 extern const char* pageGroupName;
 
+void WebStorageEventDispatcher::dispatchLocalStorageEvent(
+        const WebString& key, const WebString& oldValue,
+        const WebString& newValue, const WebURL& origin,
+        const WebURL& pageURL, WebStorageArea* sourceAreaInstance,
+        bool originatedInProcess)
+{
+    RefPtr<WebCore::SecurityOrigin> securityOrigin = WebCore::SecurityOrigin::create(origin);
+    WebCore::StorageAreaProxy::dispatchLocalStorageEvent(
+            pageGroupName, key, oldValue, newValue, securityOrigin.get(), pageURL,
+            sourceAreaInstance, originatedInProcess);
+}
+
+void WebStorageEventDispatcher::dispatchSessionStorageEvent(
+        const WebString& key, const WebString& oldValue,
+        const WebString& newValue, const WebURL& origin,
+        const WebURL& pageURL, const WebStorageNamespace& sessionNamespace,
+        WebStorageArea* sourceAreaInstance, bool originatedInProcess)
+{
+    RefPtr<WebCore::SecurityOrigin> securityOrigin = WebCore::SecurityOrigin::create(origin);
+    WebCore::StorageAreaProxy::dispatchSessionStorageEvent(
+            pageGroupName, key, oldValue, newValue, securityOrigin.get(), pageURL,
+            sessionNamespace, sourceAreaInstance, originatedInProcess);
+}
+
+
+// FIXME: remove the WebStorageEventDispatcherImpl class soon.
+
 WebStorageEventDispatcher* WebStorageEventDispatcher::create()
 {
     return new WebStorageEventDispatcherImpl();
index 7489636..b03c6b7 100644 (file)
 
 namespace WebKit {
 
+// DEPRECATED - to be removed when removing the instance methods in the public api.
 class WebStorageEventDispatcherImpl : public WebStorageEventDispatcher {
 public:
     WebStorageEventDispatcherImpl();
-
     virtual void dispatchStorageEvent(const WebString& key, const WebString& oldValue,
                                       const WebString& newValue, const WebString& origin,
                                       const WebURL&, bool isLocalStorage);
-
 private:
     OwnPtr<WebCore::StorageEventDispatcherImpl> m_eventDispatcher;
 };