Service Worker Fetch events should time out.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Oct 2019 19:50:12 +0000 (19:50 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Oct 2019 19:50:12 +0000 (19:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202188

Reviewed by Alex Christensen.

Source/WebCore:

Test: http/tests/workers/service/basic-timeout.https.html

* workers/service/server/SWServer.h:
* workers/service/server/SWServerWorker.h:
(WebCore::SWServerWorker::setHasTimedOutAnyFetchTasks):
(WebCore::SWServerWorker::hasTimedOutAnyFetchTasks const):

Source/WebKit:

When we start a fetch task in the server, we also start a timeout on that fetch task.

"Time out" means the fetch task must continue to make progress at the given frequency (once every 60 seconds by default)

If any given fetch task times out in a service worker instance, that instance loses the right to handle fetches.

* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::setServiceWorkerFetchTimeoutForTesting):
(WebKit::NetworkProcess::resetServiceWorkerFetchTimeoutForTesting):
* NetworkProcess/NetworkProcess.h:
(WebKit::NetworkProcess::serviceWorkerFetchTimeout const):
* NetworkProcess/NetworkProcess.messages.in:

* NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:
(WebKit::ServiceWorkerFetchTask::ServiceWorkerFetchTask):
(WebKit::ServiceWorkerFetchTask::didReceiveRedirectResponse):
(WebKit::ServiceWorkerFetchTask::didReceiveResponse):
(WebKit::ServiceWorkerFetchTask::didReceiveData):
(WebKit::ServiceWorkerFetchTask::didReceiveFormData):
(WebKit::ServiceWorkerFetchTask::didFinish):
(WebKit::ServiceWorkerFetchTask::didFail):
(WebKit::ServiceWorkerFetchTask::didNotHandle):
* NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h:
(WebKit::ServiceWorkerFetchTask::create):
(WebKit::ServiceWorkerFetchTask::serviceWorkerIdentifier const):
(WebKit::ServiceWorkerFetchTask::wasHandled const):
(WebKit::ServiceWorkerFetchTask::ServiceWorkerFetchTask): Deleted.

* NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
(WebKit::WebSWServerConnection::startFetch):
* NetworkProcess/ServiceWorker/WebSWServerConnection.h:

* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
(WebKit::WebSWServerToContextConnection::startFetch):
(WebKit::WebSWServerToContextConnection::fetchTaskTimedOut):
* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:

* UIProcess/API/C/WKContext.cpp:
(WKContextSetServiceWorkerFetchTimeoutForTesting):
(WKContextResetServiceWorkerFetchTimeoutForTesting):
* UIProcess/API/C/WKContext.h:

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::setServiceWorkerTimeoutForTesting):
(WebKit::WebProcessPool::resetServiceWorkerTimeoutForTesting):
* UIProcess/WebProcessPool.h:

Tools:

* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::setServiceWorkerFetchTimeout):
* WebKitTestRunner/InjectedBundle/TestRunner.h:
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::resetStateToConsistentValues):
(WTR::TestController::setServiceWorkerFetchTimeoutForTesting):
* WebKitTestRunner/TestController.h:
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):

LayoutTests:

* http/tests/workers/service/basic-timeout.https-expected.txt: Added.
* http/tests/workers/service/basic-timeout.https.html: Added.
* http/tests/workers/service/resources/basic-timeout-worker.js: Added.
(event.event.request.url.indexOf):
* http/tests/workers/service/resources/basic-timeout.js: Added.
(async.test.finishThisTest):
(async.test.try):
(async.test.try.checkSuccessAgain):
(async.test):
* http/tests/workers/service/resources/succeed-fallback-check.php: Added.
* http/tests/workers/service/resources/timeout-fallback.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250852 268f45cc-cd09-0410-ab3c-d52691b4dbfc

31 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/workers/service/basic-timeout.https-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/workers/service/basic-timeout.https.html [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/basic-timeout-worker.js [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/basic-timeout.js [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/succeed-fallback-check.php [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/timeout-fallback.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/workers/service/server/SWServer.h
Source/WebCore/workers/service/server/SWServerWorker.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkProcess.cpp
Source/WebKit/NetworkProcess/NetworkProcess.h
Source/WebKit/NetworkProcess/NetworkProcess.messages.in
Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp
Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h
Source/WebKit/UIProcess/API/C/WKContext.cpp
Source/WebKit/UIProcess/API/C/WKContext.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
Tools/WebKitTestRunner/TestController.cpp
Tools/WebKitTestRunner/TestController.h
Tools/WebKitTestRunner/TestInvocation.cpp

index 3364396..cc8f4e2 100644 (file)
@@ -1,3 +1,22 @@
+2019-10-08  Brady Eidson  <beidson@apple.com>
+
+        Service Worker Fetch events should time out.
+        https://bugs.webkit.org/show_bug.cgi?id=202188
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/workers/service/basic-timeout.https-expected.txt: Added.
+        * http/tests/workers/service/basic-timeout.https.html: Added.
+        * http/tests/workers/service/resources/basic-timeout-worker.js: Added.
+        (event.event.request.url.indexOf):
+        * http/tests/workers/service/resources/basic-timeout.js: Added.
+        (async.test.finishThisTest):
+        (async.test.try):
+        (async.test.try.checkSuccessAgain):
+        (async.test):
+        * http/tests/workers/service/resources/succeed-fallback-check.php: Added.
+        * http/tests/workers/service/resources/timeout-fallback.html: Added.
+
 2019-10-08  Alexey Shvayka  <shvaikalesh@gmail.com>
 
         Accept two values in the overflow shorthand
diff --git a/LayoutTests/http/tests/workers/service/basic-timeout.https-expected.txt b/LayoutTests/http/tests/workers/service/basic-timeout.https-expected.txt
new file mode 100644 (file)
index 0000000..a8f9e2d
--- /dev/null
@@ -0,0 +1,14 @@
+
+URL 0 - https://127.0.0.1:8443/workers/service/resources/timeout-fallback.html
+Status code 0 - 200
+Source' header 0 - null
+URL 1 - https://127.0.0.1:8443/workers/service/resources/timeout-no-fallback.html
+Status code 1 - 404
+Source' header 1 - null
+URL 2 - https://127.0.0.1:8443/workers/service/resources/succeed-fallback-check.php
+Status code 2 - 200
+Source' header 2 - network
+URL 3 - https://127.0.0.1:8443/workers/service/resources/succeed-fallback-check.php
+Status code 3 - 200
+Source' header 3 - network
+
diff --git a/LayoutTests/http/tests/workers/service/basic-timeout.https.html b/LayoutTests/http/tests/workers/service/basic-timeout.https.html
new file mode 100644 (file)
index 0000000..f6acdc6
--- /dev/null
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script src="resources/sw-test-pre.js"></script>
+</head>
+<body>
+
+<script src="resources/basic-timeout.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/workers/service/resources/basic-timeout-worker.js b/LayoutTests/http/tests/workers/service/resources/basic-timeout-worker.js
new file mode 100644 (file)
index 0000000..5c5a20b
--- /dev/null
@@ -0,0 +1,6 @@
+self.addEventListener("fetch", (event) => {
+    if (event.request.url.indexOf("timeout") !== -1) {
+        while (1) { }
+    } else
+        event.respondWith(new Response(null, { status: 200, statusText: "Hello from service worker", headers: [["Source", "ServiceWorker"]] }));
+});
diff --git a/LayoutTests/http/tests/workers/service/resources/basic-timeout.js b/LayoutTests/http/tests/workers/service/resources/basic-timeout.js
new file mode 100644 (file)
index 0000000..c1bbcc2
--- /dev/null
@@ -0,0 +1,76 @@
+async function test()
+{
+    var urlResults = new Array;
+    var statusResults = new Array;
+    var headerResults = new Array;
+
+    function finishThisTest()
+    {
+        for (var i = 0; i < urlResults.length; ++i) {
+            log("URL " + i + " - " + urlResults[i]);
+            log("Status code " + i + " - " + statusResults[i]);
+            log("Source' header " + i + " - " + headerResults[i]);
+        }
+        finishSWTest();
+    }
+    
+    try {
+        var frame = await interceptedFrame("resources/basic-timeout-worker.js", "/workers/service/resources/");
+        var fetch = frame.contentWindow.fetch;
+
+        if (window.testRunner)
+            testRunner.setServiceWorkerFetchTimeout(0);
+        
+        // The following two fetches should time out immediately
+        fetch("timeout-fallback.html").then(function(response) {
+            urlResults[0] = response.url;
+            statusResults[0] = response.status;
+            headerResults[0] = response.headers.get("Source");       
+        }, function(error) {
+            log(error);
+        });
+
+        fetch("timeout-no-fallback.html").then(function(response) {
+            urlResults[1] = response.url;
+            statusResults[1] = response.status;
+            headerResults[1] = response.headers.get("Source");   
+        }, function(error) {
+            log(error);
+        });
+
+        if (window.testRunner)
+            testRunner.setServiceWorkerFetchTimeout(60);
+
+        // The service worker knows how to handle the following fetch *and* has 60 seconds to do so.
+        // But will be cancelled with the above fetches since we're terminating the service worker, and 
+        // therefore it will then fallback to the network.
+        fetch("succeed-fallback-check.php").then(function(response) {
+            urlResults[2] = response.url;
+            statusResults[2] = response.status;
+            headerResults[2] = response.headers.get("Source");   
+            setTimeout(checkSuccessAgain, 0);
+        }, function(error) {
+            log(error);
+            finishSWTest();
+        });
+        
+        // Now we can fetch that same URL again, which *could* relaunch the service worker and handle it there, but for now this service worker registration is inert and fetches through it will go to the network instead.
+        // I'm leaving this in to cover future cases where we do relaunch the SW to handle it.
+        function checkSuccessAgain() {
+            fetch("succeed-fallback-check.php").then(function(response) {
+                urlResults[3] = response.url;
+                statusResults[3] = response.status;
+                headerResults[3] = response.headers.get("Source");   
+                finishThisTest();
+            }, function(error) {
+                log(error);
+                finishSWTest();
+            });
+        }
+    } catch(e) {
+        log("Got exception: " + e);
+        finishSWTest();
+    }
+}
+
+test();
diff --git a/LayoutTests/http/tests/workers/service/resources/succeed-fallback-check.php b/LayoutTests/http/tests/workers/service/resources/succeed-fallback-check.php
new file mode 100644 (file)
index 0000000..f1a8f0c
--- /dev/null
@@ -0,0 +1,3 @@
+<?php
+    header('Source: network');
+?>Success!
\ No newline at end of file
diff --git a/LayoutTests/http/tests/workers/service/resources/timeout-fallback.html b/LayoutTests/http/tests/workers/service/resources/timeout-fallback.html
new file mode 100644 (file)
index 0000000..894b6be
--- /dev/null
@@ -0,0 +1 @@
+Timeout resource fetched from network
\ No newline at end of file
index 50996e5..b3fbeba 100644 (file)
@@ -1,3 +1,17 @@
+2019-10-08  Brady Eidson  <beidson@apple.com>
+
+        Service Worker Fetch events should time out.
+        https://bugs.webkit.org/show_bug.cgi?id=202188
+
+        Reviewed by Alex Christensen.
+
+        Test: http/tests/workers/service/basic-timeout.https.html
+
+        * workers/service/server/SWServer.h:
+        * workers/service/server/SWServerWorker.h:
+        (WebCore::SWServerWorker::setHasTimedOutAnyFetchTasks):
+        (WebCore::SWServerWorker::hasTimedOutAnyFetchTasks const):
+
 2019-10-08  Alexey Shvayka  <shvaikalesh@gmail.com>
 
         Accept two values in the overflow shorthand
index f42c1af..4601317 100644 (file)
@@ -148,7 +148,7 @@ public:
 
     void updateWorker(Connection&, const ServiceWorkerJobDataIdentifier&, SWServerRegistration&, const URL&, const String& script, const ContentSecurityPolicyResponseHeaders&, const String& referrerPolicy, WorkerType, HashMap<URL, ServiceWorkerContextData::ImportedScript>&&);
     void terminateWorker(SWServerWorker&);
-    void syncTerminateWorker(SWServerWorker&);
+    WEBCORE_EXPORT void syncTerminateWorker(SWServerWorker&);
     void fireInstallEvent(SWServerWorker&);
     void fireActivateEvent(SWServerWorker&);
 
index 6767fc6..1b99cc9 100644 (file)
@@ -116,6 +116,9 @@ public:
     
     SWServerRegistration* registration() const;
 
+    void setHasTimedOutAnyFetchTasks() { m_hasTimedOutAnyFetchTasks = true; }
+    bool hasTimedOutAnyFetchTasks() const { return m_hasTimedOutAnyFetchTasks; }
+
 private:
     SWServerWorker(SWServer&, SWServerRegistration&, const URL&, const String& script, const ContentSecurityPolicyResponseHeaders&, String&& referrerPolicy, WorkerType, ServiceWorkerIdentifier, HashMap<URL, ServiceWorkerContextData::ImportedScript>&&);
 
@@ -136,6 +139,7 @@ private:
     Vector<Function<void(bool)>> m_whenActivatedHandlers;
     HashMap<URL, ServiceWorkerContextData::ImportedScript> m_scriptResourceMap;
     bool m_shouldSkipHandleFetch;
+    bool m_hasTimedOutAnyFetchTasks { false };
 };
 
 } // namespace WebCore
index eae5e9a..5f44fb8 100644 (file)
@@ -1,3 +1,57 @@
+2019-10-08  Brady Eidson  <beidson@apple.com>
+
+        Service Worker Fetch events should time out.
+        https://bugs.webkit.org/show_bug.cgi?id=202188
+
+        Reviewed by Alex Christensen.
+
+        When we start a fetch task in the server, we also start a timeout on that fetch task.
+        
+        "Time out" means the fetch task must continue to make progress at the given frequency (once every 60 seconds by default)
+
+        If any given fetch task times out in a service worker instance, that instance loses the right to handle fetches.
+        
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::setServiceWorkerFetchTimeoutForTesting):
+        (WebKit::NetworkProcess::resetServiceWorkerFetchTimeoutForTesting):
+        * NetworkProcess/NetworkProcess.h:
+        (WebKit::NetworkProcess::serviceWorkerFetchTimeout const):
+        * NetworkProcess/NetworkProcess.messages.in:
+
+        * NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:
+        (WebKit::ServiceWorkerFetchTask::ServiceWorkerFetchTask):
+        (WebKit::ServiceWorkerFetchTask::didReceiveRedirectResponse): 
+        (WebKit::ServiceWorkerFetchTask::didReceiveResponse): 
+        (WebKit::ServiceWorkerFetchTask::didReceiveData): 
+        (WebKit::ServiceWorkerFetchTask::didReceiveFormData): 
+        (WebKit::ServiceWorkerFetchTask::didFinish): 
+        (WebKit::ServiceWorkerFetchTask::didFail): 
+        (WebKit::ServiceWorkerFetchTask::didNotHandle): 
+        * NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h:
+        (WebKit::ServiceWorkerFetchTask::create):
+        (WebKit::ServiceWorkerFetchTask::serviceWorkerIdentifier const):
+        (WebKit::ServiceWorkerFetchTask::wasHandled const):
+        (WebKit::ServiceWorkerFetchTask::ServiceWorkerFetchTask): Deleted.
+
+        * NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
+        (WebKit::WebSWServerConnection::startFetch):
+        * NetworkProcess/ServiceWorker/WebSWServerConnection.h:
+
+        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
+        (WebKit::WebSWServerToContextConnection::startFetch):
+        (WebKit::WebSWServerToContextConnection::fetchTaskTimedOut):
+        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
+
+        * UIProcess/API/C/WKContext.cpp:
+        (WKContextSetServiceWorkerFetchTimeoutForTesting):
+        (WKContextResetServiceWorkerFetchTimeoutForTesting):
+        * UIProcess/API/C/WKContext.h:
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::setServiceWorkerTimeoutForTesting):
+        (WebKit::WebProcessPool::resetServiceWorkerTimeoutForTesting):
+        * UIProcess/WebProcessPool.h:
+
 2019-10-08  Ross Kirsling  <ross.kirsling@sony.com>
 
         Unreviewed. Restabilize non-unified build.
index b580a07..0ca1767 100644 (file)
@@ -2614,4 +2614,17 @@ NetworkConnectionToWebProcess* NetworkProcess::webProcessConnection(ProcessIdent
     return m_webProcessConnections.get(identifier);
 }
 
+const Seconds NetworkProcess::defaultServiceWorkerFetchTimeout = 70_s;
+void NetworkProcess::setServiceWorkerFetchTimeoutForTesting(Seconds timeout, CompletionHandler<void()>&& completionHandler)
+{
+    m_serviceWorkerFetchTimeout = timeout;
+    completionHandler();
+}
+
+void NetworkProcess::resetServiceWorkerFetchTimeoutForTesting(CompletionHandler<void()>&& completionHandler)
+{
+    m_serviceWorkerFetchTimeout = defaultServiceWorkerFetchTimeout;
+    completionHandler();
+}
+
 } // namespace WebKit
index 1daea57..b99d4f2 100644 (file)
@@ -344,6 +344,10 @@ public:
     NetworkConnectionToWebProcess* webProcessConnection(WebCore::ProcessIdentifier) const;
     WebCore::MessagePortChannelRegistry& messagePortChannelRegistry() { return m_messagePortChannelRegistry; }
 
+    void setServiceWorkerFetchTimeoutForTesting(Seconds, CompletionHandler<void()>&&);
+    void resetServiceWorkerFetchTimeoutForTesting(CompletionHandler<void()>&&);
+    Seconds serviceWorkerFetchTimeout() const { return m_serviceWorkerFetchTimeout; }
+
 private:
     void platformInitializeNetworkProcess(const NetworkProcessCreationParameters&);
     std::unique_ptr<WebCore::NetworkStorageSession> platformCreateDefaultStorageSession() const;
@@ -561,6 +565,9 @@ private:
 
     OptionSet<NetworkCache::CacheOption> m_cacheOptions;
     WebCore::MessagePortChannelRegistry m_messagePortChannelRegistry;
+
+    static const Seconds defaultServiceWorkerFetchTimeout;
+    Seconds m_serviceWorkerFetchTimeout { defaultServiceWorkerFetchTimeout };
 };
 
 } // namespace WebKit
index bd3c24a..7399a7e 100644 (file)
@@ -164,4 +164,7 @@ messages -> NetworkProcess LegacyReceiver {
     SetAdClickAttributionConversionURLForTesting(PAL::SessionID sessionID, URL url) -> () Async
     MarkAdClickAttributionsAsExpiredForTesting(PAL::SessionID sessionID) -> () Async
     GetLocalStorageOriginDetails(PAL::SessionID sessionID) -> (Vector<WebKit::LocalStorageDatabaseTracker::OriginDetails> details) Async
+
+    SetServiceWorkerFetchTimeoutForTesting(Seconds seconds) -> () Synchronous
+    ResetServiceWorkerFetchTimeoutForTesting() -> () Synchronous
 }
index 553cbad..0dcb3b3 100644 (file)
@@ -34,6 +34,8 @@
 #include "Logging.h"
 #include "ServiceWorkerClientFetchMessages.h"
 #include "WebCoreArgumentCoders.h"
+#include "WebSWServerConnection.h"
+#include "WebSWServerToContextConnection.h"
 
 #define RELEASE_LOG_IF_ALLOWED(fmt, ...) RELEASE_LOG_IF(m_sessionID.isAlwaysOnLoggingAllowed(), ServiceWorker, "%p - ServiceWorkerFetchTask::" fmt, this, ##__VA_ARGS__)
 #define RELEASE_LOG_ERROR_IF_ALLOWED(fmt, ...) RELEASE_LOG_ERROR_IF(m_sessionID.isAlwaysOnLoggingAllowed(), ServiceWorker, "%p - ServiceWorkerFetchTask::" fmt, this, ##__VA_ARGS__)
@@ -42,43 +44,82 @@ using namespace WebCore;
 
 namespace WebKit {
 
+ServiceWorkerFetchTask::ServiceWorkerFetchTask(PAL::SessionID sessionID, WebSWServerConnection& connection, WebSWServerToContextConnection& contextConnection, FetchIdentifier fetchIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, Seconds timeout)
+    : m_sessionID(sessionID)
+    , m_connection(makeWeakPtr(connection))
+    , m_contextConnection(contextConnection)
+    , m_identifier { connection.identifier(), fetchIdentifier }
+    , m_serviceWorkerIdentifier(serviceWorkerIdentifier)
+    , m_timeout(timeout)
+    , m_timeoutTimer(*this, &ServiceWorkerFetchTask::timeoutTimerFired)
+{
+    m_timeoutTimer.startOneShot(m_timeout);
+}
+
 void ServiceWorkerFetchTask::didReceiveRedirectResponse(const ResourceResponse& response)
 {
     RELEASE_LOG_IF_ALLOWED("didReceiveRedirectResponse: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
-    m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveRedirectResponse { response }, m_identifier.fetchIdentifier);
+    m_wasHandled = true;
+    if (m_connection)
+        m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveRedirectResponse { response }, m_identifier.fetchIdentifier);
 }
 
 void ServiceWorkerFetchTask::didReceiveResponse(const ResourceResponse& response, bool needsContinueDidReceiveResponseMessage)
 {
-    m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveResponse { response, needsContinueDidReceiveResponseMessage }, m_identifier.fetchIdentifier);
+    RELEASE_LOG_IF_ALLOWED("didReceiveResponse: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
+    m_wasHandled = true;
+    if (m_connection)
+        m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveResponse { response, needsContinueDidReceiveResponseMessage }, m_identifier.fetchIdentifier);
 }
 
 void ServiceWorkerFetchTask::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
 {
-    m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveData { data, encodedDataLength }, m_identifier.fetchIdentifier);
+    if (m_connection)
+        m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveData { data, encodedDataLength }, m_identifier.fetchIdentifier);
 }
 
 void ServiceWorkerFetchTask::didReceiveFormData(const IPC::FormDataReference& formData)
 {
-    m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveFormData { formData }, m_identifier.fetchIdentifier);
+    if (m_connection)
+        m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveFormData { formData }, m_identifier.fetchIdentifier);
 }
 
 void ServiceWorkerFetchTask::didFinish()
 {
     RELEASE_LOG_IF_ALLOWED("didFinishFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
-    m_connection->send(Messages::ServiceWorkerClientFetch::DidFinish { }, m_identifier.fetchIdentifier);
+    m_timeoutTimer.stop();
+    if (!m_didReachTerminalState && m_connection)
+        m_connection->send(Messages::ServiceWorkerClientFetch::DidFinish { }, m_identifier.fetchIdentifier);
+    m_didReachTerminalState = true;
 }
 
 void ServiceWorkerFetchTask::didFail(const ResourceError& error)
 {
     RELEASE_LOG_ERROR_IF_ALLOWED("didFailFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
-    m_connection->send(Messages::ServiceWorkerClientFetch::DidFail { error }, m_identifier.fetchIdentifier);
+    m_timeoutTimer.stop();
+    if (!m_didReachTerminalState && m_connection)
+        m_connection->send(Messages::ServiceWorkerClientFetch::DidFail { error }, m_identifier.fetchIdentifier);
+    m_didReachTerminalState = true;
 }
 
 void ServiceWorkerFetchTask::didNotHandle()
 {
     RELEASE_LOG_IF_ALLOWED("didNotHandleFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
-    m_connection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, m_identifier.fetchIdentifier);
+    m_timeoutTimer.stop();
+    if (!m_didReachTerminalState && m_connection)
+        m_connection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, m_identifier.fetchIdentifier);
+    m_didReachTerminalState = true;
+}
+
+void ServiceWorkerFetchTask::timeoutTimerFired()
+{
+    RELEASE_LOG_IF_ALLOWED("timeoutTimerFired: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
+    if (!m_wasHandled)
+        didNotHandle();
+    else
+        didFail({ errorDomainWebKitInternal, 0, { }, "Service Worker fetch timed out"_s });
+
+    m_contextConnection.fetchTaskTimedOut(*this);
 }
 
 } // namespace WebKit
index f116cc7..6add0d0 100644 (file)
@@ -29,6 +29,7 @@
 
 #include <WebCore/FetchIdentifier.h>
 #include <WebCore/ServiceWorkerTypes.h>
+#include <WebCore/Timer.h>
 #include <pal/SessionID.h>
 #include <wtf/RefCounted.h>
 
@@ -46,10 +47,17 @@ class FormDataReference;
 
 namespace WebKit {
 
+class WebSWServerConnection;
+class WebSWServerToContextConnection;
+
 class ServiceWorkerFetchTask : public RefCounted<ServiceWorkerFetchTask> {
 public:
-    static Ref<ServiceWorkerFetchTask> create(PAL::SessionID sessionID, Ref<IPC::Connection>&& connection, WebCore::SWServerConnectionIdentifier connectionIdentifier, WebCore::FetchIdentifier fetchIdentifier) { return adoptRef(*new ServiceWorkerFetchTask(sessionID, WTFMove(connection), connectionIdentifier, fetchIdentifier)); }
+    template<typename... Args> static Ref<ServiceWorkerFetchTask> create(Args&&... args)
+    {
+        return adoptRef(*new ServiceWorkerFetchTask(std::forward<Args>(args)...));
+    }
 
+    void didNotHandle();
     void fail(const WebCore::ResourceError& error) { didFail(error); }
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&);
 
@@ -67,13 +75,11 @@ public:
     };
 
     const Identifier& identifier() const { return m_identifier; }
+    const WebCore::ServiceWorkerIdentifier& serviceWorkerIdentifier() const { return m_serviceWorkerIdentifier; }
+    bool wasHandled() const { return m_wasHandled; }
 
 private:
-    ServiceWorkerFetchTask(PAL::SessionID sessionID, Ref<IPC::Connection>&& connection, WebCore::SWServerConnectionIdentifier connectionIdentifier, WebCore::FetchIdentifier fetchIdentifier)
-        : m_sessionID(sessionID)
-        , m_connection(WTFMove(connection))
-        , m_identifier { connectionIdentifier, fetchIdentifier }
-    { }
+    ServiceWorkerFetchTask(PAL::SessionID, WebSWServerConnection&, WebSWServerToContextConnection&, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, Seconds timeout);
 
     void didReceiveRedirectResponse(const WebCore::ResourceResponse&);
     void didReceiveResponse(const WebCore::ResourceResponse&, bool needsContinueDidReceiveResponseMessage);
@@ -81,11 +87,17 @@ private:
     void didReceiveFormData(const IPC::FormDataReference&);
     void didFinish();
     void didFail(const WebCore::ResourceError&);
-    void didNotHandle();
+    void timeoutTimerFired();
 
     PAL::SessionID m_sessionID;
-    Ref<IPC::Connection> m_connection;
+    WeakPtr<WebSWServerConnection> m_connection;
+    WebSWServerToContextConnection& m_contextConnection;
     Identifier m_identifier;
+    WebCore::ServiceWorkerIdentifier m_serviceWorkerIdentifier;
+    Seconds m_timeout;
+    WebCore::Timer m_timeoutTimer;
+    bool m_wasHandled { false };
+    bool m_didReachTerminalState { false };
 };
 
 inline bool operator==(const ServiceWorkerFetchTask::Identifier& a, const ServiceWorkerFetchTask::Identifier& b)
index 7674140..012dac3 100644 (file)
@@ -174,7 +174,7 @@ void WebSWServerConnection::startFetch(ServiceWorkerRegistrationIdentifier servi
         }
 
         auto* worker = server().workerByID(serviceWorkerIdentifier);
-        if (!worker) {
+        if (!worker || worker->hasTimedOutAnyFetchTasks()) {
             m_contentConnection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, fetchIdentifier);
             return;
         }
@@ -188,9 +188,9 @@ void WebSWServerConnection::startFetch(ServiceWorkerRegistrationIdentifier servi
 
             if (contextConnection) {
                 SWSERVERCONNECTION_RELEASE_LOG_IF_ALLOWED("startFetch: Starting fetch %s via service worker %s", fetchIdentifier.loggingString().utf8().data(), serviceWorkerIdentifier.loggingString().utf8().data());
-                static_cast<WebSWServerToContextConnection&>(*contextConnection).startFetch(sessionID(), m_contentConnection.get(), this->identifier(), fetchIdentifier, serviceWorkerIdentifier, request, options, formData, referrer);
+                static_cast<WebSWServerToContextConnection&>(*contextConnection).startFetch(sessionID(), *this, fetchIdentifier, serviceWorkerIdentifier, request, options, formData, referrer);
             } else {
-                SWSERVERCONNECTION_RELEASE_LOG_ERROR_IF_ALLOWED("startFetch: fetchIdentifier: %s DidNotHandle because failed to run service worker", fetchIdentifier.loggingString().utf8().data());
+                SWSERVERCONNECTION_RELEASE_LOG_ERROR_IF_ALLOWED("startFetch: fetchIdentifier: %s DidNotHandle because failed to run service worker, or service worker has had timed out fetch tasks", fetchIdentifier.loggingString().utf8().data());
                 m_contentConnection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, fetchIdentifier);
             }
         });
index 8cd495c..7bd2a33 100644 (file)
@@ -35,6 +35,7 @@
 #include <WebCore/SWServer.h>
 #include <pal/SessionID.h>
 #include <wtf/HashMap.h>
+#include <wtf/WeakPtr.h>
 
 namespace IPC {
 class FormDataReference;
index 58094d6..e36bb12 100644 (file)
@@ -35,6 +35,7 @@
 #include "ServiceWorkerFetchTaskMessages.h"
 #include "WebCoreArgumentCoders.h"
 #include "WebSWContextManagerConnectionMessages.h"
+#include "WebSWServerConnection.h"
 #include <WebCore/SWServer.h>
 #include <WebCore/ServiceWorkerContextData.h>
 
@@ -151,16 +152,18 @@ void WebSWServerToContextConnection::terminate()
     send(Messages::WebSWContextManagerConnection::TerminateProcess());
 }
 
-void WebSWServerToContextConnection::startFetch(PAL::SessionID sessionID, Ref<IPC::Connection>&& contentConnection, WebCore::SWServerConnectionIdentifier serverConnectionIdentifier, FetchIdentifier contentFetchIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, const ResourceRequest& request, const FetchOptions& options, const IPC::FormDataReference& data, const String& referrer)
+void WebSWServerToContextConnection::startFetch(PAL::SessionID sessionID, WebSWServerConnection& contentConnection, FetchIdentifier contentFetchIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, const ResourceRequest& request, const FetchOptions& options, const IPC::FormDataReference& data, const String& referrer)
 {
+    auto serverConnectionIdentifier = contentConnection.identifier();
     auto fetchIdentifier = FetchIdentifier::generate();
-    
-    m_ongoingFetches.add(fetchIdentifier, ServiceWorkerFetchTask::create(sessionID, WTFMove(contentConnection), serverConnectionIdentifier, contentFetchIdentifier));
 
-    ASSERT(!m_ongoingFetchIdentifiers.contains({serverConnectionIdentifier, contentFetchIdentifier}));
-    m_ongoingFetchIdentifiers.add({serverConnectionIdentifier, contentFetchIdentifier}, fetchIdentifier);
+    auto result = m_ongoingFetches.add(fetchIdentifier, ServiceWorkerFetchTask::create(sessionID, contentConnection, *this, contentFetchIdentifier, serviceWorkerIdentifier, m_networkProcess->serviceWorkerFetchTimeout()));
+
+    ASSERT(!m_ongoingFetchIdentifiers.contains({ serverConnectionIdentifier, contentFetchIdentifier }));
+    m_ongoingFetchIdentifiers.add({ serverConnectionIdentifier, contentFetchIdentifier }, fetchIdentifier);
 
-    send(Messages::WebSWContextManagerConnection::StartFetch { serverConnectionIdentifier, serviceWorkerIdentifier, fetchIdentifier, request, options, data, referrer });
+    if (!send(Messages::WebSWContextManagerConnection::StartFetch { serverConnectionIdentifier, serviceWorkerIdentifier, fetchIdentifier, request, options, data, referrer }))
+        result.iterator->value->didNotHandle();
 }
 
 void WebSWServerToContextConnection::cancelFetch(WebCore::SWServerConnectionIdentifier serverConnectionIdentifier, FetchIdentifier contentFetchIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier)
@@ -204,6 +207,43 @@ void WebSWServerToContextConnection::didReceiveFetchTaskMessage(IPC::Connection&
     }
 }
 
+void WebSWServerToContextConnection::fetchTaskTimedOut(ServiceWorkerFetchTask& task)
+{
+    ASSERT(m_ongoingFetchIdentifiers.contains(task.identifier()));
+    auto takenIdentifier = m_ongoingFetchIdentifiers.take(task.identifier());
+
+    ASSERT(m_ongoingFetches.contains(takenIdentifier));
+    auto takenTask = m_ongoingFetches.take(takenIdentifier);
+    ASSERT(takenTask);
+    ASSERT(takenTask->ptr() == &task);
+
+    // Gather all other fetches in this service worker
+    HashSet<Ref<ServiceWorkerFetchTask>> otherFetches;
+    for (auto& fetchTask : m_ongoingFetches.values()) {
+        if (fetchTask->serviceWorkerIdentifier() == task.serviceWorkerIdentifier())
+            otherFetches.add(fetchTask.copyRef());
+    }
+
+    // Signal load failure for them
+    for (auto& fetchTask : otherFetches) {
+        if (fetchTask->wasHandled())
+            fetchTask->fail({ errorDomainWebKitInternal, 0, { }, "Service Worker context closed"_s });
+        else
+            fetchTask->didNotHandle();
+
+        auto identifier = m_ongoingFetchIdentifiers.take(fetchTask->identifier());
+        m_ongoingFetches.remove(identifier);
+    }
+
+    if (m_server) {
+        if (auto* worker = m_server->workerByID(task.serviceWorkerIdentifier())) {
+            worker->setHasTimedOutAnyFetchTasks();
+            if (worker->isRunning())
+                m_server->syncTerminateWorker(*worker);
+        }
+    }
+}
+
 } // namespace WebKit
 
 #endif // ENABLE(SERVICE_WORKER)
index a3c64ed..57bb4f3 100644 (file)
@@ -50,6 +50,7 @@ class SessionID;
 namespace WebKit {
 
 class NetworkProcess;
+class WebSWServerConnection;
 
 class WebSWServerToContextConnection : public WebCore::SWServerToContextConnection, public IPC::MessageSender, public IPC::MessageReceiver {
 public:
@@ -63,7 +64,7 @@ public:
 
     void terminate();
 
-    void startFetch(PAL::SessionID, Ref<IPC::Connection>&&, WebCore::SWServerConnectionIdentifier, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, const WebCore::ResourceRequest&, const WebCore::FetchOptions&, const IPC::FormDataReference&, const String&);
+    void startFetch(PAL::SessionID, WebSWServerConnection&, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, const WebCore::ResourceRequest&, const WebCore::FetchOptions&, const IPC::FormDataReference&, const String&);
     void cancelFetch(WebCore::SWServerConnectionIdentifier, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier);
     void continueDidReceiveFetchResponse(WebCore::SWServerConnectionIdentifier, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier);
 
@@ -72,6 +73,8 @@ public:
     void setThrottleState(bool isThrottleable);
     bool isThrottleable() const { return m_isThrottleable; }
 
+    void fetchTaskTimedOut(ServiceWorkerFetchTask&);
+
 private:
     // IPC::MessageSender
     IPC::Connection* messageSenderConnection() const final;
index cffeb0e..5e3cede 100644 (file)
@@ -408,6 +408,16 @@ void WKContextSetCustomWebContentServiceBundleIdentifier(WKContextRef contextRef
     WebKit::toImpl(contextRef)->setCustomWebContentServiceBundleIdentifier(WebKit::toImpl(name)->string());
 }
 
+void WKContextSetServiceWorkerFetchTimeoutForTesting(WKContextRef contextRef, double seconds)
+{
+    WebKit::toImpl(contextRef)->setServiceWorkerTimeoutForTesting((Seconds)seconds);
+}
+
+void WKContextResetServiceWorkerFetchTimeoutForTesting(WKContextRef contextRef)
+{
+    WebKit::toImpl(contextRef)->resetServiceWorkerTimeoutForTesting();
+}
+
 void WKContextSetDiskCacheSpeculativeValidationEnabled(WKContextRef, bool)
 {
 }
index ed96910..7287355 100644 (file)
@@ -170,6 +170,9 @@ WK_EXPORT void WKContextRefreshPlugIns(WKContextRef context);
 
 WK_EXPORT void WKContextSetCustomWebContentServiceBundleIdentifier(WKContextRef contextRef, WKStringRef name);
 
+WK_EXPORT void WKContextSetServiceWorkerFetchTimeoutForTesting(WKContextRef contextRef, double seconds);
+WK_EXPORT void WKContextResetServiceWorkerFetchTimeoutForTesting(WKContextRef contextRef);
+
 #ifdef __cplusplus
 }
 #endif
index ad407e6..94aefe4 100644 (file)
@@ -1432,6 +1432,16 @@ void WebProcessPool::activePagesOriginsInWebProcessForTesting(ProcessID pid, Com
     completionHandler({ });
 }
 
+void WebProcessPool::setServiceWorkerTimeoutForTesting(Seconds seconds)
+{
+    sendSyncToNetworkingProcess(Messages::NetworkProcess::SetServiceWorkerFetchTimeoutForTesting(seconds), Messages::NetworkProcess::SetServiceWorkerFetchTimeoutForTesting::Reply());
+}
+
+void WebProcessPool::resetServiceWorkerTimeoutForTesting()
+{
+    sendSyncToNetworkingProcess(Messages::NetworkProcess::ResetServiceWorkerFetchTimeoutForTesting(), Messages::NetworkProcess::ResetServiceWorkerFetchTimeoutForTesting::Reply());
+}
+
 void WebProcessPool::setAlwaysUsesComplexTextCodePath(bool alwaysUseComplexText)
 {
     m_alwaysUsesComplexTextCodePath = alwaysUseComplexText;
index 612e392..3c8f639 100644 (file)
@@ -257,6 +257,8 @@ public:
     ProcessID prewarmedProcessIdentifier();
     void activePagesOriginsInWebProcessForTesting(ProcessID, CompletionHandler<void(Vector<String>&&)>&&);
     bool networkProcessHasEntitlementForTesting(const String&);
+    void setServiceWorkerTimeoutForTesting(Seconds);
+    void resetServiceWorkerTimeoutForTesting();
 
     WebPageGroup& defaultPageGroup() { return m_defaultPageGroup.get(); }
 
index ee4020e..4878461 100644 (file)
@@ -1,3 +1,21 @@
+2019-10-08  Brady Eidson  <beidson@apple.com>
+
+        Service Worker Fetch events should time out.
+        https://bugs.webkit.org/show_bug.cgi?id=202188
+
+        Reviewed by Alex Christensen.
+
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::setServiceWorkerFetchTimeout):
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetStateToConsistentValues):
+        (WTR::TestController::setServiceWorkerFetchTimeoutForTesting):
+        * WebKitTestRunner/TestController.h:
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
+
 2019-10-08  Matt Lewis  <jlewis3@apple.com>
 
         Unreviewed, rolling out r250784.
index 761812b..09ea97a 100644 (file)
@@ -390,6 +390,8 @@ interface TestRunner {
 
     void sendDisplayConfigurationChangedMessageForTesting();
 
+    void setServiceWorkerFetchTimeout(double seconds);
+
     // WebAuthN
     void setWebAuthenticationMockConfiguration(object configuration);
     void addTestKeyToKeychain(DOMString privateKeyBase64, DOMString attrLabel, DOMString applicationTagBase64);
index 6a3a31a..8dbc6ba 100644 (file)
@@ -2628,6 +2628,13 @@ void TestRunner::sendDisplayConfigurationChangedMessageForTesting()
     WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), nullptr, nullptr);
 }
 
+void TestRunner::setServiceWorkerFetchTimeout(double seconds)
+{
+    WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("SetServiceWorkerFetchTimeout"));
+    WKRetainPtr<WKDoubleRef> messageBody = adoptWK(WKDoubleCreate(seconds));
+    WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), nullptr);
+}
+
 // WebAuthN
 void TestRunner::setWebAuthenticationMockConfiguration(JSValueRef configurationValue)
 {
index 252643f..3c06353 100644 (file)
@@ -490,6 +490,8 @@ public:
 
     void sendDisplayConfigurationChangedMessageForTesting();
 
+    void setServiceWorkerFetchTimeout(double seconds);
+    
     // WebAuthN
     void setWebAuthenticationMockConfiguration(JSValueRef);
     // FIXME(189876)
index 7cd192a..ecd7d9b 100644 (file)
@@ -964,6 +964,8 @@ bool TestController::resetStateToConsistentValues(const TestOptions& options, Re
 
     WKContextClearCachedCredentials(TestController::singleton().context());
 
+    WKContextResetServiceWorkerFetchTimeoutForTesting(TestController::singleton().context());
+
     WKWebsiteDataStoreClearAllDeviceOrientationPermissions(TestController::websiteDataStore());
 
     clearIndexedDatabases();
@@ -3614,6 +3616,11 @@ void TestController::sendDisplayConfigurationChangedMessageForTesting()
     WKSendDisplayConfigurationChangedMessageForTesting(platformContext());
 }
 
+void TestController::setServiceWorkerFetchTimeoutForTesting(double seconds)
+{
+    WKContextSetServiceWorkerFetchTimeoutForTesting(platformContext(), seconds);
+}
+
 void TestController::setWebAuthenticationMockConfiguration(WKDictionaryRef configuration)
 {
     WKWebsiteDataStoreSetWebAuthenticationMockConfiguration(TestController::websiteDataStore(), configuration);
index 3c71519..530b334 100644 (file)
@@ -294,6 +294,8 @@ public:
     
     void sendDisplayConfigurationChangedMessageForTesting();
 
+    void setServiceWorkerFetchTimeoutForTesting(double seconds);
+
     void setWebAuthenticationMockConfiguration(WKDictionaryRef);
     void addTestKeyToKeychain(const String& privateKeyBase64, const String& attrLabel, const String& applicationTagBase64);
     void cleanUpKeychain(const String& attrLabel, const String& applicationTagBase64);
index 0d12e70..ae8d594 100644 (file)
@@ -1611,6 +1611,13 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
         return nullptr;
     }
 
+    if (WKStringIsEqualToUTF8CString(messageName, "SetServiceWorkerFetchTimeout")) {
+        ASSERT(WKGetTypeID(messageBody) == WKDoubleGetTypeID());
+        WKDoubleRef seconds = static_cast<WKDoubleRef>(messageBody);
+        TestController::singleton().setServiceWorkerFetchTimeoutForTesting(WKDoubleGetValue(seconds));
+        return nullptr;
+    }
+
     if (WKStringIsEqualToUTF8CString(messageName, "TerminateNetworkProcess")) {
         ASSERT(!messageBody);
         TestController::singleton().terminateNetworkProcess();