Changeset 229108 in webkit


Ignore:
Timestamp:
Feb 28, 2018 3:58:22 PM (6 years ago)
Author:
Chris Dumez
Message:

html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
​https://bugs.webkit.org/show_bug.cgi?id=183168
<rdar://problem/37951341>

Reviewed by Alex Christensen.

Source/WebCore:

The test has an anchor element with both a 'click' event handler which submits a form
and an href attribute. When clicking the link, as per specification, things happen in
this order:

  1. We fire the click event at the anchor, which will execute the event handler and submit the form. Submitting the form *schedules* a navigation to 'click.html'.
  2. We execute the anchor activation code which *navigates* to 'href.html'. The navigation to 'href' is supposed to cancel the pending navigation to 'click.html' and we should navigate to 'href.html', which is what the test asserts.

The issue for us is that we do not cancel pending navigations until after the navigation
policy decision is made, when the provisional loads actually starts, in FrameLoader::provisionalLoadStarted().
Because the policy decision for the navigation can now be made asynchronously, the NavigationScheduler
timer can now fire while the decision is made and we'll submit the form, thus navigating to
'click.html'.

To address the issue, we now cancel any pending navigations in FrameLoader::loadWithDocumentLoader(),
*before* doing the policy check for the navigation.

Test: http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::loadWithDocumentLoader):

LayoutTests:

Add layout test coverage.

  • TestExpectations:
  • http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt: Added.
  • http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html: Added.
  • http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html: Added.
  • http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html: Added.
Location:
trunk
Files:
8 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r229107 r229108  
     12018-02-28  Chris Dumez  <cdumez@apple.com>
     2
     3        html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183168
     5        <rdar://problem/37951341>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Add layout test coverage.
     10
     11        * TestExpectations:
     12        * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt: Added.
     13        * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html: Added.
     14        * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html: Added.
     15        * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html: Added.
     16
    1172018-02-28  Alicia Boya García  <aboya@igalia.com>
    218

  • trunk/LayoutTests/TestExpectations

    r229082 r229108  
    289289# This is a resources folder.
    290290http/tests/workers/service/other_resources [ Skip ]
     291
     292# These are resource files.
     293http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html
     294http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html
    291295
    292296# We fail this reftest

  • trunk/Source/WebCore/ChangeLog

    r229106 r229108  
     12018-02-28  Chris Dumez  <cdumez@apple.com>
     2
     3        html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183168
     5        <rdar://problem/37951341>
     6
     7        Reviewed by Alex Christensen.
     8
     9        The test has an anchor element with both a 'click' event handler which submits a form
     10        and an href attribute. When clicking the link, as per specification, things happen in
     11        this order:
     12        1. We fire the click event at the anchor, which will execute the event handler and submit the form.
     13           Submitting the form *schedules* a navigation to 'click.html'.
     14        2. We execute the anchor activation code which *navigates* to 'href.html'. The navigation to
     15           'href' is supposed to cancel the pending navigation to 'click.html' and we should navigate
     16           to 'href.html', which is what the test asserts.
     17
     18        The issue for us is that we do not cancel pending navigations until after the navigation
     19        policy decision is made, when the provisional loads actually starts, in FrameLoader::provisionalLoadStarted().
     20        Because the policy decision for the navigation can now be made asynchronously, the NavigationScheduler
     21        timer can now fire while the decision is made and we'll submit the form, thus navigating to
     22        'click.html'.
     23
     24        To address the issue, we now cancel any pending navigations in FrameLoader::loadWithDocumentLoader(),
     25        *before* doing the policy check for the navigation.
     26
     27        Test: http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html
     28
     29        * loader/FrameLoader.cpp:
     30        (WebCore::FrameLoader::loadWithDocumentLoader):
     31
    1322018-02-28  John Wilander  <wilander@apple.com>
    233

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

    r229100 r229108  
    15311531    }
    15321532
     1533    m_frame.navigationScheduler().cancel(true);
     1534
    15331535    policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, allowNavigationToInvalidURL] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
    15341536        continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
Note: See TracChangeset for help on using the changeset viewer.