Changeset 229341 in webkit


Ignore:
Timestamp:
Mar 6, 2018 2:01:27 PM (6 years ago)
Author:
Chris Dumez
Message:

fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed.html fails with async policy delegates
​https://bugs.webkit.org/show_bug.cgi?id=183345

Reviewed by Alex Christensen.

Source/WebCore:

FrameLoader::loadURL() was calling loadWithNavigationAction() and then resetting the
m_quickRedirectComing flag right after. This works if the navigation policy decision
triggered by loadWithNavigationAction() is made synchronously. However, when it is
made asynchronously, the flag gets reset too early, before the policy decision
handler has been called. This is an issue because the policy decision handler
relies on the m_quickRedirectComing flag.

Similarly, FrameLoader::loadFrameRequest() was calling loadPostRequest() / loadURL()
and then focusing a frame right after. This does not work as intended when the navigation
policy decision is made asynchronously.

To address the issue, we now pass a completion handler that gets called when the operation
has actually completion, after the policy decision has been made. This maintains the
behavior in place with synchronous policy delegates.

Test: fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates.html

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::loadURLIntoChildFrame):
(WebCore::FrameLoader::loadFrameRequest):
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::loadWithNavigationAction):
(WebCore::FrameLoader::load):
(WebCore::FrameLoader::loadWithDocumentLoader):
(WebCore::FrameLoader::reloadWithOverrideEncoding):
(WebCore::FrameLoader::reload):
(WebCore::FrameLoader::loadPostRequest):
(WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
(WebCore::FrameLoader::loadDifferentDocumentItem):

  • loader/FrameLoader.h:

Tools:

Add layout test infrastructure so a test can know when didCancelClientRedirectForFrame has
been called. The tests used to rely on a 0-timer but this does not work when the client
responds to the navigation policy asynchronously.

  • DumpRenderTree/TestRunner.cpp:

(getDidCancelClientRedirect):
(TestRunner::staticValues):

  • DumpRenderTree/TestRunner.h:

(TestRunner::didCancelClientRedirect const):
(TestRunner::setDidCancelClientRedirect):

  • DumpRenderTree/mac/FrameLoadDelegate.mm:

(-[FrameLoadDelegate webView:didCancelClientRedirectForFrame:]):

  • WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
  • WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:

(WTR::InjectedBundlePage::didCancelClientRedirectForFrame):

  • WebKitTestRunner/InjectedBundle/TestRunner.h:

(WTR::TestRunner::didCancelClientRedirect const):
(WTR::TestRunner::setDidCancelClientRedirect):

LayoutTests:

  • fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates-expected.txt: Added.
  • fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates.html: Added.

Add layout test coverage.

  • fast/loader/redirect-to-invalid-url-using-javascript-disallowed-expected.txt:
  • fast/loader/redirect-to-invalid-url-using-javascript-disallowed.html:
  • fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-expected.txt:
  • fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed.html:
  • fast/loader/window-open-to-invalid-url-disallowed-expected.txt:
  • fast/loader/window-open-to-invalid-url-disallowed.html:
  • platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-javascript-disallowed-expected.txt:
  • platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates-expected.txt: Added.
  • platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-expected.txt:
  • platform/mac-wk1/fast/loader/window-open-to-invalid-url-disallowed-expected.txt:

Update tests that were relying on a 0-timer to make sure that didCancelClientRedirectForFrame was
called to rely on our new test infrastructure instead. This is needed so that these tests keep passing
once we make policy delegates asynchronous by default. Without this, the didCancelClientRedirectForFrame lines
would be missing in the tests' output.

Location:
trunk
Files:
4 added
20 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r229335 r229341  
     12018-03-06  Chris Dumez  <cdumez@apple.com>
     2
     3        fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed.html fails with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183345
     5
     6        Reviewed by Alex Christensen.
     7
     8        * fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates-expected.txt: Added.
     9        * fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates.html: Added.
     10        Add layout test coverage.
     11
     12        * fast/loader/redirect-to-invalid-url-using-javascript-disallowed-expected.txt:
     13        * fast/loader/redirect-to-invalid-url-using-javascript-disallowed.html:
     14        * fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-expected.txt:
     15        * fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed.html:
     16        * fast/loader/window-open-to-invalid-url-disallowed-expected.txt:
     17        * fast/loader/window-open-to-invalid-url-disallowed.html:
     18        * platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-javascript-disallowed-expected.txt:
     19        * platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates-expected.txt: Added.
     20        * platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-expected.txt:
     21        * platform/mac-wk1/fast/loader/window-open-to-invalid-url-disallowed-expected.txt:
     22        Update tests that were relying on a 0-timer to make sure that didCancelClientRedirectForFrame was
     23        called to rely on our new test infrastructure instead. This is needed so that these tests keep passing
     24        once we make policy delegates asynchronous by default. Without this, the didCancelClientRedirectForFrame lines
     25        would be missing in the tests' output.
     26
    1272018-03-06  Youenn Fablet  <youenn@apple.com>
    228

  • trunk/LayoutTests/fast/loader/redirect-to-invalid-url-using-javascript-disallowed-expected.txt

    r207162 r229341  
     1main frame - didFinishDocumentLoadForFrame
    12main frame - willPerformClientRedirectToURL: http://A=a%B=b
    2 main frame - didFinishDocumentLoadForFrame
     3main frame - didHandleOnloadEventsForFrame
    34main frame - didFinishLoadForFrame
    45main frame - didCancelClientRedirectForFrame
    56Tests that we do not redirect to an invalid URL initiated by JavaScript. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    67
    7 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/LayoutTests/fast/loader/redirect-to-invalid-url-using-javascript-disallowed.html

    r207162 r229341  
    22<html>
    33<head>
     4<script src="../../resources/js-test.js"></script>
    45<script>
    5 if (window.testRunner) {
    6     testRunner.dumpAsText();
     6if (window.testRunner)
    77    testRunner.dumpFrameLoadCallbacks();
    8     testRunner.waitUntilDone();
    9 }
     8jsTestIsAsync = true;
    109</script>
    1110</head>
    1211<body>
    13 <p>Tests that we do not redirect to an invalid URL initiated by JavaScript. This test PASSED if you see an entry in the dumped frame load callbacks of the form: &quot;willPerformClientRedirectToURL: http://A=a%B=b&quot; followed by &quot;didCancelClientRedirectForFrame&quot;.</p>
    14 <p>Note, this test must be run in DumpRenderTree.</p>
    1512<script>
    16 window.location.href = "http://A=a%B=b";
    17 window.setTimeout(function() {
    18     if (window.testRunner)
    19         testRunner.notifyDone();
    20 }, 0);
     13description("Tests that we do not redirect to an invalid URL initiated by JavaScript. This test PASSED if you see an entry in the dumped frame load callbacks of the form: &quot;willPerformClientRedirectToURL: http://A=a%B=b&quot; followed by &quot;didCancelClientRedirectForFrame&quot;.");
     14onload = function() {
     15    window.location.href = "http://A=a%B=b";
     16    shouldBecomeEqual("testRunner.didCancelClientRedirect", "true", finishJSTest);
     17}
    2118</script>
    2219</body>

  • trunk/LayoutTests/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates-expected.txt

    r229340 r229341  
    66Tests that we do not redirect to an invalid URL initiated by <meta http-equiv="refresh">. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    77
    8 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/LayoutTests/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-expected.txt

    r207162 r229341  
    66Tests that we do not redirect to an invalid URL initiated by <meta http-equiv="refresh">. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    77
    8 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/LayoutTests/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed.html

    r207162 r229341  
    33<head>
    44<meta http-equiv="refresh" content="0; url=http://A=a%B=b">
     5<script src="../../resources/js-test.js"></script>
    56<script>
    6 if (window.testRunner) {
    7     testRunner.dumpAsText();
     7if (window.testRunner)
    88    testRunner.dumpFrameLoadCallbacks();
    9     testRunner.waitUntilDone();
    10 }
     9jsTestIsAsync = true;
    1110</script>
    1211</head>
    1312<body>
    14 <p>Tests that we do not redirect to an invalid URL initiated by &lt;meta http-equiv=&quot;refresh&quot;&gt;. This test PASSED if you see an entry in the dumped frame load callbacks of the form: &quot;willPerformClientRedirectToURL: http://A=a%B=b&quot; followed by &quot;didCancelClientRedirectForFrame&quot;.</p>
    15 <p>Note, this test must be run in DumpRenderTree.</p>
    1613<script>
    17 // This ugly double-timeout ensures that the scheduled meta-refresh, whose timer isn't even started until the frame finishes loading,
    18 // fires before notifyDone() is called, ensuring that didCancelClientRedirectForFrame is logged by DumpRenderTree.
    19 window.setTimeout(function() {
    20     window.setTimeout(function() {
    21         if (window.testRunner)
    22             testRunner.notifyDone();
    23     }, 0);
    24 }, 0);
     14description("Tests that we do not redirect to an invalid URL initiated by &lt;meta http-equiv=&quot;refresh&quot;&gt;. This test PASSED if you see an entry in the dumped frame load callbacks of the form: &quot;willPerformClientRedirectToURL: http://A=a%B=b&quot; followed by &quot;didCancelClientRedirectForFrame&quot;.");
     15
     16shouldBecomeEqual("testRunner.didCancelClientRedirect", "true", finishJSTest);
    2517</script>
    2618</body>

  • trunk/LayoutTests/fast/loader/window-open-to-invalid-url-disallowed-expected.txt

    r207162 r229341  
     1main frame - didFinishDocumentLoadForFrame
    12main frame - willPerformClientRedirectToURL: http://A=a%B=b
    2 main frame - didFinishDocumentLoadForFrame
     3main frame - didHandleOnloadEventsForFrame
    34main frame - didFinishLoadForFrame
    45main frame - didCancelClientRedirectForFrame
    56Tests that we do not open a new window to an invalid URL. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    67
    7 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS win.testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/LayoutTests/fast/loader/window-open-to-invalid-url-disallowed.html

    r207162 r229341  
    22<html>
    33<head>
     4<script src="../../resources/js-test.js"></script>
    45<script>
    56if (window.testRunner) {
    6     testRunner.dumpAsText();
    77    testRunner.dumpFrameLoadCallbacks();
    8     testRunner.waitUntilDone();
    98    testRunner.setCanOpenWindows();
    109}
     10jsTestIsAsync = true;
    1111</script>
    1212</head>
    1313<body>
    14 <p>Tests that we do not open a new window to an invalid URL. This test PASSED if you see an entry in the dumped frame load callbacks of the form: &quot;willPerformClientRedirectToURL: http://A=a%B=b&quot; followed by &quot;didCancelClientRedirectForFrame&quot;.</p>
    15 <p>Note, this test must be run in DumpRenderTree.</p>
    1614<script>
    17 window.open("http://A=a%B=b", "_top");
    18 window.setTimeout(function() {
    19     if (window.testRunner)
    20         testRunner.notifyDone();
    21 }, 0);
     15description("Tests that we do not open a new window to an invalid URL. This test PASSED if you see an entry in the dumped frame load callbacks of the form: &quot;willPerformClientRedirectToURL: http://A=a%B=b&quot; followed by &quot;didCancelClientRedirectForFrame&quot;.");
     16onload = function() {
     17    win = window.open("http://A=a%B=b", "_top");
     18    shouldBecomeEqual("win.testRunner.didCancelClientRedirect", "true", finishJSTest);
     19};
    2220</script>
    2321</body>

  • trunk/LayoutTests/platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-javascript-disallowed-expected.txt

    r207162 r229341  
     1main frame - didFinishDocumentLoadForFrame
    12main frame - willPerformClientRedirectToURL: http://A=a%25B=b
    2 main frame - didFinishDocumentLoadForFrame
     3main frame - didHandleOnloadEventsForFrame
    34main frame - didFinishLoadForFrame
    45main frame - didCancelClientRedirectForFrame
    56Tests that we do not redirect to an invalid URL initiated by JavaScript. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    67
    7 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/LayoutTests/platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates-expected.txt

    r229340 r229341  
    66Tests that we do not redirect to an invalid URL initiated by <meta http-equiv="refresh">. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    77
    8 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/LayoutTests/platform/mac-wk1/fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-expected.txt

    r207162 r229341  
    66Tests that we do not redirect to an invalid URL initiated by <meta http-equiv="refresh">. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    77
    8 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/LayoutTests/platform/mac-wk1/fast/loader/window-open-to-invalid-url-disallowed-expected.txt

    r207162 r229341  
     1main frame - didFinishDocumentLoadForFrame
    12main frame - willPerformClientRedirectToURL: http://A=a%25B=b
    2 main frame - didFinishDocumentLoadForFrame
     3main frame - didHandleOnloadEventsForFrame
    34main frame - didFinishLoadForFrame
    45main frame - didCancelClientRedirectForFrame
    56Tests that we do not open a new window to an invalid URL. This test PASSED if you see an entry in the dumped frame load callbacks of the form: "willPerformClientRedirectToURL: http://A=a%B=b" followed by "didCancelClientRedirectForFrame".
    67
    7 Note, this test must be run in DumpRenderTree.
     8On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     9
     10
     11PASS win.testRunner.didCancelClientRedirect became true
     12PASS successfullyParsed is true
     13
     14TEST COMPLETE
     15

  • trunk/Source/WebCore/ChangeLog

    r229340 r229341  
     12018-03-06  Chris Dumez  <cdumez@apple.com>
     2
     3        fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed.html fails with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183345
     5
     6        Reviewed by Alex Christensen.
     7
     8        FrameLoader::loadURL() was calling loadWithNavigationAction() and then resetting the
     9        m_quickRedirectComing flag right after. This works if the navigation policy decision
     10        triggered by loadWithNavigationAction() is made synchronously. However, when it is
     11        made asynchronously, the flag gets reset too early, before the policy decision
     12        handler has been called. This is an issue because the policy decision handler
     13        relies on the m_quickRedirectComing flag.
     14
     15        Similarly, FrameLoader::loadFrameRequest() was calling loadPostRequest() / loadURL()
     16        and then focusing a frame right after. This does not work as intended when the navigation
     17        policy decision is made asynchronously.
     18
     19        To address the issue, we now pass a completion handler that gets called when the operation
     20        has actually completion, after the policy decision has been made. This maintains the
     21        behavior in place with synchronous policy delegates.
     22
     23        Test: fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed-async-delegates.html
     24
     25        * loader/FrameLoader.cpp:
     26        (WebCore::FrameLoader::loadURLIntoChildFrame):
     27        (WebCore::FrameLoader::loadFrameRequest):
     28        (WebCore::FrameLoader::loadURL):
     29        (WebCore::FrameLoader::loadWithNavigationAction):
     30        (WebCore::FrameLoader::load):
     31        (WebCore::FrameLoader::loadWithDocumentLoader):
     32        (WebCore::FrameLoader::reloadWithOverrideEncoding):
     33        (WebCore::FrameLoader::reload):
     34        (WebCore::FrameLoader::loadPostRequest):
     35        (WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
     36        (WebCore::FrameLoader::loadDifferentDocumentItem):
     37        * loader/FrameLoader.h:
     38
    1392018-03-06  Antoine Quint  <graouts@apple.com>
    240

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r229304 r229341  
    936936
    937937    FrameLoadRequest frameLoadRequest { *m_frame.document(), m_frame.document()->securityOrigin(), { url }, ASCIILiteral("_self"), LockHistory::No, LockBackForwardList::Yes, ShouldSendReferrer::MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress, ShouldOpenExternalURLsPolicy::ShouldNotAllow, initiatedByMainFrame };
    938     childFrame->loader().loadURL(WTFMove(frameLoadRequest), referer, FrameLoadType::RedirectWithLockedBackForwardList, nullptr, nullptr);
     938    childFrame->loader().loadURL(WTFMove(frameLoadRequest), referer, FrameLoadType::RedirectWithLockedBackForwardList, nullptr, nullptr, [] { });
    939939}
    940940
    … …  
    12011201        loadType = FrameLoadType::Standard;
    12021202
    1203     String frameName = request.frameName();
     1203    auto completionHandler = [this, protectedFrame = makeRef(m_frame), formState = makeRefPtr(formState), frameName = request.frameName()] {
     1204        // FIXME: It's possible this targetFrame will not be the same frame that was targeted by the actual
     1205        // load if frame names have changed.
     1206        Frame* sourceFrame = formState ? formState->sourceDocument().frame() : &m_frame;
     1207        if (!sourceFrame)
     1208            sourceFrame = &m_frame;
     1209        Frame* targetFrame = sourceFrame->loader().findFrameForNavigation(frameName);
     1210        if (targetFrame && targetFrame != sourceFrame) {
     1211            if (Page* page = targetFrame->page())
     1212                page->chrome().focus();
     1213        }
     1214    };
     1215
    12041216    if (request.resourceRequest().httpMethod() == "POST")
    1205         loadPostRequest(WTFMove(request), referrer, loadType, event, formState);
     1217        loadPostRequest(WTFMove(request), referrer, loadType, event, formState, WTFMove(completionHandler));
    12061218    else
    1207         loadURL(WTFMove(request), referrer, loadType, event, formState);
    1208 
    1209     // FIXME: It's possible this targetFrame will not be the same frame that was targeted by the actual
    1210     // load if frame names have changed.
    1211     Frame* sourceFrame = formState ? formState->sourceDocument().frame() : &m_frame;
    1212     if (!sourceFrame)
    1213         sourceFrame = &m_frame;
    1214     Frame* targetFrame = sourceFrame->loader().findFrameForNavigation(frameName);
    1215     if (targetFrame && targetFrame != sourceFrame) {
    1216         if (Page* page = targetFrame->page())
    1217             page->chrome().focus();
    1218     }
     1219        loadURL(WTFMove(request), referrer, loadType, event, formState, WTFMove(completionHandler));
    12191220}
    12201221
    … …  
    12621263};
    12631264
    1264 void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& referrer, FrameLoadType newLoadType, Event* event, FormState* formState)
    1265 {
     1265void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& referrer, FrameLoadType newLoadType, Event* event, FormState* formState, CompletionHandler<void()>&& completionHandler)
     1266{
     1267    CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
    12661268    if (m_inStopAllLoaders)
    12671269        return;
    … …  
    12951297    if (targetFrame && targetFrame != &m_frame) {
    12961298        frameLoadRequest.setFrameName("_self");
    1297         targetFrame->loader().loadURL(WTFMove(frameLoadRequest), referrer, newLoadType, event, formState);
     1299        targetFrame->loader().loadURL(WTFMove(frameLoadRequest), referrer, newLoadType, event, formState, completionHandlerCaller.release());
    12981300        return;
    12991301    }
    … …  
    13061308    if (!targetFrame && !frameName.isEmpty()) {
    13071309        action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest));
    1308         policyChecker().checkNewWindowPolicy(WTFMove(action), request, formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
     1310        policyChecker().checkNewWindowPolicy(WTFMove(action), request, formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
    13091311            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
     1312            completionHandler();
    13101313        });
    13111314        return;
    … …  
    13411344    // must grab this now, since this load may stop the previous load and clear this flag
    13421345    bool isRedirect = m_quickRedirectComing;
    1343     loadWithNavigationAction(request, action, lockHistory, newLoadType, formState, allowNavigationToInvalidURL);
    1344     if (isRedirect) {
    1345         m_quickRedirectComing = false;
    1346         if (m_provisionalDocumentLoader)
    1347             m_provisionalDocumentLoader->setIsClientRedirect(true);
    1348         else if (m_policyDocumentLoader)
    1349             m_policyDocumentLoader->setIsClientRedirect(true);
    1350     } else if (sameURL && !isReload(newLoadType)) {
    1351         // Example of this case are sites that reload the same URL with a different cookie
    1352         // driving the generated content, or a master frame with links that drive a target
    1353         // frame, where the user has clicked on the same link repeatedly.
    1354         m_loadType = FrameLoadType::Same;
    1355     }
     1346    loadWithNavigationAction(request, action, lockHistory, newLoadType, formState, allowNavigationToInvalidURL, [this, isRedirect, sameURL, newLoadType, protectedFrame = makeRef(m_frame), completionHandler = completionHandlerCaller.release()] {
     1347        if (isRedirect) {
     1348            m_quickRedirectComing = false;
     1349            if (m_provisionalDocumentLoader)
     1350                m_provisionalDocumentLoader->setIsClientRedirect(true);
     1351            else if (m_policyDocumentLoader)
     1352                m_policyDocumentLoader->setIsClientRedirect(true);
     1353        } else if (sameURL && !isReload(newLoadType)) {
     1354            // Example of this case are sites that reload the same URL with a different cookie
     1355            // driving the generated content, or a master frame with links that drive a target
     1356            // frame, where the user has clicked on the same link repeatedly.
     1357            m_loadType = FrameLoadType::Same;
     1358        }
     1359        completionHandler();
     1360    });
    13561361}
    13571362
    … …  
    14021407}
    14031408
    1404 void FrameLoader::loadWithNavigationAction(const ResourceRequest& request, const NavigationAction& action, LockHistory lockHistory, FrameLoadType type, FormState* formState, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
     1409void FrameLoader::loadWithNavigationAction(const ResourceRequest& request, const NavigationAction& action, LockHistory lockHistory, FrameLoadType type, FormState* formState, AllowNavigationToInvalidURL allowNavigationToInvalidURL, CompletionHandler<void()>&& completionHandler)
    14051410{
    14061411    Ref<DocumentLoader> loader = m_client.createDocumentLoader(request, defaultSubstituteDataForURL(request.url()));
    … …  
    14141419        loader->setOverrideEncoding(m_documentLoader->overrideEncoding());
    14151420
    1416     loadWithDocumentLoader(loader.ptr(), type, formState, allowNavigationToInvalidURL);
     1421    loadWithDocumentLoader(loader.ptr(), type, formState, allowNavigationToInvalidURL, WTFMove(completionHandler));
    14171422}
    14181423
    … …  
    14531458    }
    14541459
    1455     loadWithDocumentLoader(newDocumentLoader, type, 0, AllowNavigationToInvalidURL::Yes);
    1456 }
    1457 
    1458 void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType type, FormState* formState, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
     1460    loadWithDocumentLoader(newDocumentLoader, type, 0, AllowNavigationToInvalidURL::Yes, [] { });
     1461}
     1462
     1463void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType type, FormState* formState, AllowNavigationToInvalidURL allowNavigationToInvalidURL, CompletionHandler<void()>&& completionHandler)
    14591464{
    14601465    // Retain because dispatchBeforeLoadEvent may release the last reference to it.
    14611466    Ref<Frame> protect(m_frame);
     1467
     1468    CompletionHandlerCallingScope completionHandlerCaller(WTFMove(completionHandler));
    14621469
    14631470    ASSERT(m_client.hasWebView());
    … …  
    15321539    m_frame.navigationScheduler().cancel(true);
    15331540
    1534     policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
     1541    policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
    15351542        continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
     1543        completionHandler();
    15361544    });
    15371545}
    … …  
    16311639    loader->setOverrideEncoding(encoding);
    16321640
    1633     loadWithDocumentLoader(loader.ptr(), FrameLoadType::Reload, 0, AllowNavigationToInvalidURL::Yes);
     1641    loadWithDocumentLoader(loader.ptr(), FrameLoadType::Reload, 0, AllowNavigationToInvalidURL::Yes, [] { });
    16341642}
    16351643
    … …  
    16761684    };
    16771685   
    1678     loadWithDocumentLoader(loader.ptr(), frameLoadTypeForReloadOptions(options), 0, AllowNavigationToInvalidURL::Yes);
     1686    loadWithDocumentLoader(loader.ptr(), frameLoadTypeForReloadOptions(options), 0, AllowNavigationToInvalidURL::Yes, [] { });
    16791687}
    16801688
    … …  
    27482756}
    27492757
    2750 void FrameLoader::loadPostRequest(FrameLoadRequest&& request, const String& referrer, FrameLoadType loadType, Event* event, FormState* formState)
     2758void FrameLoader::loadPostRequest(FrameLoadRequest&& request, const String& referrer, FrameLoadType loadType, Event* event, FormState* formState, CompletionHandler<void()>&& completionHandler)
    27512759{
    27522760    String frameName = request.frameName();
    … …  
    27782786        // The search for a target frame is done earlier in the case of form submission.
    27792787        if (Frame* targetFrame = formState ? 0 : findFrameForNavigation(frameName)) {
    2780             targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL);
     2788            targetFrame->loader().loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, WTFMove(completionHandler));
    27812789            return;
    27822790        }
    27832791
    2784         policyChecker().checkNewWindowPolicy(WTFMove(action), workingResourceRequest, WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
     2792        policyChecker().checkNewWindowPolicy(WTFMove(action), workingResourceRequest, WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
    27852793            continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
     2794            completionHandler();
    27862795        });
    27872796        return;
    … …  
    27902799    // must grab this now, since this load may stop the previous load and clear this flag
    27912800    bool isRedirect = m_quickRedirectComing;
    2792     loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL);
    2793     if (isRedirect) {
    2794         m_quickRedirectComing = false;
    2795         if (m_provisionalDocumentLoader)
    2796             m_provisionalDocumentLoader->setIsClientRedirect(true);
    2797         else if (m_policyDocumentLoader)
    2798             m_policyDocumentLoader->setIsClientRedirect(true);
    2799     }
     2801    loadWithNavigationAction(workingResourceRequest, action, lockHistory, loadType, WTFMove(formState), allowNavigationToInvalidURL, [this, isRedirect, protectedFrame = makeRef(m_frame), completionHandler = WTFMove(completionHandler)] {
     2802        if (isRedirect) {
     2803            m_quickRedirectComing = false;
     2804            if (m_provisionalDocumentLoader)
     2805                m_provisionalDocumentLoader->setIsClientRedirect(true);
     2806            else if (m_policyDocumentLoader)
     2807                m_policyDocumentLoader->setIsClientRedirect(true);
     2808        }
     2809        completionHandler();
     2810    });
    28002811}
    28012812
    … …  
    32593270
    32603271    NavigationAction newAction { *frame->document(), request, InitiatedByMainFrame::Unknown, NavigationType::Other, action.shouldOpenExternalURLsPolicy() };
    3261     mainFrame->loader().loadWithNavigationAction(request, newAction, LockHistory::No, FrameLoadType::Standard, formState, allowNavigationToInvalidURL);
     3272    mainFrame->loader().loadWithNavigationAction(request, newAction, LockHistory::No, FrameLoadType::Standard, formState, allowNavigationToInvalidURL, [] { });
    32623273}
    32633274
    … …  
    34493460        documentLoader->setTriggeringAction({ *m_frame.document(), documentLoader->request(), initiatedByMainFrame, loadType, false });
    34503461        documentLoader->setLastCheckedRequest(ResourceRequest());
    3451         loadWithDocumentLoader(documentLoader, loadType, 0, AllowNavigationToInvalidURL::Yes);
     3462        loadWithDocumentLoader(documentLoader, loadType, 0, AllowNavigationToInvalidURL::Yes, [] { });
    34523463        return;
    34533464    }
    … …  
    35353546    }
    35363547
    3537     loadWithNavigationAction(request, action, LockHistory::No, loadType, 0, AllowNavigationToInvalidURL::Yes);
     3548    loadWithNavigationAction(request, action, LockHistory::No, loadType, 0, AllowNavigationToInvalidURL::Yes, [] { });
    35383549}
    35393550

  • trunk/Source/WebCore/loader/FrameLoader.h

    r228942 r229341  
    360360    void urlSelected(FrameLoadRequest&&, Event*);
    361361
    362     void loadWithDocumentLoader(DocumentLoader*, FrameLoadType, FormState*, AllowNavigationToInvalidURL); // Calls continueLoadAfterNavigationPolicy
     362    void loadWithDocumentLoader(DocumentLoader*, FrameLoadType, FormState*, AllowNavigationToInvalidURL, CompletionHandler<void()>&&); // Calls continueLoadAfterNavigationPolicy
    363363    void load(DocumentLoader*); // Calls loadWithDocumentLoader
    364364
    365     void loadWithNavigationAction(const ResourceRequest&, const NavigationAction&, LockHistory, FrameLoadType, FormState*, AllowNavigationToInvalidURL); // Calls loadWithDocumentLoader
    366 
    367     void loadPostRequest(FrameLoadRequest&&, const String& referrer, FrameLoadType, Event*, FormState*);
    368     void loadURL(FrameLoadRequest&&, const String& referrer, FrameLoadType, Event*, FormState*);
     365    void loadWithNavigationAction(const ResourceRequest&, const NavigationAction&, LockHistory, FrameLoadType, FormState*, AllowNavigationToInvalidURL, CompletionHandler<void()>&&); // Calls loadWithDocumentLoader
     366
     367    void loadPostRequest(FrameLoadRequest&&, const String& referrer, FrameLoadType, Event*, FormState*, CompletionHandler<void()>&&);
     368    void loadURL(FrameLoadRequest&&, const String& referrer, FrameLoadType, Event*, FormState*, CompletionHandler<void()>&&);
    369369
    370370    bool shouldReload(const URL& currentURL, const URL& destinationURL);

  • trunk/Tools/ChangeLog

    r229325 r229341  
     12018-03-06  Chris Dumez  <cdumez@apple.com>
     2
     3        fast/loader/redirect-to-invalid-url-using-meta-refresh-disallowed.html fails with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183345
     5
     6        Reviewed by Alex Christensen.
     7
     8        Add layout test infrastructure so a test can know when didCancelClientRedirectForFrame has
     9        been called. The tests used to rely on a 0-timer but this does not work when the client
     10        responds to the navigation policy asynchronously.
     11
     12        * DumpRenderTree/TestRunner.cpp:
     13        (getDidCancelClientRedirect):
     14        (TestRunner::staticValues):
     15        * DumpRenderTree/TestRunner.h:
     16        (TestRunner::didCancelClientRedirect const):
     17        (TestRunner::setDidCancelClientRedirect):
     18        * DumpRenderTree/mac/FrameLoadDelegate.mm:
     19        (-[FrameLoadDelegate webView:didCancelClientRedirectForFrame:]):
     20        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
     21        * WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
     22        (WTR::InjectedBundlePage::didCancelClientRedirectForFrame):
     23        * WebKitTestRunner/InjectedBundle/TestRunner.h:
     24        (WTR::TestRunner::didCancelClientRedirect const):
     25        (WTR::TestRunner::setDidCancelClientRedirect):
     26
    1272018-03-06  Zan Dobersek  <zdobersek@igalia.com>
    228

  • trunk/Tools/DumpRenderTree/TestRunner.cpp

    r229143 r229341  
    18101810}
    18111811
     1812static JSValueRef getDidCancelClientRedirect(JSContextRef context, JSObjectRef thisObject, JSStringRef propertyName, JSValueRef* exception)
     1813{
     1814    TestRunner* controller = static_cast<TestRunner*>(JSObjectGetPrivate(thisObject));
     1815    return JSValueMakeBoolean(context, controller->didCancelClientRedirect());
     1816}
     1817
    18121818static JSValueRef getDatabaseDefaultQuotaCallback(JSContextRef context, JSObjectRef thisObject, JSStringRef propertyName, JSValueRef* exception)
    18131819{
    … …  
    20782084{
    20792085    static JSStaticValue staticValues[] = {
     2086        { "didCancelClientRedirect", getDidCancelClientRedirect, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
    20802087        { "timeout", getTimeoutCallback, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
    20812088        { "globalFlag", getGlobalFlagCallback, setGlobalFlagCallback, kJSPropertyAttributeNone },

  • trunk/Tools/DumpRenderTree/TestRunner.h

    r223211 r229341  
    381381    void setOpenPanelFiles(JSContextRef, JSValueRef);
    382382
     383    bool didCancelClientRedirect() const { return m_didCancelClientRedirect; }
     384    void setDidCancelClientRedirect(bool value) { m_didCancelClientRedirect = value; }
     385
    383386private:
    384387    TestRunner(const std::string& testURL, const std::string& expectedPixelHash);
    … …  
    446449    bool m_hasPendingWebNotificationClick;
    447450    bool m_dumpJSConsoleLogInStdErr { false };
     451    bool m_didCancelClientRedirect { false };
    448452
    449453    double m_databaseDefaultQuota;

  • trunk/Tools/DumpRenderTree/mac/FrameLoadDelegate.mm

    r210519 r229341  
    405405        printf ("%s\n", [string UTF8String]);
    406406    }
     407    gTestRunner->setDidCancelClientRedirect(true);
    407408}
    408409

  • trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl

    r229177 r229341  
    317317    void terminateServiceWorkerProcess();
    318318
     319    readonly attribute boolean didCancelClientRedirect;
     320
    319321    void removeAllSessionCredentials(object callback);
    320322

  • trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp

    r229304 r229341  
    10071007        return;
    10081008
    1009     if (!injectedBundle.testRunner()->shouldDumpFrameLoadCallbacks())
    1010         return;
    1011 
    1012     dumpLoadEvent(frame, "didCancelClientRedirectForFrame");
     1009    if (injectedBundle.testRunner()->shouldDumpFrameLoadCallbacks())
     1010        dumpLoadEvent(frame, "didCancelClientRedirectForFrame");
     1011
     1012    injectedBundle.testRunner()->setDidCancelClientRedirect(true);
    10131013}
    10141014

  • trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h

    r229177 r229341  
    335335    void setIgnoresViewportScaleLimits(bool);
    336336    void setShouldDownloadUndisplayableMIMETypes(bool);
     337
     338    bool didCancelClientRedirect() const { return m_didCancelClientRedirect; }
     339    void setDidCancelClientRedirect(bool value) { m_didCancelClientRedirect = value; }
    337340
    338341    void runUIScript(JSStringRef script, JSValueRef callback);
    … …  
    480483    bool m_shouldDecideResponsePolicyAfterDelay { false };
    481484    bool m_shouldFinishAfterDownload { false };
     485    bool m_didCancelClientRedirect { false };
    482486
    483487    bool m_userStyleSheetEnabled;
Note: See TracChangeset for help on using the changeset viewer.