Log Message
REGRESSION(r240909): Release assertion in FrameLoader::loadPostRequest when opening new window https://bugs.webkit.org/show_bug.cgi?id=194820
Reviewed by Geoffrey Garen. Source/WebCore: This release assertion was wrong. The invocation of PolicyChecker::checkNewWindowPolicy in FrameLoader doesn’t require PolicyChecker's load type to be set in PolicyChecker because FrameLoader's continueLoadAfterNewWindowPolicy invokes loadWithNavigationAction which sets the load type later, and we don't rely on PolicyChecker's load type until then. Fixed the crash by removing relese asserts before invoking checkNewWindowPolicy accordingly. This patch reverts r241015 since it too was asserting that PolicyChecker's load type is set before invoking checkNewWindowPolicy which is not the right assumption. Test: fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html * loader/FrameLoader.cpp: (WebCore::FrameLoader::loadURL): (WebCore::FrameLoader::load): (WebCore::FrameLoader::loadPostRequest): LayoutTests: Added a regression test. * fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt: Added. * fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html: Added.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/loader/FrameLoader.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (241779 => 241780)
--- trunk/LayoutTests/ChangeLog 2019-02-19 22:57:11 UTC (rev 241779)
+++ trunk/LayoutTests/ChangeLog 2019-02-19 23:07:52 UTC (rev 241780)
@@ -1,3 +1,15 @@
+2019-02-19 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r240909): Release assertion in FrameLoader::loadPostRequest when opening new window
+ https://bugs.webkit.org/show_bug.cgi?id=194820
+
+ Reviewed by Geoffrey Garen.
+
+ Added a regression test.
+
+ * fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt: Added.
+ * fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html: Added.
+
2019-02-19 Truitt Savell <[email protected]>
[ iOS ] Layout Tests in editing/pasteboard/data-transfer-set-data-* are flaky Timeouts
Added: trunk/LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt (0 => 241780)
--- trunk/LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt 2019-02-19 23:07:52 UTC (rev 241780)
@@ -0,0 +1,5 @@
+ALERT: PASS
+This tests navigating via an anchor element with a non-existent target name, which should create a new window.
+WebKit should not hit any assertions and alert "PASS".
+
+
Added: trunk/LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html (0 => 241780)
--- trunk/LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html (rev 0)
+++ trunk/LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html 2019-02-19 23:07:52 UTC (rev 241780)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ testRunner.setCanOpenWindows();
+ testRunner.setCloseRemainingWindowsWhenComplete();
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+}
+
+if (location.search == '?third') {
+ alert('PASS');
+ if (window.testRunner)
+ testRunner.notifyDone();
+} else if (self == top) {
+ document.write(`<p>This tests navigating via an anchor element with a non-existent target name, which should create a new window.<br>
+WebKit should not hit any assertions and alert "PASS".</p>`);
+ const frame = document.createElement('iframe');
+ frame.src = '';
+ let step = 0;
+ frame._onload_ = () => {
+ switch (step++) {
+ case 0:
+ setTimeout(() => frame.src = '', 0);
+ break;
+ case 1:
+ setTimeout(() => history.back(), 0);
+ break;
+ }
+ }
+ document.body.appendChild(frame);
+} else {
+ if (location.search == '?first') {
+ if (localStorage.getItem('loaded')) {
+ localStorage.removeItem('loaded');
+ window._onload_ = () => {
+ setTimeout(() => document.querySelector('form').submit(), 0);
+ }
+ } else
+ localStorage.setItem('loaded', 'true');
+ }
+ document.write(location.search);
+ document.write(` <form action="" method="post" target="unknownTarget"><input type="text" name="query"><input type="submit"></a>`);
+}
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (241779 => 241780)
--- trunk/Source/WebCore/ChangeLog 2019-02-19 22:57:11 UTC (rev 241779)
+++ trunk/Source/WebCore/ChangeLog 2019-02-19 23:07:52 UTC (rev 241780)
@@ -1,3 +1,27 @@
+2019-02-19 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r240909): Release assertion in FrameLoader::loadPostRequest when opening new window
+ https://bugs.webkit.org/show_bug.cgi?id=194820
+
+ Reviewed by Geoffrey Garen.
+
+ This release assertion was wrong. The invocation of PolicyChecker::checkNewWindowPolicy in FrameLoader
+ doesn’t require PolicyChecker's load type to be set in PolicyChecker because FrameLoader's
+ continueLoadAfterNewWindowPolicy invokes loadWithNavigationAction which sets the load type later,
+ and we don't rely on PolicyChecker's load type until then.
+
+ Fixed the crash by removing relese asserts before invoking checkNewWindowPolicy accordingly.
+
+ This patch reverts r241015 since it too was asserting that PolicyChecker's load type is set before
+ invoking checkNewWindowPolicy which is not the right assumption.
+
+ Test: fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::load):
+ (WebCore::FrameLoader::loadPostRequest):
+
2019-02-19 Zalan Bujtas <[email protected]>
Fix post-commit feedback.
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (241779 => 241780)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2019-02-19 22:57:11 UTC (rev 241779)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2019-02-19 23:07:52 UTC (rev 241780)
@@ -1381,8 +1381,6 @@
if (!targetFrame && !effectiveFrameName.isEmpty()) {
action = "" frameLoadRequest));
- policyChecker().setLoadType(newLoadType);
- RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType()));
policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), WTFMove(formState), effectiveFrameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) mutable {
continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
completionHandler();
@@ -1467,7 +1465,6 @@
if (request.shouldCheckNewWindowPolicy()) {
NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() };
- RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType()));
policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request.resourceRequest()), { }, request.frameName(), [this] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
});
@@ -3022,7 +3019,6 @@
return;
}
- RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType()));
policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(workingResourceRequest), WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) mutable {
continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
completionHandler();
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
