Attachment 339310 Details for Bug 185191 – Patch

[patch] Patch

bug-185191-20180502095327.patch (text/plain), 4.19 KB, created by Chris Dumez on 2018-05-02 09:53:28 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-02 09:53:28 PDT

Size: 4.19 KB

patch

obsolete

>Subversion Revision: 231206
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 2a5ab73430be9982cec3adf5efbe9e40c45dd816..2377b858aba971aa56b076807eaca35e9458bede 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-05-02  Chris Dumez  <cdumez@apple.com>
>+
>+        document.open() event listener removal is not immediate
>+        https://bugs.webkit.org/show_bug.cgi?id=185191
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We need to make sure we set the 'wasremoved' flag on RegisteredEventListeners
>+        whenever they get removed from the EventListenerMap. We were doing so correctly
>+        in EventListenerMap:remove() but not EventListenerMap::clear(). This patch
>+        updates clear() accordingly.
>+
>+        The reason we need to set this flag is that RegisteredEventListeners is RefCounted
>+        and EventTarget::fireEventListeners() may be currently running and calling
>+        each listener one by one, holding a reference to all listener of a given event.
>+
>+        Test: fast/dom/Document/document-open-removes-all-listeners.html
>+
>+        * dom/EventListenerMap.cpp:
>+        (WebCore::EventListenerMap::clear):
>+
> 2018-05-01  Oleksandr Skachkov  <gskachkov@gmail.com>
> 
>         WebAssembly: add support for stream APIs - JavaScript API
>diff --git a/Source/WebCore/dom/EventListenerMap.cpp b/Source/WebCore/dom/EventListenerMap.cpp
>index 9fe3c27c2d9fb74a001b623ca67dbdf41c920ca9..bf922a942f687be255eca9c08de056d3227f4ab6 100644
>--- a/Source/WebCore/dom/EventListenerMap.cpp
>+++ b/Source/WebCore/dom/EventListenerMap.cpp
>@@ -86,6 +86,11 @@ void EventListenerMap::clear()
>     
>     assertNoActiveIterators();
> 
>+    for (auto& entry : m_entries) {
>+        for (auto& listener : *entry.second)
>+            listener->markAsRemoved();
>+    }
>+
>     m_entries.clear();
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 8357fb5c49a7c485333961aaa24582e039adaf8f..8d97aa5c586ddf2db24213575dc66b89669f5792 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-05-02  Chris Dumez  <cdumez@apple.com>
>+
>+        document.open() event listener removal is not immediate
>+        https://bugs.webkit.org/show_bug.cgi?id=185191
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add layout test coverage.
>+
>+        * fast/dom/Document/document-open-removes-all-listeners-expected.txt: Added.
>+        * fast/dom/Document/document-open-removes-all-listeners.html: Added.
>+
> 2018-05-01  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed test gardening, correct a typo in the iOS TestExpectation file.
>diff --git a/LayoutTests/fast/dom/Document/document-open-removes-all-listeners-expected.txt b/LayoutTests/fast/dom/Document/document-open-removes-all-listeners-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..410ddd06e8ae814c238eeedf99d7bbb6ee1daf69
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Document/document-open-removes-all-listeners-expected.txt
>@@ -0,0 +1,10 @@
>+Event listeners are to be removed with immediate effect by document.open()
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS once is true
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+ 
>diff --git a/LayoutTests/fast/dom/Document/document-open-removes-all-listeners.html b/LayoutTests/fast/dom/Document/document-open-removes-all-listeners.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..45e66be931168aecabba61cdaa89452a4c592373
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Document/document-open-removes-all-listeners.html
>@@ -0,0 +1,21 @@
>+<DOCTYPE html>
>+<html>
>+<body>
>+<script src="../../../resources/js-test.js"></script>
>+<script>
>+description("Event listeners are to be removed with immediate effect by document.open()");
>+
>+const frame = document.body.appendChild(document.createElement("iframe"));
>+let once = false;
>+frame.contentDocument.addEventListener("x", () => {
>+    frame.contentDocument.open();
>+    once = true;
>+});
>+frame.contentDocument.addEventListener("x", function() {
>+    testFailed("second event listener not removed")
>+});
>+frame.contentDocument.dispatchEvent(new Event("x"));
>+shouldBeTrue("once");
>+frame.contentDocument.close();
>+</script>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 185191: 339310