Changeset 262058 in webkit

Timestamp:

May 22, 2020 8:17:10 AM (4 years ago)

Author:

Chris Dumez

Message:

Regression(r254859) DOM storage event gets fired at the frame that caused the storage modification
​https://bugs.webkit.org/show_bug.cgi?id=211503
<rdar://problem/62983284>

Reviewed by Maciej Stachowiak.

Source/WebKit:

r254859 refactored StorageAreaMap's dispatchSessionStorageEvent() &
dispatchLocalStorageEvent() to share more code by moving that code to
a new framesForEventDispatching() static function. However,
framesForEventDispatching() was always using the session storage no
matter the call site. It should be using the local storage when called
from dispatchLocalStorageEvent().

Test: storage/domstorage/events/storage-event-not-in-originator.html

• WebProcess/WebStorage/StorageAreaMap.cpp:

(WebKit::framesForEventDispatching):
(WebKit::StorageAreaMap::dispatchSessionStorageEvent):
(WebKit::StorageAreaMap::dispatchLocalStorageEvent):

LayoutTests:

Add layout test coverage.

• storage/domstorage/events/resources/storage-event-not-in-originator-frame.html: Added.
• storage/domstorage/events/storage-event-not-in-originator-expected.txt: Added.
• storage/domstorage/events/storage-event-not-in-originator.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/storage/domstorage/events/resources/storage-event-not-in-originator-frame.html (added)
• LayoutTests/storage/domstorage/events/storage-event-not-in-originator-expected.txt (added)
• LayoutTests/storage/domstorage/events/storage-event-not-in-originator.html (added)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-05-22  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r254859) DOM storage event gets fired at the frame that caused the storage modification
+        https://bugs.webkit.org/show_bug.cgi?id=211503
+        <rdar://problem/62983284>
+
+        Reviewed by Maciej Stachowiak.
+
+        Add layout test coverage.
+
+        * storage/domstorage/events/resources/storage-event-not-in-originator-frame.html: Added.
+        * storage/domstorage/events/storage-event-not-in-originator-expected.txt: Added.
+        * storage/domstorage/events/storage-event-not-in-originator.html: Added.
+
 2020-05-22  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-05-22  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r254859) DOM storage event gets fired at the frame that caused the storage modification
+        https://bugs.webkit.org/show_bug.cgi?id=211503
+        <rdar://problem/62983284>
+
+        Reviewed by Maciej Stachowiak.
+
+        r254859 refactored StorageAreaMap's dispatchSessionStorageEvent() &
+        dispatchLocalStorageEvent() to share more code by moving that code to
+        a new framesForEventDispatching() static function. However,
+        framesForEventDispatching() was always using the session storage no
+        matter the call site. It should be using the local storage when called
+        from dispatchLocalStorageEvent().
+
+        Test: storage/domstorage/events/storage-event-not-in-originator.html
+
+        * WebProcess/WebStorage/StorageAreaMap.cpp:
+        (WebKit::framesForEventDispatching):
+        (WebKit::StorageAreaMap::dispatchSessionStorageEvent):
+        (WebKit::StorageAreaMap::dispatchLocalStorageEvent):
+
 2020-05-22  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp

@@ -277 +277 @@
 }
 
-static Vector<RefPtr<Frame>> framesForEventDispatching(Page& page, SecurityOrigin& origin, const Optional<StorageAreaImplIdentifier>& storageAreaImplID)
+static Vector<RefPtr<Frame>> framesForEventDispatching(Page& page, SecurityOrigin& origin, StorageType storageType, const Optional<StorageAreaImplIdentifier>& storageAreaImplID)
 {
     Vector<RefPtr<Frame>> frames;
@@ -283 +283 @@
         if (!document.securityOrigin().equal(&origin))
             return;
+
+        auto* window = document.domWindow();
+        if (!window)
+            return;
         
-        auto* storage = document.domWindow() ? document.domWindow()->optionalSessionStorage() : nullptr;
+        Storage* storage = nullptr;
+        switch (storageType) {
+        case StorageType::Session:
+            storage = window->optionalSessionStorage();
+            break;
+        case StorageType::Local:
+        case StorageType::TransientLocal:
+            storage = window->optionalLocalStorage();
+            break;
+        }
+
         if (!storage)
             return;
@@ -312 +326 @@
         return;
 
-    auto frames = framesForEventDispatching(*page, m_securityOrigin, storageAreaImplID);
+    auto frames = framesForEventDispatching(*page, m_securityOrigin, StorageType::Session, storageAreaImplID);
     StorageEventDispatcher::dispatchSessionStorageEventsToFrames(*page, frames, key, oldValue, newValue, urlString, m_securityOrigin->data());
 }
@@ -325 +339 @@
     auto& pageGroup = *WebProcess::singleton().webPageGroup(m_namespace.pageGroupID())->corePageGroup();
     for (auto* page : pageGroup.pages())
-        frames.appendVector(framesForEventDispatching(*page, m_securityOrigin, storageAreaImplID));
+        frames.appendVector(framesForEventDispatching(*page, m_securityOrigin, StorageType::Local, storageAreaImplID));
 
     StorageEventDispatcher::dispatchLocalStorageEventsToFrames(pageGroup, frames, key, oldValue, newValue, urlString, m_securityOrigin->data());