Diff
Modified: trunk/Source/WebCore/ChangeLog (229614 => 229615)
--- trunk/Source/WebCore/ChangeLog 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebCore/ChangeLog 2018-03-14 22:32:28 UTC (rev 229615)
@@ -1,3 +1,35 @@
+2018-03-14 Chris Dumez <[email protected]>
+
+ Reduce use of SWServerToContextConnection::globalServerToContextConnection()
+ https://bugs.webkit.org/show_bug.cgi?id=183626
+
+ Reviewed by Youenn Fablet.
+
+ Reduce use of SWServerToContextConnection::globalServerToContextConnection() as we are moving towards
+ having multiple context connections.
+
+ No new tests, no expected behavior change.
+
+ * workers/service/server/SWServer.cpp:
+ (WebCore::SWServer::matchAll):
+ (WebCore::SWServer::serverToContextConnectionCreated):
+ (WebCore::SWServer::runServiceWorkerIfNecessary):
+ (WebCore::SWServer::runServiceWorker):
+ (WebCore::SWServer::terminateWorkerInternal):
+ (WebCore::SWServer::markAllWorkersAsTerminated):
+ (WebCore::SWServer::workerContextTerminated):
+ (WebCore::SWServer::fireInstallEvent):
+ (WebCore::SWServer::fireActivateEvent):
+ * workers/service/server/SWServer.h:
+ * workers/service/server/SWServerToContextConnection.cpp:
+ (WebCore::SWServerToContextConnection::findClientByIdentifier):
+ (WebCore::SWServerToContextConnection::matchAll):
+ (WebCore::SWServerToContextConnection::claim):
+ * workers/service/server/SWServerWorker.cpp:
+ (WebCore::SWServerWorker::contextConnection):
+ (WebCore::SWServerWorker::matchAll):
+ * workers/service/server/SWServerWorker.h:
+
2018-03-14 Youenn Fablet <[email protected]>
MessagePort should remove its listeners when being closed
Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (229614 => 229615)
--- trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-03-14 22:32:28 UTC (rev 229615)
@@ -410,7 +410,7 @@
}
// https://w3c.github.io/ServiceWorker/#clients-getall
-void SWServer::matchAll(SWServerWorker& worker, const ServiceWorkerClientQueryOptions& options, ServiceWorkerClientsMatchAllCallback&& callback)
+void SWServer::matchAll(SWServerWorker& worker, const ServiceWorkerClientQueryOptions& options, const ServiceWorkerClientsMatchAllCallback& callback)
{
// FIXME: Support reserved client filtering.
// FIXME: Support WindowClient additional properties.
@@ -510,6 +510,10 @@
void SWServer::serverToContextConnectionCreated(SWServerToContextConnection& contextConnection)
{
+ // FIXME: This will need to update only workers using this connection once we use several context connections.
+ for (auto* worker : SWServerWorker::allWorkers().values())
+ worker->setContextConnectionIdentifier(contextConnection.identifier());
+
auto pendingContextDatas = WTFMove(m_pendingContextDatas);
for (auto& data : pendingContextDatas)
installContextData(data);
@@ -518,7 +522,7 @@
for (auto& item : serviceWorkerRunRequests) {
bool success = runServiceWorker(item.key);
for (auto& callback : item.value)
- callback(success, contextConnection);
+ callback(success ? &contextConnection : nullptr);
}
}
@@ -551,16 +555,20 @@
void SWServer::runServiceWorkerIfNecessary(ServiceWorkerIdentifier identifier, RunServiceWorkerCallback&& callback)
{
- auto* connection = SWServerToContextConnection::globalServerToContextConnection();
- if (auto* worker = m_runningOrTerminatingWorkers.get(identifier)) {
- if (worker->isRunning()) {
- ASSERT(connection);
- callback(true, *connection);
- return;
- }
+ auto* worker = workerByID(identifier);
+ if (!worker) {
+ callback(nullptr);
+ return;
}
- if (!connection) {
+ auto* contextConnection = worker->contextConnection();
+ if (worker->isRunning()) {
+ ASSERT(contextConnection);
+ callback(contextConnection);
+ return;
+ }
+
+ if (!contextConnection) {
m_serviceWorkerRunRequests.ensure(identifier, [&] {
return Vector<RunServiceWorkerCallback> { };
}).iterator->value.append(WTFMove(callback));
@@ -567,7 +575,8 @@
return;
}
- callback(runServiceWorker(identifier), *connection);
+ bool success = runServiceWorker(identifier);
+ callback(success ? contextConnection : nullptr);
}
bool SWServer::runServiceWorker(ServiceWorkerIdentifier identifier)
@@ -586,15 +595,11 @@
worker->setState(SWServerWorker::State::Running);
- auto* connection = SWServerToContextConnection::globalServerToContextConnection();
- ASSERT(connection);
+ auto* contextConnection = worker->contextConnection();
+ ASSERT(contextConnection);
- // When re-running a service worker after a context process crash, the connection identifier may have changed
- // so we update it here.
- worker->setContextConnectionIdentifier(connection->identifier());
+ contextConnection->installServiceWorkerContext(worker->contextData(), m_sessionID);
- connection->installServiceWorkerContext(worker->contextData(), m_sessionID);
-
return true;
}
@@ -617,17 +622,9 @@
worker.setState(SWServerWorker::State::Terminating);
- auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
- ASSERT(contextConnectionIdentifier);
- if (!contextConnectionIdentifier) {
- LOG_ERROR("Request to terminate a worker whose contextConnectionIdentifier is invalid");
- workerContextTerminated(worker);
- return;
- }
-
- auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
- ASSERT(connection);
- if (!connection) {
+ auto* contextConnection = worker.contextConnection();
+ ASSERT(contextConnection);
+ if (!contextConnection) {
LOG_ERROR("Request to terminate a worker whose context connection does not exist");
workerContextTerminated(worker);
return;
@@ -635,10 +632,10 @@
switch (mode) {
case Asynchronous:
- connection->terminateWorker(worker.identifier());
+ contextConnection->terminateWorker(worker.identifier());
break;
case Synchronous:
- connection->syncTerminateWorker(worker.identifier());
+ contextConnection->syncTerminateWorker(worker.identifier());
break;
};
}
@@ -645,14 +642,16 @@
void SWServer::markAllWorkersAsTerminated()
{
- while (!m_runningOrTerminatingWorkers.isEmpty())
- workerContextTerminated(m_runningOrTerminatingWorkers.begin()->value);
+ while (!m_runningOrTerminatingWorkers.isEmpty()) {
+ auto& worker = m_runningOrTerminatingWorkers.begin()->value;
+ worker->setContextConnectionIdentifier(std::nullopt);
+ workerContextTerminated(worker);
+ }
}
void SWServer::workerContextTerminated(SWServerWorker& worker)
{
worker.setState(SWServerWorker::State::NotRunning);
- worker.setContextConnectionIdentifier(std::nullopt);
if (auto* jobQueue = m_jobQueues.get(worker.registrationKey()))
jobQueue->cancelJobsFromServiceWorker(worker.identifier());
@@ -665,35 +664,24 @@
void SWServer::fireInstallEvent(SWServerWorker& worker)
{
- auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
- if (!contextConnectionIdentifier) {
- LOG_ERROR("Request to fire install event on a worker whose contextConnectionIdentifier is invalid");
- return;
- }
- auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
- if (!connection) {
+ auto* contextConnection = worker.contextConnection();
+ if (!contextConnection) {
LOG_ERROR("Request to fire install event on a worker whose context connection does not exist");
return;
}
- connection->fireInstallEvent(worker.identifier());
+ contextConnection->fireInstallEvent(worker.identifier());
}
void SWServer::fireActivateEvent(SWServerWorker& worker)
{
- auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
- if (!contextConnectionIdentifier) {
- LOG_ERROR("Request to fire install event on a worker whose contextConnectionIdentifier is invalid");
- return;
- }
-
- auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
- if (!connection) {
+ auto* contextConnection = worker.contextConnection();
+ if (!contextConnection) {
LOG_ERROR("Request to fire install event on a worker whose context connection does not exist");
return;
}
- connection->fireActivateEvent(worker.identifier());
+ contextConnection->fireActivateEvent(worker.identifier());
}
void SWServer::registerConnection(Connection& connection)
Modified: trunk/Source/WebCore/workers/service/server/SWServer.h (229614 => 229615)
--- trunk/Source/WebCore/workers/service/server/SWServer.h 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebCore/workers/service/server/SWServer.h 2018-03-14 22:32:28 UTC (rev 229615)
@@ -154,7 +154,7 @@
void didFinishInstall(const std::optional<ServiceWorkerJobDataIdentifier>&, SWServerWorker&, bool wasSuccessful);
void didFinishActivation(SWServerWorker&);
void workerContextTerminated(SWServerWorker&);
- void matchAll(SWServerWorker&, const ServiceWorkerClientQueryOptions&, ServiceWorkerClientsMatchAllCallback&&);
+ void matchAll(SWServerWorker&, const ServiceWorkerClientQueryOptions&, const ServiceWorkerClientsMatchAllCallback&);
void claim(SWServerWorker&);
WEBCORE_EXPORT void serverToContextConnectionCreated(SWServerToContextConnection&);
@@ -164,7 +164,7 @@
WEBCORE_EXPORT void registerServiceWorkerClient(ClientOrigin&&, ServiceWorkerClientData&&, const std::optional<ServiceWorkerRegistrationIdentifier>&);
WEBCORE_EXPORT void unregisterServiceWorkerClient(const ClientOrigin&, ServiceWorkerClientIdentifier);
- using RunServiceWorkerCallback = WTF::Function<void(bool, SWServerToContextConnection&)>;
+ using RunServiceWorkerCallback = WTF::Function<void(SWServerToContextConnection*)>;
WEBCORE_EXPORT void runServiceWorkerIfNecessary(ServiceWorkerIdentifier, RunServiceWorkerCallback&&);
void resolveRegistrationReadyRequests(SWServerRegistration&);
Modified: trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.cpp (229614 => 229615)
--- trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.cpp 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.cpp 2018-03-14 22:32:28 UTC (rev 229615)
@@ -111,14 +111,14 @@
void SWServerToContextConnection::findClientByIdentifier(uint64_t requestIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, ServiceWorkerClientIdentifier clientId)
{
if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier))
- globalServerToContextConnection()->findClientByIdentifierCompleted(requestIdentifier, worker->findClientByIdentifier(clientId), false);
+ worker->contextConnection()->findClientByIdentifierCompleted(requestIdentifier, worker->findClientByIdentifier(clientId), false);
}
void SWServerToContextConnection::matchAll(uint64_t requestIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, const ServiceWorkerClientQueryOptions& options)
{
if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier)) {
- worker->matchAll(options, [requestIdentifier] (auto&& data) {
- globalServerToContextConnection()->matchAllCompleted(requestIdentifier, data);
+ worker->matchAll(options, [&] (auto&& data) {
+ worker->contextConnection()->matchAllCompleted(requestIdentifier, data);
});
}
}
@@ -127,7 +127,7 @@
{
if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier)) {
worker->claim();
- globalServerToContextConnection()->claimCompleted(requestIdentifier);
+ worker->contextConnection()->claimCompleted(requestIdentifier);
}
}
Modified: trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp (229614 => 229615)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2018-03-14 22:32:28 UTC (rev 229615)
@@ -32,7 +32,7 @@
namespace WebCore {
-static HashMap<ServiceWorkerIdentifier, SWServerWorker*>& allWorkers()
+HashMap<ServiceWorkerIdentifier, SWServerWorker*>& SWServerWorker::allWorkers()
{
static NeverDestroyed<HashMap<ServiceWorkerIdentifier, SWServerWorker*>> workers;
return workers;
@@ -90,6 +90,14 @@
return *m_origin;
}
+SWServerToContextConnection* SWServerWorker::contextConnection()
+{
+ if (!m_contextConnectionIdentifier)
+ return nullptr;
+
+ return SWServerToContextConnection::connectionForIdentifier(*m_contextConnectionIdentifier);
+}
+
void SWServerWorker::scriptContextFailedToStart(const std::optional<ServiceWorkerJobDataIdentifier>& jobDataIdentifier, const String& message)
{
m_server.scriptContextFailedToStart(jobDataIdentifier, *this, message);
@@ -120,9 +128,9 @@
return m_server.serviceWorkerClientWithOriginByID(origin(), clientId);
}
-void SWServerWorker::matchAll(const ServiceWorkerClientQueryOptions& options, ServiceWorkerClientsMatchAllCallback&& callback)
+void SWServerWorker::matchAll(const ServiceWorkerClientQueryOptions& options, const ServiceWorkerClientsMatchAllCallback& callback)
{
- return m_server.matchAll(*this, options, WTFMove(callback));
+ return m_server.matchAll(*this, options, callback);
}
void SWServerWorker::claim()
Modified: trunk/Source/WebCore/workers/service/server/SWServerWorker.h (229614 => 229615)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2018-03-14 22:32:28 UTC (rev 229615)
@@ -40,6 +40,7 @@
struct ClientOrigin;
class SWServer;
class SWServerRegistration;
+class SWServerToContextConnection;
struct ServiceWorkerClientData;
struct ServiceWorkerClientIdentifier;
struct ServiceWorkerClientQueryOptions;
@@ -78,7 +79,6 @@
ServiceWorkerIdentifier identifier() const { return m_data.identifier; }
- std::optional<SWServerToContextConnectionIdentifier> contextConnectionIdentifier() const { return m_contextConnectionIdentifier; }
void setContextConnectionIdentifier(std::optional<SWServerToContextConnectionIdentifier> identifier) { m_contextConnectionIdentifier = identifier; }
ServiceWorkerState state() const { return m_data.state; }
@@ -93,7 +93,7 @@
void didFinishActivation();
void contextTerminated();
WEBCORE_EXPORT std::optional<ServiceWorkerClientData> findClientByIdentifier(const ServiceWorkerClientIdentifier&) const;
- void matchAll(const ServiceWorkerClientQueryOptions&, ServiceWorkerClientsMatchAllCallback&&);
+ void matchAll(const ServiceWorkerClientQueryOptions&, const ServiceWorkerClientsMatchAllCallback&);
void claim();
void skipWaiting();
@@ -100,11 +100,14 @@
bool isSkipWaitingFlagSet() const { return m_isSkipWaitingFlagSet; }
WEBCORE_EXPORT static SWServerWorker* existingWorkerForIdentifier(ServiceWorkerIdentifier);
+ static HashMap<ServiceWorkerIdentifier, SWServerWorker*>& allWorkers();
const ServiceWorkerData& data() const { return m_data; }
ServiceWorkerContextData contextData() const;
const ClientOrigin& origin() const;
+ WEBCORE_EXPORT SWServerToContextConnection* contextConnection();
+
private:
SWServerWorker(SWServer&, SWServerRegistration&, std::optional<SWServerToContextConnectionIdentifier>, const URL&, const String& script, const ContentSecurityPolicyResponseHeaders&, WorkerType, ServiceWorkerIdentifier);
Modified: trunk/Source/WebKit/ChangeLog (229614 => 229615)
--- trunk/Source/WebKit/ChangeLog 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebKit/ChangeLog 2018-03-14 22:32:28 UTC (rev 229615)
@@ -1,5 +1,19 @@
2018-03-14 Chris Dumez <[email protected]>
+ Reduce use of SWServerToContextConnection::globalServerToContextConnection()
+ https://bugs.webkit.org/show_bug.cgi?id=183626
+
+ Reviewed by Youenn Fablet.
+
+ Reduce use of SWServerToContextConnection::globalServerToContextConnection() as we are moving
+ towards having multiple context connections.
+
+ * StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
+ (WebKit::WebSWServerConnection::startFetch):
+ (WebKit::WebSWServerConnection::postMessageToServiceWorker):
+
+2018-03-14 Chris Dumez <[email protected]>
+
Make policy decisions asynchronous
https://bugs.webkit.org/show_bug.cgi?id=180568
<rdar://problem/37131297>
Modified: trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp (229614 => 229615)
--- trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp 2018-03-14 22:32:28 UTC (rev 229615)
@@ -154,13 +154,13 @@
if (!StorageProcess::singleton().globalServerToContextConnection())
StorageProcess::singleton().createServerToContextConnection(server().sessionID());
- server().runServiceWorkerIfNecessary(serviceWorkerIdentifier, [weakThis = WTFMove(weakThis), this, fetchIdentifier, serviceWorkerIdentifier, request = WTFMove(request), options = WTFMove(options), formData = WTFMove(formData), referrer = WTFMove(referrer)](bool success, auto& contextConnection) {
+ server().runServiceWorkerIfNecessary(serviceWorkerIdentifier, [weakThis = WTFMove(weakThis), this, fetchIdentifier, serviceWorkerIdentifier, request = WTFMove(request), options = WTFMove(options), formData = WTFMove(formData), referrer = WTFMove(referrer)](auto* contextConnection) {
if (!weakThis)
return;
- if (success) {
+ if (contextConnection) {
SWSERVERCONNECTION_RELEASE_LOG_IF_ALLOWED("startFetch: Starting fetch %llu via service worker %llu", fetchIdentifier, serviceWorkerIdentifier.toUInt64());
- sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::StartFetch { this->identifier(), fetchIdentifier, serviceWorkerIdentifier, request, options, formData, referrer });
+ sendToContextProcess(*contextConnection, Messages::WebSWContextManagerConnection::StartFetch { this->identifier(), fetchIdentifier, serviceWorkerIdentifier, request, options, formData, referrer });
} else {
SWSERVERCONNECTION_RELEASE_LOG_ERROR_IF_ALLOWED("startFetch: fetchIdentifier: %llu -> DidNotHandle because failed to run service worker", fetchIdentifier);
m_contentConnection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, fetchIdentifier);
@@ -198,9 +198,9 @@
StorageProcess::singleton().createServerToContextConnection(server().sessionID());
// It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
- server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceData = WTFMove(*sourceData)](bool success, auto& contextConnection) mutable {
- if (success)
- sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::PostMessageToServiceWorker { destinationIdentifier, WTFMove(message), WTFMove(sourceData) });
+ server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceData = WTFMove(*sourceData)](auto* contextConnection) mutable {
+ if (contextConnection)
+ sendToContextProcess(*contextConnection, Messages::WebSWContextManagerConnection::PostMessageToServiceWorker { destinationIdentifier, WTFMove(message), WTFMove(sourceData) });
});
}
@@ -295,12 +295,6 @@
m_clientOrigins.remove(iterator);
}
-template<typename U> void WebSWServerConnection::sendToContextProcess(U&& message)
-{
- if (auto* connection = StorageProcess::singleton().globalServerToContextConnection())
- connection->send(WTFMove(message));
-}
-
template<typename U> void WebSWServerConnection::sendToContextProcess(WebCore::SWServerToContextConnection& connection, U&& message)
{
static_cast<WebSWServerToContextConnection&>(connection).send(WTFMove(message));
Modified: trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h (229614 => 229615)
--- trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h 2018-03-14 22:20:33 UTC (rev 229614)
+++ trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h 2018-03-14 22:32:28 UTC (rev 229615)
@@ -98,7 +98,6 @@
IPC::Connection* messageSenderConnection() final { return m_contentConnection.ptr(); }
uint64_t messageSenderDestinationID() final { return identifier().toUInt64(); }
- template<typename U> void sendToContextProcess(U&& message);
template<typename U> static void sendToContextProcess(WebCore::SWServerToContextConnection&, U&& message);
PAL::SessionID m_sessionID;
