Diff
Modified: trunk/LayoutTests/ChangeLog (250984 => 250985)
--- trunk/LayoutTests/ChangeLog 2019-10-10 19:49:27 UTC (rev 250984)
+++ trunk/LayoutTests/ChangeLog 2019-10-10 19:52:04 UTC (rev 250985)
@@ -1,3 +1,17 @@
+2019-10-10 Youenn Fablet <you...@apple.com>
+
+ 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.
+
+ * 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):
+
2019-10-10 Myles C. Maxfield <mmaxfi...@apple.com>
FontFaceSet's ready promise is not always resolved
Added: trunk/LayoutTests/http/wpt/service-workers/fetch-timeout-worker.js (0 => 250985)
--- trunk/LayoutTests/http/wpt/service-workers/fetch-timeout-worker.js (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/fetch-timeout-worker.js 2019-10-10 19:52:04 UTC (rev 250985)
@@ -0,0 +1,6 @@
+async function doTest(event)
+{
+ event.respondWith(fetch(event.request));
+}
+
+self.addEventListener("fetch", doTest);
Added: trunk/LayoutTests/http/wpt/service-workers/fetch-timeout.https-expected.txt (0 => 250985)
--- trunk/LayoutTests/http/wpt/service-workers/fetch-timeout.https-expected.txt (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/fetch-timeout.https-expected.txt 2019-10-10 19:52:04 UTC (rev 250985)
@@ -0,0 +1,4 @@
+
+PASS Setup worker
+PASS Make sure a load that makes progress does not time out
+
Added: trunk/LayoutTests/http/wpt/service-workers/fetch-timeout.https.html (0 => 250985)
--- trunk/LayoutTests/http/wpt/service-workers/fetch-timeout.https.html (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/fetch-timeout.https.html 2019-10-10 19:52:04 UTC (rev 250985)
@@ -0,0 +1,39 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+var scope = "resources";
+var activeWorker;
+
+promise_test(async (test) => {
+ var registration = await navigator.serviceWorker.register("fetch-timeout-worker.js", { scope : scope });
+ activeWorker = registration.active;
+ if (activeWorker)
+ return;
+ activeWorker = registration.installing;
+ return new Promise(resolve => {
+ activeWorker.addEventListener('statechange', () => {
+ if (activeWorker.state === "activated")
+ resolve();
+ });
+ });
+}, "Setup worker");
+
+promise_test(async (test) => {
+ const iframe = await with_iframe(scope);
+
+ if (window.testRunner)
+ testRunner.setServiceWorkerFetchTimeout(1);
+
+ const response = await iframe.contentWindow.fetch("/WebKit/service-workers/resources/lengthy-pass.py?delay=0.5");
+ const text = await response.text();
+ assert_equals(text, "document.body.innerHTML = 'PASS'");
+ iframe.remove();
+}, "Make sure a load that makes progress does not time out");
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/http/wpt/service-workers/resources/lengthy-pass.py (250984 => 250985)
--- trunk/LayoutTests/http/wpt/service-workers/resources/lengthy-pass.py 2019-10-10 19:49:27 UTC (rev 250984)
+++ trunk/LayoutTests/http/wpt/service-workers/resources/lengthy-pass.py 2019-10-10 19:52:04 UTC (rev 250985)
@@ -2,6 +2,9 @@
def main(request, response):
delay = 0.05
+ headers = []
+ if "delay" in request.GET:
+ delay = float(request.GET.first("delay"))
response.headers.set("Content-type", "text/_javascript_")
response.headers.append("Access-Control-Allow-Origin", "*")
response.write_status_headers()
Modified: trunk/Source/WebKit/ChangeLog (250984 => 250985)
--- trunk/Source/WebKit/ChangeLog 2019-10-10 19:49:27 UTC (rev 250984)
+++ trunk/Source/WebKit/ChangeLog 2019-10-10 19:52:04 UTC (rev 250985)
@@ -1,3 +1,37 @@
+2019-10-10 Youenn Fablet <you...@apple.com>
+
+ 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.
+
+ 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:
+
2019-10-10 Rob Buis <rb...@igalia.com>
SpeculativeLoad should use CompletionHandler
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp (250984 => 250985)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp 2019-10-10 19:49:27 UTC (rev 250984)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp 2019-10-10 19:52:04 UTC (rev 250985)
@@ -59,6 +59,7 @@
void ServiceWorkerFetchTask::didReceiveRedirectResponse(const ResourceResponse& response)
{
RELEASE_LOG_IF_ALLOWED("didReceiveRedirectResponse: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
+ m_timeoutTimer.stop();
m_wasHandled = true;
if (m_connection)
m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveRedirectResponse { response }, m_identifier.fetchIdentifier);
@@ -67,6 +68,7 @@
void ServiceWorkerFetchTask::didReceiveResponse(const ResourceResponse& response, bool needsContinueDidReceiveResponseMessage)
{
RELEASE_LOG_IF_ALLOWED("didReceiveResponse: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
+ m_timeoutTimer.stop();
m_wasHandled = true;
if (m_connection)
m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveResponse { response, needsContinueDidReceiveResponseMessage }, m_identifier.fetchIdentifier);
@@ -74,6 +76,7 @@
void ServiceWorkerFetchTask::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
{
+ ASSERT(!m_timeoutTimer.isActive());
if (m_connection)
m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveData { data, encodedDataLength }, m_identifier.fetchIdentifier);
}
@@ -80,6 +83,7 @@
void ServiceWorkerFetchTask::didReceiveFormData(const IPC::FormDataReference& formData)
{
+ ASSERT(!m_timeoutTimer.isActive());
if (m_connection)
m_connection->send(Messages::ServiceWorkerClientFetch::DidReceiveFormData { formData }, m_identifier.fetchIdentifier);
}
@@ -86,11 +90,11 @@
void ServiceWorkerFetchTask::didFinish()
{
+ ASSERT(!m_timeoutTimer.isActive());
RELEASE_LOG_IF_ALLOWED("didFinishFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
m_timeoutTimer.stop();
- if (!m_didReachTerminalState && m_connection)
+ if (m_connection)
m_connection->send(Messages::ServiceWorkerClientFetch::DidFinish { }, m_identifier.fetchIdentifier);
- m_didReachTerminalState = true;
}
void ServiceWorkerFetchTask::didFail(const ResourceError& error)
@@ -97,9 +101,8 @@
{
RELEASE_LOG_ERROR_IF_ALLOWED("didFailFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
m_timeoutTimer.stop();
- if (!m_didReachTerminalState && m_connection)
+ if (m_connection)
m_connection->send(Messages::ServiceWorkerClientFetch::DidFail { error }, m_identifier.fetchIdentifier);
- m_didReachTerminalState = true;
}
void ServiceWorkerFetchTask::didNotHandle()
@@ -106,19 +109,14 @@
{
RELEASE_LOG_IF_ALLOWED("didNotHandleFetch: fetchIdentifier: %s", m_identifier.fetchIdentifier.loggingString().utf8().data());
m_timeoutTimer.stop();
- if (!m_didReachTerminalState && m_connection)
+ if (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 });
-
+ didNotHandle();
m_contextConnection.fetchTaskTimedOut(*this);
}
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h (250984 => 250985)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h 2019-10-10 19:49:27 UTC (rev 250984)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h 2019-10-10 19:52:04 UTC (rev 250985)
@@ -31,7 +31,6 @@
#include <WebCore/ServiceWorkerTypes.h>
#include <WebCore/Timer.h>
#include <pal/SessionID.h>
-#include <wtf/RefCounted.h>
namespace WebCore {
class ResourceError;
@@ -50,12 +49,10 @@
class WebSWServerConnection;
class WebSWServerToContextConnection;
-class ServiceWorkerFetchTask : public RefCounted<ServiceWorkerFetchTask> {
+class ServiceWorkerFetchTask {
+ WTF_MAKE_FAST_ALLOCATED;
public:
- template<typename... Args> static Ref<ServiceWorkerFetchTask> create(Args&&... args)
- {
- return adoptRef(*new ServiceWorkerFetchTask(std::forward<Args>(args)...));
- }
+ ServiceWorkerFetchTask(PAL::SessionID, WebSWServerConnection&, WebSWServerToContextConnection&, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, Seconds timeout);
void didNotHandle();
void fail(const WebCore::ResourceError& error) { didFail(error); }
@@ -79,8 +76,6 @@
bool wasHandled() const { return m_wasHandled; }
private:
- ServiceWorkerFetchTask(PAL::SessionID, WebSWServerConnection&, WebSWServerToContextConnection&, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, Seconds timeout);
-
void didReceiveRedirectResponse(const WebCore::ResourceResponse&);
void didReceiveResponse(const WebCore::ResourceResponse&, bool needsContinueDidReceiveResponseMessage);
void didReceiveData(const IPC::DataReference&, int64_t encodedDataLength);
@@ -97,7 +92,6 @@
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)
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp (250984 => 250985)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp 2019-10-10 19:49:27 UTC (rev 250984)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp 2019-10-10 19:52:04 UTC (rev 250985)
@@ -157,7 +157,7 @@
auto serverConnectionIdentifier = contentConnection.identifier();
auto fetchIdentifier = FetchIdentifier::generate();
- auto result = m_ongoingFetches.add(fetchIdentifier, ServiceWorkerFetchTask::create(sessionID, contentConnection, *this, contentFetchIdentifier, serviceWorkerIdentifier, m_networkProcess->serviceWorkerFetchTimeout()));
+ auto result = m_ongoingFetches.add(fetchIdentifier, makeUnique<ServiceWorkerFetchTask>(sessionID, contentConnection, *this, contentFetchIdentifier, serviceWorkerIdentifier, m_networkProcess->serviceWorkerFetchTimeout()));
ASSERT(!m_ongoingFetchIdentifiers.contains({ serverConnectionIdentifier, contentFetchIdentifier }));
m_ongoingFetchIdentifiers.add({ serverConnectionIdentifier, contentFetchIdentifier }, fetchIdentifier);
@@ -215,17 +215,17 @@
ASSERT(m_ongoingFetches.contains(takenIdentifier));
auto takenTask = m_ongoingFetches.take(takenIdentifier);
ASSERT(takenTask);
- ASSERT(takenTask->ptr() == &task);
+ ASSERT(takenTask.get() == &task);
// Gather all other fetches in this service worker
- HashSet<Ref<ServiceWorkerFetchTask>> otherFetches;
+ Vector<ServiceWorkerFetchTask*> otherFetches;
for (auto& fetchTask : m_ongoingFetches.values()) {
if (fetchTask->serviceWorkerIdentifier() == task.serviceWorkerIdentifier())
- otherFetches.add(fetchTask.copyRef());
+ otherFetches.append(fetchTask.get());
}
// Signal load failure for them
- for (auto& fetchTask : otherFetches) {
+ for (auto* fetchTask : otherFetches) {
if (fetchTask->wasHandled())
fetchTask->fail({ errorDomainWebKitInternal, 0, { }, "Service Worker context closed"_s });
else
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h (250984 => 250985)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h 2019-10-10 19:49:27 UTC (rev 250984)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h 2019-10-10 19:52:04 UTC (rev 250985)
@@ -103,7 +103,7 @@
WeakPtr<WebCore::SWServer> m_server;
HashMap<ServiceWorkerFetchTask::Identifier, WebCore::FetchIdentifier> m_ongoingFetchIdentifiers;
- HashMap<WebCore::FetchIdentifier, Ref<ServiceWorkerFetchTask>> m_ongoingFetches;
+ HashMap<WebCore::FetchIdentifier, std::unique_ptr<ServiceWorkerFetchTask>> m_ongoingFetches;
bool m_isThrottleable { true };
}; // class WebSWServerToContextConnection
