Log Message
fast/events/before-unload-remove-itself.html crashes with async policy delegates https://bugs.webkit.org/show_bug.cgi?id=183290 <rdar://problem/38069045>
Reviewed by Alex Christensen. Source/WebCore: When the navigation policy happens asynchronously, it is now possible for the Frame / FrameLoader to get destroyed between the point that policyChecker().checkNavigationPolicy() is called and when continueLoadAfterNavigationPolicy() is called. To address the issue, we now protect the Frame and capture it in the lambda passed to policyChecker().checkNavigationPolicy(). Test: fast/events/before-unload-remove-itself-async-delegate.html * loader/FrameLoader.cpp: (WebCore::FrameLoader::loadURL): (WebCore::FrameLoader::loadWithDocumentLoader): LayoutTests: Add layout test coverage. * fast/events/before-unload-remove-itself-async-delegate-expected.txt: Added. * fast/events/before-unload-remove-itself-async-delegate.html: Added.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/loader/FrameLoader.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (229178 => 229179)
--- trunk/LayoutTests/ChangeLog 2018-03-02 17:48:06 UTC (rev 229178)
+++ trunk/LayoutTests/ChangeLog 2018-03-02 17:52:17 UTC (rev 229179)
@@ -1,5 +1,18 @@
2018-03-02 Chris Dumez <cdu...@apple.com>
+ fast/events/before-unload-remove-itself.html crashes with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183290
+ <rdar://problem/38069045>
+
+ Reviewed by Alex Christensen.
+
+ Add layout test coverage.
+
+ * fast/events/before-unload-remove-itself-async-delegate-expected.txt: Added.
+ * fast/events/before-unload-remove-itself-async-delegate.html: Added.
+
+2018-03-02 Chris Dumez <cdu...@apple.com>
+
Converting a load to a download does not work with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183254
<rdar://problem/38035334>
Added: trunk/LayoutTests/fast/events/before-unload-remove-itself-async-delegate-expected.txt (0 => 229179)
--- trunk/LayoutTests/fast/events/before-unload-remove-itself-async-delegate-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/before-unload-remove-itself-async-delegate-expected.txt 2018-03-02 17:52:17 UTC (rev 229179)
@@ -0,0 +1,4 @@
+This test ensures a beforeunload event handler can safely remove the frame to which the event is fired. You should see PASS below:
+
+PASS
+
Added: trunk/LayoutTests/fast/events/before-unload-remove-itself-async-delegate.html (0 => 229179)
--- trunk/LayoutTests/fast/events/before-unload-remove-itself-async-delegate.html (rev 0)
+++ trunk/LayoutTests/fast/events/before-unload-remove-itself-async-delegate.html 2018-03-02 17:52:17 UTC (rev 229179)
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This test ensures a beforeunload event handler can safely remove the frame to which the event is fired. You should see PASS below:</p>
+<pre id="log"></pre>
+<script>
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.dumpChildFramesAsText();
+ testRunner.waitUntilDone();
+ if (testRunner.setShouldDecideNavigationPolicyAfterDelay)
+ testRunner.setShouldDecideNavigationPolicyAfterDelay(true);
+}
+
+var log = document.getElementById('log');
+var done = false;
+
+function test() {
+ if (done) {
+ // log's not having any content implies that load event was dispatched for the second time before beforeunload is dispatched.
+ if (!log.innerHTML.length)
+ log.innerHTML = 'FAIL: beforeunload event was never dispatched.\n';
+ else
+ log.innerHTML = 'FAIL: beforeunload event handler did not remove the frame.\n';
+ if (window.testRunner)
+ testRunner.notifyDone();
+ return;
+ }
+ done = true;
+ document.getElementsByTagName('iframe')[0].contentWindow.location.href = '';
+}
+
+function fired() {
+ document.body.removeChild(document.body.getElementsByTagName('iframe')[0]);
+ log.innerHTML = 'PASS\n';
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+
+</script>
+<iframe _onload_="test()" src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (229178 => 229179)
--- trunk/Source/WebCore/ChangeLog 2018-03-02 17:48:06 UTC (rev 229178)
+++ trunk/Source/WebCore/ChangeLog 2018-03-02 17:52:17 UTC (rev 229179)
@@ -1,5 +1,26 @@
2018-03-02 Chris Dumez <cdu...@apple.com>
+ fast/events/before-unload-remove-itself.html crashes with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183290
+ <rdar://problem/38069045>
+
+ Reviewed by Alex Christensen.
+
+ When the navigation policy happens asynchronously, it is now possible for the
+ Frame / FrameLoader to get destroyed between the point that policyChecker().checkNavigationPolicy()
+ is called and when continueLoadAfterNavigationPolicy() is called.
+
+ To address the issue, we now protect the Frame and capture it in the lambda passed
+ to policyChecker().checkNavigationPolicy().
+
+ Test: fast/events/before-unload-remove-itself-async-delegate.html
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+
+2018-03-02 Chris Dumez <cdu...@apple.com>
+
Converting a load to a download does not work with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183254
<rdar://problem/38035334>
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (229178 => 229179)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-03-02 17:48:06 UTC (rev 229178)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-03-02 17:52:17 UTC (rev 229179)
@@ -1326,7 +1326,7 @@
policyChecker().stopCheck();
policyChecker().setLoadType(newLoadType);
auto completionHandlerCalled = adoptRef(*new SharedBool);
- policyChecker().checkNavigationPolicy(ResourceRequest(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, bool shouldContinue) {
+ policyChecker().checkNavigationPolicy(ResourceRequest(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame), completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, bool shouldContinue) {
if (!completionHandlerCalled->value) {
completionHandlerCalled->value = true;
continueFragmentScrollAfterNavigationPolicy(request, shouldContinue);
@@ -1496,7 +1496,7 @@
oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
policyChecker().stopCheck();
auto completionHandlerCalled = adoptRef(*new SharedBool);
- policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, bool shouldContinue) {
+ policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame), completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, bool shouldContinue) {
if (!completionHandlerCalled->value) {
completionHandlerCalled->value = true;
continueFragmentScrollAfterNavigationPolicy(request, shouldContinue);
@@ -1532,7 +1532,7 @@
m_frame.navigationScheduler().cancel(true);
- policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, allowNavigationToInvalidURL] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
+ policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
});
}
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
