Changeset 250985 in webkit


Ignore:
Timestamp:
Oct 10, 2019 12:52:04 PM (5 years ago)
Author:
youenn@apple.com
Message:

Do not timeout a load intercepted by service worker that receives a response
​https://bugs.webkit.org/show_bug.cgi?id=202787

Reviewed by Chris Dumez.

Source/WebKit:

Stop making ServiceWorkerFetchTask ref counted since it is not needed and
can potentially make ServiceWorkerFetchTask oulive its WebSWServerToContextConnection member.

Stop the ServiceWorkerFetchTask timeout timer whenever receiving a response so that the load will not timeout in that case.
This ensures that a load that is starting in a service worker will not be failing.
Instead the load will go to network process.

Removed m_didReachTerminalState which is not needed as WebSWServerToContextConnection unregisters the ServiceWorkerFetchTask
as an IPC listener for all terminating messages.

  • NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:

(WebKit::ServiceWorkerFetchTask::didReceiveRedirectResponse):
(WebKit::ServiceWorkerFetchTask::didReceiveResponse):
(WebKit::ServiceWorkerFetchTask::didReceiveData):
(WebKit::ServiceWorkerFetchTask::didReceiveFormData):
(WebKit::ServiceWorkerFetchTask::didFinish):
(WebKit::ServiceWorkerFetchTask::didFail):
(WebKit::ServiceWorkerFetchTask::didNotHandle):
(WebKit::ServiceWorkerFetchTask::timeoutTimerFired):

  • NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h:
  • NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:

(WebKit::WebSWServerToContextConnection::startFetch):
(WebKit::WebSWServerToContextConnection::fetchTaskTimedOut):
Use a Vector instead of a HasSet for performance reasons.
Update according fetch map using unique_ptr instead of Ref<>.

  • NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:

LayoutTests:

  • http/wpt/service-workers/fetch-timeout-worker.js: Added.

(async.doTest):

  • http/wpt/service-workers/fetch-timeout.https-expected.txt: Added.
  • http/wpt/service-workers/fetch-timeout.https.html: Added.
  • http/wpt/service-workers/resources/lengthy-pass.py:

(main):

Location:
trunk
Files:
3 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r250983 r250985  
     12019-10-10  Youenn Fablet  <youenn@apple.com>
     2
     3        Do not timeout a load intercepted by service worker that receives a response
     4        https://bugs.webkit.org/show_bug.cgi?id=202787
     5
     6        Reviewed by Chris Dumez.
     7
     8        * http/wpt/service-workers/fetch-timeout-worker.js: Added.
     9        (async.doTest):
     10        * http/wpt/service-workers/fetch-timeout.https-expected.txt: Added.
     11        * http/wpt/service-workers/fetch-timeout.https.html: Added.
     12        * http/wpt/service-workers/resources/lengthy-pass.py:
     13        (main):
     14
    1152019-10-10  Myles C. Maxfield  <mmaxfield@apple.com>
    216

  • trunk/LayoutTests/http/wpt/service-workers/resources/lengthy-pass.py

    r249096 r250985  
    33def main(request, response):
    44    delay = 0.05
     5    headers = []
     6    if "delay" in request.GET:
     7        delay = float(request.GET.first("delay"))
    58    response.headers.set("Content-type", "text/javascript")
    69    response.headers.append("Access-Control-Allow-Origin", "*")

  • trunk/Source/WebKit/ChangeLog

    r250974 r250985  
     12019-10-10  Youenn Fablet  <youenn@apple.com>
     2
     3        Do not timeout a load intercepted by service worker that receives a response
     4        https://bugs.webkit.org/show_bug.cgi?id=202787
     5
     6        Reviewed by Chris Dumez.
     7
     8        Stop making ServiceWorkerFetchTask ref counted since it is not needed and
     9        can potentially make ServiceWorkerFetchTask oulive its WebSWServerToContextConnection member.
     10
     11        Stop the ServiceWorkerFetchTask timeout timer whenever receiving a response so that the load will not timeout in that case.
     12        This ensures that a load that is starting in a service worker will not be failing.
     13        Instead the load will go to network process.
     14
     15        Removed m_didReachTerminalState which is not needed as WebSWServerToContextConnection unregisters the ServiceWorkerFetchTask
     16        as an IPC listener for all terminating messages.
     17
     18        * NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:
     19        (WebKit::ServiceWorkerFetchTask::didReceiveRedirectResponse):
     20        (WebKit::ServiceWorkerFetchTask::didReceiveResponse):
     21        (WebKit::ServiceWorkerFetchTask::didReceiveData):
     22        (WebKit::ServiceWorkerFetchTask::didReceiveFormData):
     23        (WebKit::ServiceWorkerFetchTask::didFinish):
     24        (WebKit::ServiceWorkerFetchTask::didFail):
     25        (WebKit::ServiceWorkerFetchTask::didNotHandle):
     26        (WebKit::ServiceWorkerFetchTask::timeoutTimerFired):
     27        * NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h:
     28        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
     29        (WebKit::WebSWServerToContextConnection::startFetch):
     30        (WebKit::WebSWServerToContextConnection::fetchTaskTimedOut):
     31        Use a Vector instead of a HasSet for performance reasons.
     32        Update according fetch map using unique_ptr instead of Ref<>.
     33        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
     34
    1352019-10-10  Rob Buis  <rbuis@igalia.com>
    236

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp

    r250852 r250985  
    6060{
    6161    RELEASE_LOG_IF_ALLOWED("didReceiveRedirectResponse: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
     62    m_timeoutTimer.stop();
    6263    m_wasHandled = true;
    6364    if (m_connection)
    … …  
    6869{
    6970    RELEASE_LOG_IF_ALLOWED("didReceiveResponse: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
     71    m_timeoutTimer.stop();
    7072    m_wasHandled = true;
    7173    if (m_connection)
    … …  
    7577void ServiceWorkerFetchTask::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
    7678{
     79    ASSERT(!m_timeoutTimer.isActive());
    7780    if (m_connection)
    7881        m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveData { data, encodedDataLength }, m_identifier.fetchIdentifier);
    … …  
    8184void ServiceWorkerFetchTask::didReceiveFormData(const IPC::FormDataReference& formData)
    8285{
     86    ASSERT(!m_timeoutTimer.isActive());
    8387    if (m_connection)
    8488        m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveFormData { formData }, m_identifier.fetchIdentifier);
    … …  
    8791void ServiceWorkerFetchTask::didFinish()
    8892{
     93    ASSERT(!m_timeoutTimer.isActive());
    8994    RELEASE_LOG_IF_ALLOWED("didFinishFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
    9095    m_timeoutTimer.stop();
    91     if (!m_didReachTerminalState && m_connection)
     96    if (m_connection)
    9297        m_connection->send(Messages::ServiceWorkerClientFetch::DidFinish { }, m_identifier.fetchIdentifier);
    93     m_didReachTerminalState = true;
    9498}
    9599
    … …  
    98102    RELEASE_LOG_ERROR_IF_ALLOWED("didFailFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
    99103    m_timeoutTimer.stop();
    100     if (!m_didReachTerminalState && m_connection)
     104    if (m_connection)
    101105        m_connection->send(Messages::ServiceWorkerClientFetch::DidFail { error }, m_identifier.fetchIdentifier);
    102     m_didReachTerminalState = true;
    103106}
    104107
    … …  
    107110    RELEASE_LOG_IF_ALLOWED("didNotHandleFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
    108111    m_timeoutTimer.stop();
    109     if (!m_didReachTerminalState && m_connection)
     112    if (m_connection)
    110113        m_connection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, m_identifier.fetchIdentifier);
    111     m_didReachTerminalState = true;
    112114}
    113115
    … …  
    115117{
    116118    RELEASE_LOG_IF_ALLOWED("timeoutTimerFired: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
    117     if (!m_wasHandled)
    118         didNotHandle();
    119     else
    120         didFail({ errorDomainWebKitInternal, 0, { }, "Service Worker fetch timed out"_s });
    121 
     119    didNotHandle();
    122120    m_contextConnection.fetchTaskTimedOut(*this);
    123121}

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h

    r250852 r250985  
    3232#include <WebCore/Timer.h>
    3333#include <pal/SessionID.h>
    34 #include <wtf/RefCounted.h>
    3534
    3635namespace WebCore {
    … …  
    5150class WebSWServerToContextConnection;
    5251
    53 class ServiceWorkerFetchTask : public RefCounted<ServiceWorkerFetchTask> {
     52class ServiceWorkerFetchTask {
     53    WTF_MAKE_FAST_ALLOCATED;
    5454public:
    55     template<typename... Args> static Ref<ServiceWorkerFetchTask> create(Args&&... args)
    56     {
    57         return adoptRef(*new ServiceWorkerFetchTask(std::forward<Args>(args)...));
    58     }
     55    ServiceWorkerFetchTask(PAL::SessionID, WebSWServerConnection&, WebSWServerToContextConnection&, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, Seconds timeout);
    5956
    6057    void didNotHandle();
    … …  
    8077
    8178private:
    82     ServiceWorkerFetchTask(PAL::SessionID, WebSWServerConnection&, WebSWServerToContextConnection&, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, Seconds timeout);
    83 
    8479    void didReceiveRedirectResponse(const WebCore::ResourceResponse&);
    8580    void didReceiveResponse(const WebCore::ResourceResponse&, bool needsContinueDidReceiveResponseMessage);
    … …  
    9893    WebCore::Timer m_timeoutTimer;
    9994    bool m_wasHandled { false };
    100     bool m_didReachTerminalState { false };
    10195};
    10296

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp

    r250852 r250985  
    158158    auto fetchIdentifier = FetchIdentifier::generate();
    159159
    160     auto result = m_ongoingFetches.add(fetchIdentifier, ServiceWorkerFetchTask::create(sessionID, contentConnection, *this, contentFetchIdentifier, serviceWorkerIdentifier, m_networkProcess->serviceWorkerFetchTimeout()));
     160    auto result = m_ongoingFetches.add(fetchIdentifier, makeUnique<ServiceWorkerFetchTask>(sessionID, contentConnection, *this, contentFetchIdentifier, serviceWorkerIdentifier, m_networkProcess->serviceWorkerFetchTimeout()));
    161161
    162162    ASSERT(!m_ongoingFetchIdentifiers.contains({ serverConnectionIdentifier, contentFetchIdentifier }));
    … …  
    216216    auto takenTask = m_ongoingFetches.take(takenIdentifier);
    217217    ASSERT(takenTask);
    218     ASSERT(takenTask->ptr() == &task);
     218    ASSERT(takenTask.get() == &task);
    219219
    220220    // Gather all other fetches in this service worker
    221     HashSet<Ref<ServiceWorkerFetchTask>> otherFetches;
     221    Vector<ServiceWorkerFetchTask*> otherFetches;
    222222    for (auto& fetchTask : m_ongoingFetches.values()) {
    223223        if (fetchTask->serviceWorkerIdentifier() == task.serviceWorkerIdentifier())
    224             otherFetches.add(fetchTask.copyRef());
     224            otherFetches.append(fetchTask.get());
    225225    }
    226226
    227227    // Signal load failure for them
    228     for (auto& fetchTask : otherFetches) {
     228    for (auto* fetchTask : otherFetches) {
    229229        if (fetchTask->wasHandled())
    230230            fetchTask->fail({ errorDomainWebKitInternal, 0, { }, "Service Worker context closed"_s });

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h

    r250852 r250985  
    104104
    105105    HashMap<ServiceWorkerFetchTask::Identifier, WebCore::FetchIdentifier> m_ongoingFetchIdentifiers;
    106     HashMap<WebCore::FetchIdentifier, Ref<ServiceWorkerFetchTask>> m_ongoingFetches;
     106    HashMap<WebCore::FetchIdentifier, std::unique_ptr<ServiceWorkerFetchTask>> m_ongoingFetches;
    107107    bool m_isThrottleable { true };
    108108}; // class WebSWServerToContextConnection
Note: See TracChangeset for help on using the changeset viewer.