WebKit Bugzilla
Attachment 362393 Details for
Bug 194820
: REGRESSION(r240909): Release assertion in FrameLoader::loadPostRequest when opening new window
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Fixes the bug
bug-194820-20190219104724.patch (text/plain), 7.52 KB, created by
Ryosuke Niwa
on 2019-02-19 10:47:25 PST
(
hide
)
Description:
Fixes the bug
Filename:
MIME Type:
Creator:
Ryosuke Niwa
Created:
2019-02-19 10:47:25 PST
Size:
7.52 KB
patch
obsolete
>Index: Source/WebCore/ChangeLog >=================================================================== >--- Source/WebCore/ChangeLog (revision 241761) >+++ Source/WebCore/ChangeLog (working copy) >@@ -1,3 +1,27 @@ >+2019-02-19 Ryosuke Niwa <rniwa@webkit.org> >+ >+ REGRESSION(r240909): Release assertion in FrameLoader::loadPostRequest when opening new window >+ https://bugs.webkit.org/show_bug.cgi?id=194820 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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 Commit Queue <commit-queue@webkit.org> > > Unreviewed, rolling out r241722. >Index: Source/WebCore/loader/FrameLoader.cpp >=================================================================== >--- Source/WebCore/loader/FrameLoader.cpp (revision 241755) >+++ Source/WebCore/loader/FrameLoader.cpp (working copy) >@@ -1381,8 +1381,6 @@ void FrameLoader::loadURL(FrameLoadReque > > if (!targetFrame && !effectiveFrameName.isEmpty()) { > action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, 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 @@ void FrameLoader::load(FrameLoadRequest& > > 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 @@ void FrameLoader::loadPostRequest(FrameL > 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(); >Index: LayoutTests/ChangeLog >=================================================================== >--- LayoutTests/ChangeLog (revision 241755) >+++ LayoutTests/ChangeLog (working copy) >@@ -1,3 +1,15 @@ >+2019-02-19 Ryosuke Niwa <rniwa@webkit.org> >+ >+ REGRESSION(r240909): Release assertion in FrameLoader::loadPostRequest when opening new window >+ https://bugs.webkit.org/show_bug.cgi?id=194820 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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-18 Alex Christensen <achristensen@webkit.org> > > Adjust test expectations after r241754 >Index: LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt >=================================================================== >--- LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt (nonexistent) >+++ LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation-expected.txt (working copy) >@@ -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". >+ >+ >Index: LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html >=================================================================== >--- LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html (nonexistent) >+++ LayoutTests/fast/loader/navigate-with-post-to-new-target-after-back-forward-navigation.html (working copy) >@@ -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 = '?first'; >+ let step = 0; >+ frame.onload = () => { >+ switch (step++) { >+ case 0: >+ setTimeout(() => frame.src = '?second', 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="?third" method="post" target="unknownTarget"><input type="text" name="query"><input type="submit"></a>`); >+} >+ >+</script> >+</body> >+</html>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194820
: 362393 |
362413
|
362442
|
362467