Regression(r254859) DOM storage event gets fired at the frame that caused the storage...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 15:17:10 +0000 (15:17 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 15:17:10 +0000 (15:17 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/storage/domstorage/events/resources/storage-event-not-in-originator-frame.html [new file with mode: 0644]
LayoutTests/storage/domstorage/events/storage-event-not-in-originator-expected.txt [new file with mode: 0644]
LayoutTests/storage/domstorage/events/storage-event-not-in-originator.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp

index 180a61a..8ea54c3 100644 (file)
@@ -1,3 +1,17 @@
+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>
 
         [css-grid] Update WPT imported tests and deduplicate common tests
diff --git a/LayoutTests/storage/domstorage/events/resources/storage-event-not-in-originator-frame.html b/LayoutTests/storage/domstorage/events/resources/storage-event-not-in-originator-frame.html
new file mode 100644 (file)
index 0000000..3d3a310
--- /dev/null
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+function storageEventHandler(e) {
+  top.childReceivedStorageEvent(e);
+}
+addEventListener("storage", storageEventHandler);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/storage/domstorage/events/storage-event-not-in-originator-expected.txt b/LayoutTests/storage/domstorage/events/storage-event-not-in-originator-expected.txt
new file mode 100644 (file)
index 0000000..af198b8
--- /dev/null
@@ -0,0 +1,18 @@
+Tests that the storage event gets fired in other frames but not the originating one
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS sessionStorage.getItem(sessionStorageKey) is null
+PASS sessionStorageKey === localStorageKey is false
+PASS localStorage.getItem(localStorageKey) is null
+PASS Child frame received storage event.
+PASS event.newValue === expectedValue is true
+PASS Child frame received storage event.
+PASS event.newValue === expectedValue is true
+PASS topStorageEventCount is 0
+PASS childStorageEventCount is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/domstorage/events/storage-event-not-in-originator.html b/LayoutTests/storage/domstorage/events/storage-event-not-in-originator.html
new file mode 100644 (file)
index 0000000..618f978
--- /dev/null
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../../resources/js-test.js"></script>
+<script>
+description("Tests that the storage event gets fired in other frames but not the originating one");
+jsTestIsAsync = true;
+
+function uuidv4()
+{
+    return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) {
+        var r = Math.random() * 16 | 0, v = c == 'x' ? r : (r & 0x3 | 0x8);
+        return v.toString(16);
+    });
+}
+
+let topStorageEventCount = 0;
+let childStorageEventCount = 0;
+
+function childReceivedStorageEvent(e)
+{
+   event = e;
+   testPassed("Child frame received storage event.");
+   if (e.key === sessionStorageKey)
+        expectedValue = sessionStorageValue;
+   else
+        expectedValue = localStorageValue;
+   shouldBeTrue("event.newValue === expectedValue");
+
+   childStorageEventCount++;
+   if (childStorageEventCount == 2) {
+       setTimeout(() => {
+           shouldBe("topStorageEventCount", "0");
+           shouldBe("childStorageEventCount", "2");
+           finishJSTest();
+       }, 0);
+   } 
+}
+
+function storageEventHandler(e) {
+  testFailed("Top frame received storage event.");
+  topStorageEventCount++;
+}
+addEventListener("storage", storageEventHandler);
+
+onload = () => {
+    sessionStorageKey = uuidv4();
+    sessionStorageValue = "foo";
+    shouldBeNull("sessionStorage.getItem(sessionStorageKey)");
+    sessionStorage.setItem(sessionStorageKey, sessionStorageValue);
+
+    localStorageKey = uuidv4();
+    localStorageValue = "bar";
+    shouldBeFalse("sessionStorageKey === localStorageKey");
+    shouldBeNull("localStorage.getItem(localStorageKey)");
+    localStorage.setItem(localStorageKey, localStorageValue);
+}
+</script>
+<iframe src="resources/storage-event-not-in-originator-frame.html">
+</body>
+</html>
index de634ef..8e6475a 100644 (file)
@@ -1,3 +1,25 @@
+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>
 
         REGRESSION (r261978): Cannot click on links with trackpad on iPad
index 73e2b4c..f491b6d 100644 (file)
@@ -276,14 +276,28 @@ void StorageAreaMap::clearCache()
     resetValues();
 }
 
-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;
     page.forEachDocument([&](auto& document) {
         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;
         
@@ -311,7 +325,7 @@ void StorageAreaMap::dispatchSessionStorageEvent(const Optional<StorageAreaImplI
     if (!page)
         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());
 }
 
@@ -324,7 +338,7 @@ void StorageAreaMap::dispatchLocalStorageEvent(const Optional<StorageAreaImplIde
     // Namespace IDs for local storage namespaces are equivalent to web page group IDs.
     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());
 }