Title: [253528] trunk/Source/WebKit
- Revision
- 253528
- Author
- cdu...@apple.com
- Date
- 2019-12-14 13:49:11 -0800 (Sat, 14 Dec 2019)
Log Message
WebSWServerConnection::startFetch() should never fail synchronously
https://bugs.webkit.org/show_bug.cgi?id=205225
<rdar://problem/57490508>
Reviewed by Geoffrey Garen.
WebSWServerConnection::startFetch() should never fail synchronously. If it does, it will
confuse the NetworkResourceLoader. NetworkResourceLoader::serviceWorkerDidNotHandle() will
get called *before* NetworkResourceLoader::m_serviceWorkerFetchTask has been sent, which
means that we would not properly deal with redirects. Worse, the call site which creates
the ServiceWorkerFetchTask would then null out m_networkLoad, which would silently cancel
the load that WebSWServerConnection::startFetch() started synchronously.
No new tests, I am not sure how to test this. It would have to cause a redirect do a URL
that has a service worker and then we would have to get WebSWServerConnection::startFetch()
to fail synchronously. I think that to fail synchronously, we would have to not find a
ServiceWorker with the given ID, or the ServiceWorker's hasTimedOutAnyFetchTasks would have
to be set.
I added a RELEASE_ASSERT() in NetworkResourceLoader::serviceWorkerDidNotHandle() that would
have caught this.
* NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
(WebKit::WebSWServerConnection::startFetch):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (253527 => 253528)
--- trunk/Source/WebKit/ChangeLog 2019-12-14 20:18:31 UTC (rev 253527)
+++ trunk/Source/WebKit/ChangeLog 2019-12-14 21:49:11 UTC (rev 253528)
@@ -1,3 +1,30 @@
+2019-12-14 Chris Dumez <cdu...@apple.com>
+
+ WebSWServerConnection::startFetch() should never fail synchronously
+ https://bugs.webkit.org/show_bug.cgi?id=205225
+ <rdar://problem/57490508>
+
+ Reviewed by Geoffrey Garen.
+
+ WebSWServerConnection::startFetch() should never fail synchronously. If it does, it will
+ confuse the NetworkResourceLoader. NetworkResourceLoader::serviceWorkerDidNotHandle() will
+ get called *before* NetworkResourceLoader::m_serviceWorkerFetchTask has been sent, which
+ means that we would not properly deal with redirects. Worse, the call site which creates
+ the ServiceWorkerFetchTask would then null out m_networkLoad, which would silently cancel
+ the load that WebSWServerConnection::startFetch() started synchronously.
+
+ No new tests, I am not sure how to test this. It would have to cause a redirect do a URL
+ that has a service worker and then we would have to get WebSWServerConnection::startFetch()
+ to fail synchronously. I think that to fail synchronously, we would have to not find a
+ ServiceWorker with the given ID, or the ServiceWorker's hasTimedOutAnyFetchTasks would have
+ to be set.
+
+ I added a RELEASE_ASSERT() in NetworkResourceLoader::serviceWorkerDidNotHandle() that would
+ have caught this.
+
+ * NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
+ (WebKit::WebSWServerConnection::startFetch):
+
2019-12-14 Simon Fraser <simon.fra...@apple.com>
Move code out of WKWebView.mm into platform-specific files, for easier navigation
Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (253527 => 253528)
--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp 2019-12-14 20:18:31 UTC (rev 253527)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp 2019-12-14 21:49:11 UTC (rev 253528)
@@ -1265,11 +1265,12 @@
if (m_serviceWorkerFetchTask)
return;
- serviceWorkerDidNotHandle();
+ serviceWorkerDidNotHandle(nullptr);
}
-void NetworkResourceLoader::serviceWorkerDidNotHandle()
+void NetworkResourceLoader::serviceWorkerDidNotHandle(ServiceWorkerFetchTask* fetchTask)
{
+ RELEASE_ASSERT(m_serviceWorkerFetchTask.get() == fetchTask);
if (m_parameters.serviceWorkersMode == ServiceWorkersMode::Only) {
send(Messages::WebResourceLoader::ServiceWorkerDidNotHandle { }, identifier());
abort();
Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h (253527 => 253528)
--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h 2019-12-14 20:18:31 UTC (rev 253527)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h 2019-12-14 21:49:11 UTC (rev 253528)
@@ -129,7 +129,7 @@
#if ENABLE(SERVICE_WORKER)
void startWithServiceWorker();
- void serviceWorkerDidNotHandle();
+ void serviceWorkerDidNotHandle(ServiceWorkerFetchTask*);
#endif
private:
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp (253527 => 253528)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp 2019-12-14 20:18:31 UTC (rev 253527)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp 2019-12-14 21:49:11 UTC (rev 253528)
@@ -165,7 +165,7 @@
m_timeoutTimer.stop();
softUpdateIfNeeded();
- m_loader.serviceWorkerDidNotHandle();
+ m_loader.serviceWorkerDidNotHandle(this);
}
void ServiceWorkerFetchTask::cancelFromClient()
@@ -183,7 +183,7 @@
{
m_timeoutTimer.startOneShot(m_loader.connectionToWebProcess().networkProcess().serviceWorkerFetchTimeout());
if (!m_serviceWorkerConnection) {
- m_loader.serviceWorkerDidNotHandle();
+ m_loader.serviceWorkerDidNotHandle(this);
return;
}
m_currentRequest = WTFMove(request);
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp (253527 => 253528)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp 2019-12-14 20:18:31 UTC (rev 253527)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp 2019-12-14 21:49:11 UTC (rev 253528)
@@ -242,7 +242,10 @@
}
ASSERT(worker.state() == ServiceWorkerState::Activated);
- runServerWorkerAndStartFetch(true);
+ // Make sure we start the fetch asynchronously because failing synchronously would get the NetworkResourceLoader in a bad state.
+ RunLoop::main().dispatch([runServerWorkerAndStartFetch = WTFMove(runServerWorkerAndStartFetch)]() mutable {
+ runServerWorkerAndStartFetch(true);
+ });
}
void WebSWServerConnection::postMessageToServiceWorker(ServiceWorkerIdentifier destinationIdentifier, MessageWithMessagePorts&& message, const ServiceWorkerOrClientIdentifier& sourceIdentifier)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
