Changeset 229191 in webkit


Ignore:
Timestamp:
Mar 2, 2018 3:14:33 PM (6 years ago)
Author:
Chris Dumez
Message:

imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html crashes with async policy delegates
​https://bugs.webkit.org/show_bug.cgi?id=183294
<rdar://problem/38073596>

Reviewed by Youenn Fablet.

Source/WebCore:

Drop code that was added to SubresourceLoader::willCancel() in r228852. The purpose of this code
was to make sure that SubresourceLoader::m_policyForResponseCompletionHandler always gets called,
even when the load is cancelled. However, this code is not needed (since m_policyForResponseCompletionHandler
is a CompletionHandler, an assertion will be hit if we fail to call it and we'll know). Calling
the completionHandler inside SubresourceLoader::willCancel() is too early and leads to crashes.

The completionHandler currently gets called DocumentLoader::responseReceived() via a call to
mainResourceLoader->didReceiveResponsePolicy(). Note that in r229177, we made sure that the
call to didReceiveResponsePolicy() happens *after* the call to continueAfterContentPolicy()
to maintain our non-async policy delegate behavior. However, continueAfterContentPolicy()
would end up calling willCancel() and call the completionHandler when shouldContinue was
false.

Test: http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate.html

  • loader/SubresourceLoader.cpp:

(WebCore::SubresourceLoader::willCancel):

LayoutTests:

Add layout test coverage.

  • http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate-expected.txt: Added.
  • http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate.html: Added.
Location:
trunk
Files:
9 added
1 deleted
7 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r229179 r229191  
     12018-03-02  Chris Dumez  <cdumez@apple.com>
     2
     3        imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html crashes with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183294
     5        <rdar://problem/38073596>
     6
     7        Reviewed by Youenn Fablet.
     8
     9        Add layout test coverage.
     10
     11        * http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate-expected.txt: Added.
     12        * http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate.html: Added.
     13
    1142018-03-02  Chris Dumez  <cdumez@apple.com>
    215

  • trunk/LayoutTests/TestExpectations

    r229160 r229191  
    174174
    175175imported/w3c/web-platform-tests/service-workers/service-worker/websocket.https.html [ Pass Failure ]
     176
     177# Console log lines may appear in a different order so we silence them.
     178http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate.html [ DumpJSConsoleLogInStdErr ]
     179imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html [ DumpJSConsoleLogInStdErr ]
     180imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html [ DumpJSConsoleLogInStdErr ]
    176181
    177182webkit.org/b/181901 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-cors-xhr.https.html [ DumpJSConsoleLogInStdErr ]
    … …  
    614619imported/w3c/web-platform-tests/fetch/http-cache/partial.html [ Failure ]
    615620webkit.org/b/159724 imported/w3c/web-platform-tests/XMLHttpRequest/send-redirect-post-upload.htm [ Failure Pass ]
    616 webkit.org/b/167380 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html [ Failure Pass ]
    617621
    618622# Flaky tests due to always changing assertion error message

  • trunk/LayoutTests/http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate-expected.txt

    r229190 r229191  
    1 CONSOLE MESSAGE: Not allowed to load local resource: script%3E
    2 CONSOLE MESSAGE: Not allowed to load local resource: blank.html
    31
    42PASS Set HTTP URL frame location.protocol to x
    53PASS Set data URL frame location.protocol to x
    64PASS Set HTTP URL frame location.protocol to data
    7 FAIL Set data URL frame location.protocol to data The object can not be cloned.
     5PASS Set data URL frame location.protocol to data
    86PASS Set HTTP URL frame location.protocol to file
    97PASS Set data URL frame location.protocol to file

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-expected.txt

    r212202 r229191  
    1 CONSOLE MESSAGE: Not allowed to load local resource: script%3E
    2 CONSOLE MESSAGE: Not allowed to load local resource: blank.html
    31
    42PASS Set HTTP URL frame location.protocol to x

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird-expected.txt

    r210823 r229191  
    1 CONSOLE MESSAGE: Not allowed to load local resource: blank.html
    21
    32PASS Set location.protocol to x

  • trunk/LayoutTests/platform/ios/TestExpectations

    r229147 r229191  
    28782878quicklook/password-protected.html [ Skip ]
    28792879
    2880 webkit.org/b/167376 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html [ Pass Failure ]
    2881 webkit.org/b/167211 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html [ Pass Failure ]
    2882 
    28832880webkit.org/b/167619 css3/filters/backdrop/dynamic-with-clip-path.html [ ImageOnlyFailure ]
    28842881

  • trunk/LayoutTests/platform/mac-wk1/http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate-expected.txt

    r229190 r229191  
    1 CONSOLE MESSAGE: Not allowed to load local resource: script%3E
    2 CONSOLE MESSAGE: Not allowed to load local resource: blank.html
    31
    42PASS Set HTTP URL frame location.protocol to x

  • trunk/Source/WebCore/ChangeLog

    r229190 r229191  
     12018-03-02  Chris Dumez  <cdumez@apple.com>
     2
     3        imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html crashes with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183294
     5        <rdar://problem/38073596>
     6
     7        Reviewed by Youenn Fablet.
     8
     9        Drop code that was added to SubresourceLoader::willCancel() in r228852. The purpose of this code
     10        was to make sure that SubresourceLoader::m_policyForResponseCompletionHandler always gets called,
     11        even when the load is cancelled. However, this code is not needed (since m_policyForResponseCompletionHandler
     12        is a CompletionHandler, an assertion will be hit if we fail to call it and we'll know). Calling
     13        the completionHandler inside SubresourceLoader::willCancel() is too early and leads to crashes.
     14
     15        The completionHandler currently gets called DocumentLoader::responseReceived() via a call to
     16        mainResourceLoader->didReceiveResponsePolicy(). Note that in r229177, we made sure that the
     17        call to didReceiveResponsePolicy() happens *after* the call to continueAfterContentPolicy()
     18        to maintain our non-async policy delegate behavior. However, continueAfterContentPolicy()
     19        would end up calling willCancel() and call the completionHandler when shouldContinue was
     20        false.
     21
     22        Test: http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate.html
     23
     24        * loader/SubresourceLoader.cpp:
     25        (WebCore::SubresourceLoader::willCancel):
     26
    1272018-03-02  Tim Horton  <timothy_horton@apple.com>
    228

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

    r228852 r229191  
    675675    LOG(ResourceLoading, "Cancelled load of '%s'.\n", m_resource->url().string().latin1().data());
    676676
    677     if (auto policyForResponseCompletionHandler = WTFMove(m_policyForResponseCompletionHandler))
    678         policyForResponseCompletionHandler();
    679 
    680677    Ref<SubresourceLoader> protectedThis(*this);
    681678#if PLATFORM(IOS)
Note: See TracChangeset for help on using the changeset viewer.