Do not fire readystatechange events at documents about to get replaced by javascript...
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jul 2019 22:53:47 +0000 (22:53 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jul 2019 22:53:47 +0000 (22:53 +0000)
<rdar://problem/51665406> and https://bugs.webkit.org/show_bug.cgi?id=198786

Reviewed by Ryosuke Niwa.

Source/WebCore:

Test: http/tests/dom/ready-state-on-javascript-replace.html

We were firing too many readystatechange events, more than other browsers.
Our behavior on this test with this patch now matches Chrome.

(There was even an ancient FIXME alluding to this referencing a spec issue, and that issues has long been resolvedv)

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::stopLoading):

LayoutTests:

* http/tests/dom/ready-state-on-javascript-replace-expected.txt: Added.
* http/tests/dom/ready-state-on-javascript-replace.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/dom/ready-state-on-javascript-replace-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/dom/ready-state-on-javascript-replace.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp

index 6e00a09..985b42e 100644 (file)
@@ -1,3 +1,13 @@
+2019-07-26  Brady Eidson  <beidson@apple.com>
+
+        Do not fire readystatechange events at documents about to get replaced by javascript URLs.
+        <rdar://problem/51665406> and https://bugs.webkit.org/show_bug.cgi?id=198786
+
+        Reviewed by Ryosuke Niwa.
+
+        * http/tests/dom/ready-state-on-javascript-replace-expected.txt: Added.
+        * http/tests/dom/ready-state-on-javascript-replace.html: Added.
+
 2019-07-26  Chris Dumez  <cdumez@apple.com>
 
         [iOS] WebPage::TouchEventSync() & WebPage::GetPositionInformation() sync IPC causes UIProcess hangs
diff --git a/LayoutTests/http/tests/dom/ready-state-on-javascript-replace-expected.txt b/LayoutTests/http/tests/dom/ready-state-on-javascript-replace-expected.txt
new file mode 100644 (file)
index 0000000..33fa5bb
--- /dev/null
@@ -0,0 +1,3 @@
+This test makes sure an iframe whose document is being closed to be replaced by a javascript: url doesn't fire a readystatechange event.
+You should see no alerts.
+
diff --git a/LayoutTests/http/tests/dom/ready-state-on-javascript-replace.html b/LayoutTests/http/tests/dom/ready-state-on-javascript-replace.html
new file mode 100644 (file)
index 0000000..5cf5e14
--- /dev/null
@@ -0,0 +1,26 @@
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+window.onload = () => {
+    frame = document.createElement('iframe');
+    frame.src = location;
+    document.body.appendChild(frame);
+
+    frame.contentDocument.open();
+    frame.contentDocument.onreadystatechange = () => {
+        alert("Outer handler: " + frame.contentDocument.readyState);
+        frame.contentWindow.addEventListener('readystatechange', () => {
+            alert("Inner handler: " + frame.contentDocument.readyState);
+        }, {capture: true, once: true});
+    }
+    frame.src = 'javascript:"<script>function endIt() { if (window.top.testRunner) window.top.testRunner.notifyDone(); }; setTimeout(endIt, 0);</scr' + 'ipt>"';
+}
+
+</script>
+<body>
+This test makes sure an iframe whose document is being closed to be replaced by a javascript: url doesn't fire a readystatechange event.<br>
+You should see no alerts.<br>
+</body>
\ No newline at end of file
index 9f028dc..a4d55ec 100644 (file)
@@ -1,3 +1,20 @@
+2019-07-26  Brady Eidson  <beidson@apple.com>
+
+        Do not fire readystatechange events at documents about to get replaced by javascript URLs.
+        <rdar://problem/51665406> and https://bugs.webkit.org/show_bug.cgi?id=198786
+
+        Reviewed by Ryosuke Niwa.
+
+        Test: http/tests/dom/ready-state-on-javascript-replace.html
+
+        We were firing too many readystatechange events, more than other browsers.
+        Our behavior on this test with this patch now matches Chrome.
+
+        (There was even an ancient FIXME alluding to this referencing a spec issue, and that issues has long been resolvedv)
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::stopLoading):
+
 2019-07-26  Zalan Bujtas  <zalan@apple.com>
 
         [iPadOS] wix.com cannot select a template to edit or view
index fc23a14..fb4c5b8 100644 (file)
@@ -497,10 +497,6 @@ void FrameLoader::stopLoading(UnloadEventPolicy unloadEventPolicy)
     }
 
     if (auto* document = m_frame.document()) {
-        // FIXME: HTML5 doesn't tell us to set the state to complete when aborting, but we do anyway to match legacy behavior.
-        // http://www.w3.org/Bugs/Public/show_bug.cgi?id=10537
-        document->setReadyState(Document::Complete);
-
         // FIXME: Should the DatabaseManager watch for something like ActiveDOMObject::stop() rather than being special-cased here?
         DatabaseManager::singleton().stopDatabases(*document, nullptr);
     }