Changeset 267227 in webkit


Ignore:
Timestamp:
Sep 18, 2020 1:15:32 AM (4 years ago)
Author:
youenn@apple.com
Message:

XHR.timeout is affected by long tasks
​https://bugs.webkit.org/show_bug.cgi?id=216266
<rdar://problem/68908150>

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

  • web-platform-tests/xhr/xhr-timeout-longtask.any-expected.txt: Added.
  • web-platform-tests/xhr/xhr-timeout-longtask.any.html: Added.
  • web-platform-tests/xhr/xhr-timeout-longtask.any.js: Added.
  • web-platform-tests/xhr/xhr-timeout-longtask.any.worker-expected.txt: Added.
  • web-platform-tests/xhr/xhr-timeout-longtask.any.worker.html: Added.

Source/WebCore:

Long tasks may block the main thread, which may block IPC processing of load messages.
In that case, even though the load is finished, WebProcess did not know that yet and will cancel the load.
To prevent that, in case of XHR timeout, do an explicit check to compute the done flag.
​https://fetch.spec.whatwg.org/#done-flag

Tests: imported/w3c/web-platform-tests/xhr/xhr-timeout-longtask.any.html

imported/w3c/web-platform-tests/xhr/xhr-timeout-longtask.any.worker.html

  • loader/DocumentThreadableLoader.cpp:

(WebCore::DocumentThreadableLoader::computeIsDone):

  • loader/DocumentThreadableLoader.h:
  • loader/LoaderStrategy.h:
  • loader/ThreadableLoader.h:
  • loader/ThreadableLoaderClient.h:

(WebCore::ThreadableLoaderClient::notifyIsDone):

  • loader/ThreadableLoaderClientWrapper.h:

(WebCore::ThreadableLoaderClientWrapper::notifyIsDone):

  • loader/WorkerThreadableLoader.cpp:

(WebCore::WorkerThreadableLoader::computeIsDone):
(WebCore::WorkerThreadableLoader::MainThreadBridge::computeIsDone):
(WebCore::WorkerThreadableLoader::MainThreadBridge::notifyIsDone):

  • loader/WorkerThreadableLoader.h:

(WebCore::WorkerThreadableLoader::MainThreadBridge::loaderProxy):

  • xml/XMLHttpRequest.cpp:

(WebCore::XMLHttpRequest::didReachTimeout):
(WebCore::XMLHttpRequest::notifyIsDone):

  • xml/XMLHttpRequest.h:

Source/WebKit:

Go to network process to know whether a load is finished or not.

  • NetworkProcess/NetworkConnectionToWebProcess.cpp:

(WebKit::NetworkConnectionToWebProcess::isResourceLoadFinished):

  • NetworkProcess/NetworkConnectionToWebProcess.h:
  • NetworkProcess/NetworkConnectionToWebProcess.messages.in:
  • WebProcess/Network/WebLoaderStrategy.cpp:

(WebKit::WebLoaderStrategy::isResourceLoadFinished):

  • WebProcess/Network/WebLoaderStrategy.h:

Source/WebKitLegacy:

  • WebCoreSupport/WebResourceLoadScheduler.cpp:

(WebResourceLoadScheduler::isResourceLoadFinished):

  • WebCoreSupport/WebResourceLoadScheduler.h:

LayoutTests:

platform/mac-wk1/TestExpectations: Skip WK1 test.

Location:
trunk
Files:
5 added
23 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r267220 r267227  
     12020-09-18  Youenn Fablet  <youenn@apple.com>
     2
     3        XHR.timeout is affected by long tasks
     4        https://bugs.webkit.org/show_bug.cgi?id=216266
     5        <rdar://problem/68908150>
     6
     7        Reviewed by Alex Christensen.
     8
     9        platform/mac-wk1/TestExpectations: Skip WK1 test.
     10
    1112020-09-16  Darin Adler  <darin@apple.com>
    212

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r267218 r267227  
     12020-09-18  Youenn Fablet  <youenn@apple.com>
     2
     3        XHR.timeout is affected by long tasks
     4        https://bugs.webkit.org/show_bug.cgi?id=216266
     5        <rdar://problem/68908150>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * web-platform-tests/xhr/xhr-timeout-longtask.any-expected.txt: Added.
     10        * web-platform-tests/xhr/xhr-timeout-longtask.any.html: Added.
     11        * web-platform-tests/xhr/xhr-timeout-longtask.any.js: Added.
     12        * web-platform-tests/xhr/xhr-timeout-longtask.any.worker-expected.txt: Added.
     13        * web-platform-tests/xhr/xhr-timeout-longtask.any.worker.html: Added.
     14
    1152020-09-17  Chris Dumez  <cdumez@apple.com>
    216

  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r267168 r267227  
    270270# WK1 does not support sync XHR redirections as does WK2
    271271http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-insecure-sync-xhr-in-main-frame.html [ Skip ]
     272# WK1 networking is blocked by JS
     273imported/w3c/web-platform-tests/xhr/xhr-timeout-longtask.any.html
    272274
    273275webkit.org/b/187230 editing/mac/pasteboard/can-copy-url-without-title.html [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r267222 r267227  
     12020-09-18  Youenn Fablet  <youenn@apple.com>
     2
     3        XHR.timeout is affected by long tasks
     4        https://bugs.webkit.org/show_bug.cgi?id=216266
     5        <rdar://problem/68908150>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Long tasks may block the main thread, which may block IPC processing of load messages.
     10        In that case, even though the load is finished, WebProcess did not know that yet and will cancel the load.
     11        To prevent that, in case of XHR timeout, do an explicit check to compute the done flag.
     12        https://fetch.spec.whatwg.org/#done-flag
     13
     14        Tests: imported/w3c/web-platform-tests/xhr/xhr-timeout-longtask.any.html
     15               imported/w3c/web-platform-tests/xhr/xhr-timeout-longtask.any.worker.html
     16
     17        * loader/DocumentThreadableLoader.cpp:
     18        (WebCore::DocumentThreadableLoader::computeIsDone):
     19        * loader/DocumentThreadableLoader.h:
     20        * loader/LoaderStrategy.h:
     21        * loader/ThreadableLoader.h:
     22        * loader/ThreadableLoaderClient.h:
     23        (WebCore::ThreadableLoaderClient::notifyIsDone):
     24        * loader/ThreadableLoaderClientWrapper.h:
     25        (WebCore::ThreadableLoaderClientWrapper::notifyIsDone):
     26        * loader/WorkerThreadableLoader.cpp:
     27        (WebCore::WorkerThreadableLoader::computeIsDone):
     28        (WebCore::WorkerThreadableLoader::MainThreadBridge::computeIsDone):
     29        (WebCore::WorkerThreadableLoader::MainThreadBridge::notifyIsDone):
     30        * loader/WorkerThreadableLoader.h:
     31        (WebCore::WorkerThreadableLoader::MainThreadBridge::loaderProxy):
     32        * xml/XMLHttpRequest.cpp:
     33        (WebCore::XMLHttpRequest::didReachTimeout):
     34        (WebCore::XMLHttpRequest::notifyIsDone):
     35        * xml/XMLHttpRequest.h:
     36
    1372020-09-17  Sam Weinig  <weinig@apple.com>
    238

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

    r267222 r267227  
    269269}
    270270
     271void DocumentThreadableLoader::computeIsDone()
     272{
     273    if (!m_async || m_preflightChecker || !m_resource) {
     274        if (m_client)
     275            m_client->notifyIsDone(m_async && !m_preflightChecker && !m_resource);
     276        return;
     277    }
     278    platformStrategies()->loaderStrategy()->isResourceLoadFinished(*m_resource, [this, protectedThis = makeRef(*this)](bool isDone) {
     279        if (m_client)
     280            m_client->notifyIsDone(isDone);
     281    });
     282}
     283
    271284void DocumentThreadableLoader::setDefersLoading(bool value)
    272285{

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

    r261597 r267227  
    5858        void cancel() override;
    5959        virtual void setDefersLoading(bool);
     60        void computeIsDone() final;
    6061
    6162        friend CrossOriginPreflightChecker;

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

    r257206 r267227  
    8989    virtual NetworkLoadMetrics networkMetricsFromResourceLoadIdentifier(uint64_t resourceLoadIdentifier);
    9090
     91    virtual void isResourceLoadFinished(CachedResource&, CompletionHandler<void(bool)>&& callback) = 0;
     92
    9193    // Used for testing only.
    9294    virtual Vector<uint64_t> ongoingLoads() const { return { }; }

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

    r246490 r267227  
    7777        static RefPtr<ThreadableLoader> create(ScriptExecutionContext&, ThreadableLoaderClient&, ResourceRequest&&, const ThreadableLoaderOptions&, String&& referrer = String());
    7878
     79        virtual void computeIsDone() = 0;
    7980        virtual void cancel() = 0;
    8081        void ref() { refThreadableLoader(); }

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

    r223728 r267227  
    4747        virtual void didFail(const ResourceError&) { }
    4848        virtual void didFinishTiming(const ResourceTiming&) { }
     49        virtual void notifyIsDone(bool) { ASSERT_NOT_REACHED(); }
    4950
    5051    protected:

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

    r228545 r267227  
    8080    }
    8181
     82    void notifyIsDone(bool isDone)
     83    {
     84        if (m_client)
     85            m_client->notifyIsDone(isDone);
     86    }
     87
    8288    void didFail(const ResourceError& error)
    8389    {

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

    r248846 r267227  
    8686}
    8787
     88void WorkerThreadableLoader::computeIsDone()
     89{
     90    m_bridge.computeIsDone();
     91}
     92
    8893struct LoaderTaskOptions {
    8994    WTF_MAKE_STRUCT_FAST_ALLOCATED;
    … …  
    183188}
    184189
     190void WorkerThreadableLoader::MainThreadBridge::computeIsDone()
     191{
     192    m_loaderProxy.postTaskToLoader([this](auto&) {
     193        if (!m_mainThreadLoader) {
     194            notifyIsDone(true);
     195            return;
     196        }
     197        m_mainThreadLoader->computeIsDone();
     198    });
     199}
     200
     201void WorkerThreadableLoader::MainThreadBridge::notifyIsDone(bool isDone)
     202{
     203    m_loaderProxy.postTaskForModeToWorkerGlobalScope([protectedWorkerClientWrapper = makeRef(*m_workerClientWrapper), isDone](auto&) {
     204        protectedWorkerClientWrapper->notifyIsDone(isDone);
     205    }, m_taskMode);
     206}
     207
    185208void WorkerThreadableLoader::MainThreadBridge::clearClientWrapper()
    186209{

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

    r231813 r267227  
    6161        bool done() const { return m_workerClientWrapper->done(); }
    6262
     63        void notifyIsDone(bool isDone);
     64
    6365        using RefCounted<WorkerThreadableLoader>::ref;
    6466        using RefCounted<WorkerThreadableLoader>::deref;
    … …  
    9395            void cancel();
    9496            void destroy();
     97            void computeIsDone();
    9598
    9699        private:
    … …  
    105108            void didFail(const ResourceError&) override;
    106109            void didFinishTiming(const ResourceTiming&) override;
     110            void notifyIsDone(bool isDone) final;
    107111
    108112            // Only to be used on the main thread.
    … …  
    125129        WorkerThreadableLoader(WorkerGlobalScope&, ThreadableLoaderClient&, const String& taskMode, ResourceRequest&&, const ThreadableLoaderOptions&, const String& referrer);
    126130
     131        void computeIsDone() final;
     132
    127133        Ref<WorkerGlobalScope> m_workerGlobalScope;
    128134        Ref<ThreadableLoaderClientWrapper> m_workerClientWrapper;

  • trunk/Source/WebCore/xml/XMLHttpRequest.cpp

    r266168 r267227  
    119119    , m_responseType(static_cast<unsigned>(ResponseType::EmptyString))
    120120    , m_progressEventThrottle(*this)
    121     , m_timeoutTimer(*this, &XMLHttpRequest::didReachTimeout)
     121    , m_timeoutTimer(*this, &XMLHttpRequest::timeoutTimerFired)
    122122{
    123123#ifndef NDEBUG
    … …  
    11041104}
    11051105
     1106void XMLHttpRequest::timeoutTimerFired()
     1107{
     1108    if (!m_loadingActivity)
     1109        return;
     1110    m_loadingActivity->loader->computeIsDone();
     1111}
     1112
     1113void XMLHttpRequest::notifyIsDone(bool isDone)
     1114{
     1115    if (isDone)
     1116        return;
     1117    didReachTimeout();
     1118}
     1119
    11061120void XMLHttpRequest::didReachTimeout()
    11071121{

  • trunk/Source/WebCore/xml/XMLHttpRequest.h

    r264853 r267227  
    160160    void didFinishLoading(unsigned long identifier) override;
    161161    void didFail(const ResourceError&) override;
     162    void notifyIsDone(bool) final;
    162163
    163164    bool responseIsXML() const;
    … …  
    184185
    185186    ExceptionOr<void> createRequest();
     187
     188    void timeoutTimerFired();
    186189
    187190    void genericError();

  • trunk/Source/WebKit/ChangeLog

    r267226 r267227  
     12020-09-18  Youenn Fablet  <youenn@apple.com>
     2
     3        XHR.timeout is affected by long tasks
     4        https://bugs.webkit.org/show_bug.cgi?id=216266
     5        <rdar://problem/68908150>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Go to network process to know whether a load is finished or not.
     10
     11        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
     12        (WebKit::NetworkConnectionToWebProcess::isResourceLoadFinished):
     13        * NetworkProcess/NetworkConnectionToWebProcess.h:
     14        * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
     15        * WebProcess/Network/WebLoaderStrategy.cpp:
     16        (WebKit::WebLoaderStrategy::isResourceLoadFinished):
     17        * WebProcess/Network/WebLoaderStrategy.h:
     18
    1192020-09-18  Carlos Garcia Campos  <cgarcia@igalia.com>
    220

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

    r266467 r267227  
    594594}
    595595
     596void NetworkConnectionToWebProcess::isResourceLoadFinished(uint64_t loadIdentifier, CompletionHandler<void(bool)>&& callback)
     597{
     598    callback(!m_networkResourceLoaders.contains(loadIdentifier));
     599}
     600
    596601void NetworkConnectionToWebProcess::didFinishPreconnection(uint64_t preconnectionIdentifier, const ResourceError& error)
    597602{

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h

    r266467 r267227  
    200200    void sendH2Ping(NetworkResourceLoadParameters&&, CompletionHandler<void(Expected<WTF::Seconds, WebCore::ResourceError>&&)>&&);
    201201    void preconnectTo(Optional<uint64_t> preconnectionIdentifier, NetworkResourceLoadParameters&&);
     202    void isResourceLoadFinished(uint64_t loadIdentifier, CompletionHandler<void(bool)>&&);
    202203
    203204    void removeLoadIdentifier(ResourceLoadIdentifier);

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in

    r266467 r267227  
    3333    SendH2Ping(WebKit::NetworkResourceLoadParameters parameters) -> (Expected<Seconds, WebCore::ResourceError> result) Async
    3434    PreconnectTo(Optional<uint64_t> preconnectionIdentifier, WebKit::NetworkResourceLoadParameters loadParameters);
     35    IsResourceLoadFinished(uint64_t resourceLoadIdentifier) -> (bool isFinished) Async
    3536
    3637    StartDownload(WebKit::DownloadID downloadID, WebCore::ResourceRequest request, enum:bool Optional<WebKit::NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain, String suggestedName)

  • trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp

    r266990 r267227  
    782782}
    783783
     784void WebLoaderStrategy::isResourceLoadFinished(CachedResource& resource, CompletionHandler<void(bool)>&& callback)
     785{
     786    if (!resource.loader()) {
     787        callback(true);
     788        return;
     789    }
     790    WebProcess::singleton().ensureNetworkProcessConnection().connection().sendWithAsyncReply(Messages::NetworkConnectionToWebProcess::IsResourceLoadFinished(resource.loader()->identifier()), WTFMove(callback), 0);
     791}
     792
    784793void WebLoaderStrategy::setOnLineState(bool isOnLine)
    785794{

  • trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.h

    r264021 r267227  
    113113    bool havePerformedSecurityChecks(const WebCore::ResourceResponse&) const final;
    114114
     115    void isResourceLoadFinished(WebCore::CachedResource&, CompletionHandler<void(bool)>&&) final;
     116
    115117    Vector<uint64_t> ongoingLoads() const final
    116118    {

  • trunk/Source/WebKitLegacy/ChangeLog

    r267034 r267227  
     12020-09-18  Youenn Fablet  <youenn@apple.com>
     2
     3        XHR.timeout is affected by long tasks
     4        https://bugs.webkit.org/show_bug.cgi?id=216266
     5        <rdar://problem/68908150>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * WebCoreSupport/WebResourceLoadScheduler.cpp:
     10        (WebResourceLoadScheduler::isResourceLoadFinished):
     11        * WebCoreSupport/WebResourceLoadScheduler.h:
     12
    1132020-09-14  Fujii Hironori  <Hironori.Fujii@sony.com>
    214

  • trunk/Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.cpp

    r257206 r267227  
    2626
    2727#include "PingHandle.h"
     28#include <WebCore/CachedResource.h>
    2829#include <WebCore/Document.h>
    2930#include <WebCore/DocumentLoader.h>
    … …  
    204205}
    205206
     207void WebResourceLoadScheduler::isResourceLoadFinished(CachedResource& resource, CompletionHandler<void(bool)>&& callback)
     208{
     209    if (!resource.loader()) {
     210        callback(true);
     211        return;
     212    }
     213    callback(!hostForURL(resource.loader()->url()));
     214}
     215
    206216void WebResourceLoadScheduler::setDefersLoading(ResourceLoader& loader, bool defers)
    207217{

  • trunk/Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.h

    r260415 r267227  
    8080
    8181    bool isSuspendingPendingRequests() const { return !!m_suspendPendingRequestsCount; }
     82    void isResourceLoadFinished(WebCore::CachedResource&, CompletionHandler<void(bool)>&&) final;
    8283
    8384    class HostInformation {
Note: See TracChangeset for help on using the changeset viewer.