SWClientConnection should not keep references to service worker jobs
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jan 2018 12:48:34 +0000 (12:48 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jan 2018 12:48:34 +0000 (12:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181381

Patch by Youenn Fablet <youenn@apple.com> on 2018-01-09
Reviewed by Chris Dumez.

Source/WebCore:

Difficult to test determiniscally but corresponding crash log should no longer happen in debug builds.

Stopped passing ServiceWorkerJob references from ServiceWorkerContainer (potentially in service worker thread) to SWClientConnection (main thread).
Instead pass job identifiers and related data to the main thread.

Minor refactoring to use ServiceWorkerJobIdentifier instead of ServiceWorkerJobDataIdentifier which contains more data than needed.

* workers/service/SWClientConnection.cpp:
(WebCore::SWClientConnection::scheduleJob):
(WebCore::SWClientConnection::failedFetchingScript):
(WebCore::SWClientConnection::postTaskForJob):
(WebCore::SWClientConnection::jobRejectedInServer):
(WebCore::SWClientConnection::registrationJobResolvedInServer):
(WebCore::SWClientConnection::unregistrationJobResolvedInServer):
(WebCore::SWClientConnection::startScriptFetchForServer):
(WebCore::SWClientConnection::clearPendingJobs):
(WebCore::SWClientConnection::finishedFetchingScript): Deleted.
* workers/service/SWClientConnection.h:
* workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::scheduleJob):
(WebCore::ServiceWorkerContainer::startScriptFetchForJob):
(WebCore::ServiceWorkerContainer::jobFinishedLoadingScript):
(WebCore::ServiceWorkerContainer::jobFailedLoadingScript):
* workers/service/ServiceWorkerContainer.h:
* workers/service/server/SWServer.cpp:
(WebCore::SWServer::rejectJob):
(WebCore::SWServer::resolveRegistrationJob):
(WebCore::SWServer::resolveUnregistrationJob):
(WebCore::SWServer::startScriptFetch):
* workers/service/server/SWServer.h:

Source/WebKit:

Updated IPC handling based on WebCore refactoring.

* Scripts/webkit/messages.py:
(forward_declarations_and_headers):
(headers_for_type):
* StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
(WebKit::WebSWServerConnection::rejectJobInClient):
(WebKit::WebSWServerConnection::resolveRegistrationJobInClient):
(WebKit::WebSWServerConnection::resolveUnregistrationJobInClient):
(WebKit::WebSWServerConnection::startScriptFetchInClient):
* StorageProcess/ServiceWorker/WebSWServerConnection.h:
* WebProcess/Storage/WebSWClientConnection.messages.in:

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

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/workers/service/SWClientConnection.cpp
Source/WebCore/workers/service/SWClientConnection.h
Source/WebCore/workers/service/ServiceWorkerContainer.cpp
Source/WebCore/workers/service/ServiceWorkerContainer.h
Source/WebCore/workers/service/server/SWServer.cpp
Source/WebCore/workers/service/server/SWServer.h
Source/WebKit/ChangeLog
Source/WebKit/Scripts/webkit/messages.py
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h
Source/WebKit/WebProcess/Storage/WebSWClientConnection.messages.in

index 5c31497..f482c15 100644 (file)
@@ -1,3 +1,41 @@
+2018-01-09  Youenn Fablet  <youenn@apple.com>
+
+        SWClientConnection should not keep references to service worker jobs
+        https://bugs.webkit.org/show_bug.cgi?id=181381
+
+        Reviewed by Chris Dumez.
+
+        Difficult to test determiniscally but corresponding crash log should no longer happen in debug builds.
+
+        Stopped passing ServiceWorkerJob references from ServiceWorkerContainer (potentially in service worker thread) to SWClientConnection (main thread).
+        Instead pass job identifiers and related data to the main thread.
+
+        Minor refactoring to use ServiceWorkerJobIdentifier instead of ServiceWorkerJobDataIdentifier which contains more data than needed.
+
+        * workers/service/SWClientConnection.cpp:
+        (WebCore::SWClientConnection::scheduleJob):
+        (WebCore::SWClientConnection::failedFetchingScript):
+        (WebCore::SWClientConnection::postTaskForJob):
+        (WebCore::SWClientConnection::jobRejectedInServer):
+        (WebCore::SWClientConnection::registrationJobResolvedInServer):
+        (WebCore::SWClientConnection::unregistrationJobResolvedInServer):
+        (WebCore::SWClientConnection::startScriptFetchForServer):
+        (WebCore::SWClientConnection::clearPendingJobs):
+        (WebCore::SWClientConnection::finishedFetchingScript): Deleted.
+        * workers/service/SWClientConnection.h:
+        * workers/service/ServiceWorkerContainer.cpp:
+        (WebCore::ServiceWorkerContainer::scheduleJob):
+        (WebCore::ServiceWorkerContainer::startScriptFetchForJob):
+        (WebCore::ServiceWorkerContainer::jobFinishedLoadingScript):
+        (WebCore::ServiceWorkerContainer::jobFailedLoadingScript):
+        * workers/service/ServiceWorkerContainer.h:
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::rejectJob):
+        (WebCore::SWServer::resolveRegistrationJob):
+        (WebCore::SWServer::resolveUnregistrationJob):
+        (WebCore::SWServer::startScriptFetch):
+        * workers/service/server/SWServer.h:
+
 2018-01-09  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         REGRESSION(r224460): Text fields sometimes get "messed up"
index 1f3221f..f931f37 100644 (file)
@@ -38,7 +38,6 @@
 #include "ServiceWorkerJobData.h"
 #include "ServiceWorkerRegistration.h"
 #include <wtf/CrossThreadCopier.h>
-#include <wtf/Scope.h>
 
 namespace WebCore {
 
@@ -46,103 +45,77 @@ SWClientConnection::SWClientConnection() = default;
 
 SWClientConnection::~SWClientConnection() = default;
 
-void SWClientConnection::scheduleJob(ServiceWorkerJob& job)
+void SWClientConnection::scheduleJob(DocumentOrWorkerIdentifier contextIdentifier, const ServiceWorkerJobData& jobData)
 {
     ASSERT(isMainThread());
 
-    auto addResult = m_scheduledJobs.add(job.identifier(), &job);
+    auto addResult = m_scheduledJobSources.add(jobData.identifier().jobIdentifier, contextIdentifier);
     ASSERT_UNUSED(addResult, addResult.isNewEntry);
 
-    scheduleJobInServer(job.data());
+    scheduleJobInServer(jobData);
 }
 
-void SWClientConnection::finishedFetchingScript(ServiceWorkerJob& job, const String& script)
+void SWClientConnection::failedFetchingScript(ServiceWorkerJobIdentifier jobIdentifier, const ServiceWorkerRegistrationKey& registrationKey, const ResourceError& error)
 {
     ASSERT(isMainThread());
-    ASSERT(m_scheduledJobs.get(job.identifier()) == &job);
 
-    finishFetchingScriptInServer({ job.data().identifier(), job.data().registrationKey(), script, { } });
+    finishFetchingScriptInServer({ { serverConnectionIdentifier(), jobIdentifier }, registrationKey, { }, error });
 }
 
-void SWClientConnection::failedFetchingScript(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, const ServiceWorkerRegistrationKey& registrationKey, const ResourceError& error)
+bool SWClientConnection::postTaskForJob(ServiceWorkerJobIdentifier jobIdentifier, IsJobComplete isJobComplete, WTF::Function<void(ServiceWorkerJob&)>&& task)
 {
     ASSERT(isMainThread());
 
-    finishFetchingScriptInServer({ jobDataIdentifier, registrationKey, { }, error });
-}
-
-void SWClientConnection::jobRejectedInServer(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, const ExceptionData& exceptionData)
-{
-    ASSERT(isMainThread());
-
-    auto job = m_scheduledJobs.take(jobDataIdentifier.jobIdentifier);
-    if (!job) {
-        LOG_ERROR("Job %s rejected from server, but was not found", jobDataIdentifier.loggingString().utf8().data());
-        return;
+    auto iterator = m_scheduledJobSources.find(jobIdentifier);
+    if (iterator == m_scheduledJobSources.end()) {
+        LOG_ERROR("Job %s was not found", jobIdentifier.loggingString().utf8().data());
+        return false;
     }
-
-    ScriptExecutionContext::postTaskTo(job->contextIdentifier(), [job, exceptionData = exceptionData.isolatedCopy()](ScriptExecutionContext&) {
-        job->failedWithException(exceptionData.toException());
+    auto isPosted = ScriptExecutionContext::postTaskTo(iterator->value, [jobIdentifier, task = WTFMove(task)] (ScriptExecutionContext& context) mutable {
+        if (auto* container = context.serviceWorkerContainer()) {
+            if (auto* job = container->job(jobIdentifier))
+                task(*job);
+        }
     });
+    if (isJobComplete == IsJobComplete::Yes)
+        m_scheduledJobSources.remove(iterator);
+    return isPosted;
 }
 
-void SWClientConnection::registrationJobResolvedInServer(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, ServiceWorkerRegistrationData&& registrationData, ShouldNotifyWhenResolved shouldNotifyWhenResolved)
+void SWClientConnection::jobRejectedInServer(ServiceWorkerJobIdentifier jobIdentifier, const ExceptionData& exceptionData)
 {
-    ASSERT(isMainThread());
-
-    auto guard = WTF::makeScopeExit([this, shouldNotifyWhenResolved, registrationKey = registrationData.key] {
-        if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
-            didResolveRegistrationPromise(registrationKey);
+    postTaskForJob(jobIdentifier, IsJobComplete::Yes, [exceptionData = exceptionData.isolatedCopy()] (auto& job) {
+        job.failedWithException(exceptionData.toException());
     });
+}
 
-    auto job = m_scheduledJobs.take(jobDataIdentifier.jobIdentifier);
-    if (!job) {
-        LOG_ERROR("Job %s resolved in server, but was not found", jobDataIdentifier.loggingString().utf8().data());
-        return;
-    }
-
-    bool wasPosted = ScriptExecutionContext::postTaskTo(job->contextIdentifier(), [job, registrationData = registrationData.isolatedCopy(), shouldNotifyWhenResolved](ScriptExecutionContext&) mutable {
-        job->resolvedWithRegistration(WTFMove(registrationData), shouldNotifyWhenResolved);
+void SWClientConnection::registrationJobResolvedInServer(ServiceWorkerJobIdentifier jobIdentifier, ServiceWorkerRegistrationData&& registrationData, ShouldNotifyWhenResolved shouldNotifyWhenResolved)
+{
+    bool isPosted = postTaskForJob(jobIdentifier, IsJobComplete::Yes, [registrationData = registrationData.isolatedCopy(), shouldNotifyWhenResolved] (auto& job) mutable {
+        job.resolvedWithRegistration(WTFMove(registrationData), shouldNotifyWhenResolved);
     });
 
-    if (wasPosted)
-        guard.release();
+    if (!isPosted && shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
+        didResolveRegistrationPromise(registrationData.key);
 }
 
-void SWClientConnection::unregistrationJobResolvedInServer(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, bool unregistrationResult)
+void SWClientConnection::unregistrationJobResolvedInServer(ServiceWorkerJobIdentifier jobIdentifier, bool unregistrationResult)
 {
-    ASSERT(isMainThread());
-
-    auto job = m_scheduledJobs.take(jobDataIdentifier.jobIdentifier);
-    if (!job) {
-        LOG_ERROR("Job %s resolved in server, but was not found", jobDataIdentifier.loggingString().utf8().data());
-        return;
-    }
-
-    ScriptExecutionContext::postTaskTo(job->contextIdentifier(), [job, unregistrationResult](ScriptExecutionContext&) {
-        job->resolvedWithUnregistrationResult(unregistrationResult);
+    postTaskForJob(jobIdentifier, IsJobComplete::Yes, [unregistrationResult] (auto& job) {
+        job.resolvedWithUnregistrationResult(unregistrationResult);
     });
 }
 
-void SWClientConnection::startScriptFetchForServer(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, const ServiceWorkerRegistrationKey& registrationKey, FetchOptions::Cache cachePolicy)
+void SWClientConnection::startScriptFetchForServer(ServiceWorkerJobIdentifier jobIdentifier, const ServiceWorkerRegistrationKey& registrationKey, FetchOptions::Cache cachePolicy)
 {
-    ASSERT(isMainThread());
-
-    auto job = m_scheduledJobs.get(jobDataIdentifier.jobIdentifier);
-    if (!job) {
-        LOG_ERROR("Job %s instructed to start fetch from server, but job was not found", jobDataIdentifier.loggingString().utf8().data());
-
-        failedFetchingScript(jobDataIdentifier, registrationKey, ResourceError { errorDomainWebKitInternal, 0, URL(), ASCIILiteral("Failed to fetch service worker script") });
-        return;
-    }
-
-    bool wasPosted = ScriptExecutionContext::postTaskTo(job->contextIdentifier(), [job, cachePolicy](ScriptExecutionContext&) {
-        job->startScriptFetch(cachePolicy);
+    bool isPosted = postTaskForJob(jobIdentifier, IsJobComplete::No, [cachePolicy] (auto& job) {
+        job.startScriptFetch(cachePolicy);
     });
-    if (!wasPosted)
-        failedFetchingScript(jobDataIdentifier, registrationKey, ResourceError { errorDomainWebKitInternal, 0, job->data().scriptURL, ASCIILiteral("Failed to fetch service worker script") });
+    if (!isPosted)
+        failedFetchingScript(jobIdentifier, registrationKey, ResourceError { errorDomainWebKitInternal, 0, { }, makeString("Failed to fetch script for service worker with scope ", registrationKey.scope().string()) });
 }
 
+
 void SWClientConnection::postMessageToServiceWorkerClient(DocumentIdentifier destinationContextIdentifier, Ref<SerializedScriptValue>&& message, ServiceWorkerData&& sourceData, const String& sourceOrigin)
 {
     ASSERT(isMainThread());
@@ -278,10 +251,13 @@ void SWClientConnection::clearPendingJobs()
 {
     ASSERT(isMainThread());
 
-    auto jobs = WTFMove(m_scheduledJobs);
-    for (auto& job : jobs.values()) {
-        ScriptExecutionContext::postTaskTo(job->contextIdentifier(), [job](ScriptExecutionContext&) {
-            job->failedWithException(Exception { TypeError, ASCIILiteral("Internal error") });
+    auto jobSources = WTFMove(m_scheduledJobSources);
+    for (auto& keyValue : jobSources) {
+        ScriptExecutionContext::postTaskTo(keyValue.value, [identifier = keyValue.key] (auto& context) {
+            if (auto* container = context.serviceWorkerContainer()) {
+                if (auto* job = container->job(identifier))
+                    job->failedWithException(Exception { TypeError, ASCIILiteral("Internal error") });
+            }
         });
     }
 }
index 4ae78a3..1de2a53 100644 (file)
@@ -68,9 +68,8 @@ public:
     virtual void addServiceWorkerRegistrationInServer(ServiceWorkerRegistrationIdentifier) = 0;
     virtual void removeServiceWorkerRegistrationInServer(ServiceWorkerRegistrationIdentifier) = 0;
 
-    void scheduleJob(ServiceWorkerJob&);
-    void finishedFetchingScript(ServiceWorkerJob&, const String&);
-    void failedFetchingScript(const ServiceWorkerJobDataIdentifier&, const ServiceWorkerRegistrationKey&, const ResourceError&);
+    void scheduleJob(DocumentOrWorkerIdentifier, const ServiceWorkerJobData&);
+    void failedFetchingScript(ServiceWorkerJobIdentifier, const ServiceWorkerRegistrationKey&, const ResourceError&);
 
     virtual void didResolveRegistrationPromise(const ServiceWorkerRegistrationKey&) = 0;
 
@@ -83,13 +82,15 @@ public:
     virtual void registerServiceWorkerClient(const SecurityOrigin& topOrigin, const ServiceWorkerClientData&, const std::optional<ServiceWorkerIdentifier>&) = 0;
     virtual void unregisterServiceWorkerClient(DocumentIdentifier) = 0;
 
+    virtual void finishFetchingScriptInServer(const ServiceWorkerFetchResult&) = 0;
+
 protected:
     WEBCORE_EXPORT SWClientConnection();
 
-    WEBCORE_EXPORT void jobRejectedInServer(const ServiceWorkerJobDataIdentifier&, const ExceptionData&);
-    WEBCORE_EXPORT void registrationJobResolvedInServer(const ServiceWorkerJobDataIdentifier&, ServiceWorkerRegistrationData&&, ShouldNotifyWhenResolved);
-    WEBCORE_EXPORT void unregistrationJobResolvedInServer(const ServiceWorkerJobDataIdentifier&, bool unregistrationResult);
-    WEBCORE_EXPORT void startScriptFetchForServer(const ServiceWorkerJobDataIdentifier&, const ServiceWorkerRegistrationKey&, FetchOptions::Cache);
+    WEBCORE_EXPORT void jobRejectedInServer(ServiceWorkerJobIdentifier, const ExceptionData&);
+    WEBCORE_EXPORT void registrationJobResolvedInServer(ServiceWorkerJobIdentifier, ServiceWorkerRegistrationData&&, ShouldNotifyWhenResolved);
+    WEBCORE_EXPORT void unregistrationJobResolvedInServer(ServiceWorkerJobIdentifier, bool unregistrationResult);
+    WEBCORE_EXPORT void startScriptFetchForServer(ServiceWorkerJobIdentifier, const ServiceWorkerRegistrationKey&, FetchOptions::Cache);
     WEBCORE_EXPORT void postMessageToServiceWorkerClient(DocumentIdentifier destinationContextIdentifier, Ref<SerializedScriptValue>&& message, ServiceWorkerData&& source, const String& sourceOrigin);
     WEBCORE_EXPORT void updateRegistrationState(ServiceWorkerRegistrationIdentifier, ServiceWorkerRegistrationState, const std::optional<ServiceWorkerData>&);
     WEBCORE_EXPORT void updateWorkerState(ServiceWorkerIdentifier, ServiceWorkerState);
@@ -102,9 +103,11 @@ protected:
 
 private:
     virtual void scheduleJobInServer(const ServiceWorkerJobData&) = 0;
-    virtual void finishFetchingScriptInServer(const ServiceWorkerFetchResult&) = 0;
 
-    HashMap<ServiceWorkerJobIdentifier, RefPtr<ServiceWorkerJob>> m_scheduledJobs;
+    enum class IsJobComplete { No, Yes };
+    bool postTaskForJob(ServiceWorkerJobIdentifier, IsJobComplete, WTF::Function<void(ServiceWorkerJob&)>&&);
+
+    HashMap<ServiceWorkerJobIdentifier, DocumentOrWorkerIdentifier> m_scheduledJobSources;
 };
 
 } // namespace WebCore
index 1970f0a..27dd8f3 100644 (file)
@@ -42,6 +42,7 @@
 #include "ScriptExecutionContext.h"
 #include "SecurityOrigin.h"
 #include "ServiceWorker.h"
+#include "ServiceWorkerFetchResult.h"
 #include "ServiceWorkerGlobalScope.h"
 #include "ServiceWorkerJob.h"
 #include "ServiceWorkerJobData.h"
@@ -223,11 +224,12 @@ void ServiceWorkerContainer::scheduleJob(Ref<ServiceWorkerJob>&& job)
 
     setPendingActivity(this);
 
-    auto result = m_jobMap.add(job->identifier(), job.copyRef());
+    auto jobData = job->data();
+    auto result = m_jobMap.add(job->identifier(), WTFMove(job));
     ASSERT_UNUSED(result, result.isNewEntry);
 
-    callOnMainThread([connection = m_swConnection, job = WTFMove(job)] {
-        connection->scheduleJob(job);
+    callOnMainThread([connection = m_swConnection, contextIdentifier = this->contextIdentifier(), jobData = WTFMove(jobData)] {
+        connection->scheduleJob(contextIdentifier, jobData);
     });
 }
 
@@ -458,8 +460,8 @@ void ServiceWorkerContainer::startScriptFetchForJob(ServiceWorkerJob& job, Fetch
     auto* context = scriptExecutionContext();
     if (!context) {
         LOG_ERROR("ServiceWorkerContainer::jobResolvedWithRegistration called but the container's ScriptExecutionContext is gone");
-        callOnMainThread([connection = m_swConnection, job = makeRef(job)] {
-            connection->failedFetchingScript(job->data().identifier(), job->data().registrationKey(), { errorDomainWebKitInternal, 0, job->data().scriptURL, ASCIILiteral("Attempt to fetch service worker script with no ScriptExecutionContext") });
+        callOnMainThread([connection = m_swConnection, jobIdentifier = job.identifier(), registrationKey = job.data().registrationKey(), scriptURL = job.data().scriptURL.isolatedCopy()] {
+            connection->failedFetchingScript(jobIdentifier, registrationKey, { errorDomainWebKitInternal, 0, scriptURL, ASCIILiteral("Attempt to fetch service worker script with no ScriptExecutionContext") });
         });
         jobDidFinish(job);
         return;
@@ -476,8 +478,8 @@ void ServiceWorkerContainer::jobFinishedLoadingScript(ServiceWorkerJob& job, con
 
     LOG(ServiceWorker, "SeviceWorkerContainer %p finished fetching script for job %s", this, job.identifier().loggingString().utf8().data());
 
-    callOnMainThread([connection = m_swConnection, job = makeRef(job), script = script.isolatedCopy()] {
-        connection->finishedFetchingScript(job, script);
+    callOnMainThread([connection = m_swConnection, jobDataIdentifier = job.data().identifier(), registrationKey = job.data().registrationKey(), script = script.isolatedCopy()] {
+        connection->finishFetchingScriptInServer({ jobDataIdentifier, registrationKey, script, { } });
     });
 }
 
@@ -493,8 +495,8 @@ void ServiceWorkerContainer::jobFailedLoadingScript(ServiceWorkerJob& job, const
     if (exception && job.promise())
         job.promise()->reject(*exception);
 
-    callOnMainThread([connection = m_swConnection, job = makeRef(job), error = error.isolatedCopy()] {
-        connection->failedFetchingScript(job->data().identifier(), job->data().registrationKey(), error);
+    callOnMainThread([connection = m_swConnection, jobIdentifier = job.identifier(), registrationKey = job.data().registrationKey(), error = error.isolatedCopy()] {
+        connection->failedFetchingScript(jobIdentifier, registrationKey, error);
     });
 }
 
index 19d25f5..2b5b0a9 100644 (file)
@@ -75,6 +75,8 @@ public:
     void addRegistration(ServiceWorkerRegistration&);
     void removeRegistration(ServiceWorkerRegistration&);
 
+    ServiceWorkerJob* job(ServiceWorkerJobIdentifier identifier) { return m_jobMap.get(identifier); }
+
     void startMessages();
 
     void ref() final { refEventTarget(); }
index 68addee..f008e31 100644 (file)
@@ -273,7 +273,7 @@ void SWServer::rejectJob(const ServiceWorkerJobData& jobData, const ExceptionDat
     if (!connection)
         return;
 
-    connection->rejectJobInClient(jobData.identifier(), exceptionData);
+    connection->rejectJobInClient(jobData.identifier().jobIdentifier, exceptionData);
 }
 
 void SWServer::resolveRegistrationJob(const ServiceWorkerJobData& jobData, const ServiceWorkerRegistrationData& registrationData, ShouldNotifyWhenResolved shouldNotifyWhenResolved)
@@ -283,7 +283,7 @@ void SWServer::resolveRegistrationJob(const ServiceWorkerJobData& jobData, const
     if (!connection)
         return;
 
-    connection->resolveRegistrationJobInClient(jobData.identifier(), registrationData, shouldNotifyWhenResolved);
+    connection->resolveRegistrationJobInClient(jobData.identifier().jobIdentifier, registrationData, shouldNotifyWhenResolved);
 }
 
 void SWServer::resolveUnregistrationJob(const ServiceWorkerJobData& jobData, const ServiceWorkerRegistrationKey& registrationKey, bool unregistrationResult)
@@ -292,7 +292,7 @@ void SWServer::resolveUnregistrationJob(const ServiceWorkerJobData& jobData, con
     if (!connection)
         return;
 
-    connection->resolveUnregistrationJobInClient(jobData.identifier(), registrationKey, unregistrationResult);
+    connection->resolveUnregistrationJobInClient(jobData.identifier().jobIdentifier, registrationKey, unregistrationResult);
 }
 
 void SWServer::startScriptFetch(const ServiceWorkerJobData& jobData, FetchOptions::Cache cachePolicy)
@@ -302,7 +302,7 @@ void SWServer::startScriptFetch(const ServiceWorkerJobData& jobData, FetchOption
     if (!connection)
         return;
 
-    connection->startScriptFetchInClient(jobData.identifier(), jobData.registrationKey(), cachePolicy);
+    connection->startScriptFetchInClient(jobData.identifier().jobIdentifier, jobData.registrationKey(), cachePolicy);
 }
 
 void SWServer::scriptFetchFinished(Connection& connection, const ServiceWorkerFetchResult& result)
index 66d6e5e..2d366a7 100644 (file)
@@ -103,10 +103,10 @@ public:
 
     private:
         // Messages to the client WebProcess
-        virtual void rejectJobInClient(const ServiceWorkerJobDataIdentifier&, const ExceptionData&) = 0;
-        virtual void resolveRegistrationJobInClient(const ServiceWorkerJobDataIdentifier&, const ServiceWorkerRegistrationData&, ShouldNotifyWhenResolved) = 0;
-        virtual void resolveUnregistrationJobInClient(const ServiceWorkerJobDataIdentifier&, const ServiceWorkerRegistrationKey&, bool registrationResult) = 0;
-        virtual void startScriptFetchInClient(const ServiceWorkerJobDataIdentifier&, const ServiceWorkerRegistrationKey&, FetchOptions::Cache) = 0;
+        virtual void rejectJobInClient(ServiceWorkerJobIdentifier, const ExceptionData&) = 0;
+        virtual void resolveRegistrationJobInClient(ServiceWorkerJobIdentifier, const ServiceWorkerRegistrationData&, ShouldNotifyWhenResolved) = 0;
+        virtual void resolveUnregistrationJobInClient(ServiceWorkerJobIdentifier, const ServiceWorkerRegistrationKey&, bool registrationResult) = 0;
+        virtual void startScriptFetchInClient(ServiceWorkerJobIdentifier, const ServiceWorkerRegistrationKey&, FetchOptions::Cache) = 0;
 
         struct RegistrationReadyRequest {
             SecurityOriginData topOrigin;
index 95d8b8a..f8b0127 100644 (file)
@@ -1,3 +1,23 @@
+2018-01-09  Youenn Fablet  <youenn@apple.com>
+
+        SWClientConnection should not keep references to service worker jobs
+        https://bugs.webkit.org/show_bug.cgi?id=181381
+
+        Reviewed by Chris Dumez.
+
+        Updated IPC handling based on WebCore refactoring.
+
+        * Scripts/webkit/messages.py:
+        (forward_declarations_and_headers):
+        (headers_for_type):
+        * StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
+        (WebKit::WebSWServerConnection::rejectJobInClient):
+        (WebKit::WebSWServerConnection::resolveRegistrationJobInClient):
+        (WebKit::WebSWServerConnection::resolveUnregistrationJobInClient):
+        (WebKit::WebSWServerConnection::startScriptFetchInClient):
+        * StorageProcess/ServiceWorker/WebSWServerConnection.h:
+        * WebProcess/Storage/WebSWClientConnection.messages.in:
+
 2018-01-09  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Update OptionsGTK.cmake and NEWS for 2.19.4 release.
index 65c89ad..a8bcc97 100644 (file)
@@ -186,6 +186,7 @@ def forward_declarations_and_headers(receiver):
     no_forward_declaration_types = frozenset([
         'WebCore::DocumentIdentifier',
         'WebCore::ServiceWorkerIdentifier',
+        'WebCore::ServiceWorkerJobIdentifier',
         'WebCore::ServiceWorkerOrClientData',
         'WebCore::ServiceWorkerOrClientIdentifier',
         'WebCore::ServiceWorkerRegistrationIdentifier',
@@ -387,6 +388,7 @@ def headers_for_type(type):
         'WebCore::PolicyAction': ['<WebCore/FrameLoaderTypes.h>'],
         'WebCore::RecentSearch': ['<WebCore/SearchPopupMenu.h>'],
         'WebCore::SWServerConnectionIdentifier': ['<WebCore/ServiceWorkerTypes.h>'],
+        'WebCore::ServiceWorkerJobIdentifier': ['<WebCore/ServiceWorkerTypes.h>'],
         'WebCore::ServiceWorkerOrClientData': ['<WebCore/ServiceWorkerTypes.h>', '<WebCore/ServiceWorkerClientData.h>', '<WebCore/ServiceWorkerData.h>'],
         'WebCore::ServiceWorkerOrClientIdentifier': ['<WebCore/ServiceWorkerTypes.h>', '<WebCore/ServiceWorkerClientIdentifier.h>'],
         'WebCore::ServiceWorkerRegistrationIdentifier': ['<WebCore/ServiceWorkerTypes.h>'],
index d4d9137..47218d2 100644 (file)
@@ -76,24 +76,24 @@ void WebSWServerConnection::disconnectedFromWebProcess()
     notImplemented();
 }
 
-void WebSWServerConnection::rejectJobInClient(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, const ExceptionData& exceptionData)
+void WebSWServerConnection::rejectJobInClient(ServiceWorkerJobIdentifier jobIdentifier, const ExceptionData& exceptionData)
 {
-    send(Messages::WebSWClientConnection::JobRejectedInServer(jobDataIdentifier, exceptionData));
+    send(Messages::WebSWClientConnection::JobRejectedInServer(jobIdentifier, exceptionData));
 }
 
-void WebSWServerConnection::resolveRegistrationJobInClient(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, const ServiceWorkerRegistrationData& registrationData, ShouldNotifyWhenResolved shouldNotifyWhenResolved)
+void WebSWServerConnection::resolveRegistrationJobInClient(ServiceWorkerJobIdentifier jobIdentifier, const ServiceWorkerRegistrationData& registrationData, ShouldNotifyWhenResolved shouldNotifyWhenResolved)
 {
-    send(Messages::WebSWClientConnection::RegistrationJobResolvedInServer(jobDataIdentifier, registrationData, shouldNotifyWhenResolved));
+    send(Messages::WebSWClientConnection::RegistrationJobResolvedInServer(jobIdentifier, registrationData, shouldNotifyWhenResolved));
 }
 
-void WebSWServerConnection::resolveUnregistrationJobInClient(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, const ServiceWorkerRegistrationKey& registrationKey, bool unregistrationResult)
+void WebSWServerConnection::resolveUnregistrationJobInClient(ServiceWorkerJobIdentifier jobIdentifier, const ServiceWorkerRegistrationKey& registrationKey, bool unregistrationResult)
 {
-    send(Messages::WebSWClientConnection::UnregistrationJobResolvedInServer(jobDataIdentifier, unregistrationResult));
+    send(Messages::WebSWClientConnection::UnregistrationJobResolvedInServer(jobIdentifier, unregistrationResult));
 }
 
-void WebSWServerConnection::startScriptFetchInClient(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, const ServiceWorkerRegistrationKey& registrationKey, FetchOptions::Cache cachePolicy)
+void WebSWServerConnection::startScriptFetchInClient(ServiceWorkerJobIdentifier jobIdentifier, const ServiceWorkerRegistrationKey& registrationKey, FetchOptions::Cache cachePolicy)
 {
-    send(Messages::WebSWClientConnection::StartScriptFetchForServer(jobDataIdentifier, registrationKey, cachePolicy));
+    send(Messages::WebSWClientConnection::StartScriptFetchForServer(jobIdentifier, registrationKey, cachePolicy));
 }
 
 void WebSWServerConnection::updateRegistrationStateInClient(ServiceWorkerRegistrationIdentifier identifier, ServiceWorkerRegistrationState state, const std::optional<ServiceWorkerData>& serviceWorkerData)
index 3b93e38..e244cdc 100644 (file)
@@ -71,10 +71,10 @@ public:
 
 private:
     // Implement SWServer::Connection (Messages to the client WebProcess)
-    void rejectJobInClient(const WebCore::ServiceWorkerJobDataIdentifier&, const WebCore::ExceptionData&) final;
-    void resolveRegistrationJobInClient(const WebCore::ServiceWorkerJobDataIdentifier&, const WebCore::ServiceWorkerRegistrationData&, WebCore::ShouldNotifyWhenResolved) final;
-    void resolveUnregistrationJobInClient(const WebCore::ServiceWorkerJobDataIdentifier&, const WebCore::ServiceWorkerRegistrationKey&, bool unregistrationResult) final;
-    void startScriptFetchInClient(const WebCore::ServiceWorkerJobDataIdentifier&, const WebCore::ServiceWorkerRegistrationKey&, WebCore::FetchOptions::Cache) final;
+    void rejectJobInClient(WebCore::ServiceWorkerJobIdentifier, const WebCore::ExceptionData&) final;
+    void resolveRegistrationJobInClient(WebCore::ServiceWorkerJobIdentifier, const WebCore::ServiceWorkerRegistrationData&, WebCore::ShouldNotifyWhenResolved) final;
+    void resolveUnregistrationJobInClient(WebCore::ServiceWorkerJobIdentifier, const WebCore::ServiceWorkerRegistrationKey&, bool unregistrationResult) final;
+    void startScriptFetchInClient(WebCore::ServiceWorkerJobIdentifier, const WebCore::ServiceWorkerRegistrationKey&, WebCore::FetchOptions::Cache) final;
     void updateRegistrationStateInClient(WebCore::ServiceWorkerRegistrationIdentifier, WebCore::ServiceWorkerRegistrationState, const std::optional<WebCore::ServiceWorkerData>&) final;
     void updateWorkerStateInClient(WebCore::ServiceWorkerIdentifier, WebCore::ServiceWorkerState) final;
     void fireUpdateFoundEvent(WebCore::ServiceWorkerRegistrationIdentifier) final;
index dba647e..56d2e78 100644 (file)
 
 messages -> WebSWClientConnection {
     # When possible, these messages can be implemented directly by WebCore::SWServer::Connection
-    JobRejectedInServer(struct WebCore::ServiceWorkerJobDataIdentifier jobDataIdentifier, struct WebCore::ExceptionData exception)
-    RegistrationJobResolvedInServer(struct WebCore::ServiceWorkerJobDataIdentifier jobDataIdentifier, struct WebCore::ServiceWorkerRegistrationData registration, enum WebCore::ShouldNotifyWhenResolved shouldNotifyWhenResolved)
-    UnregistrationJobResolvedInServer(struct WebCore::ServiceWorkerJobDataIdentifier jobDataIdentifier, bool unregistrationResult)
-    StartScriptFetchForServer(struct WebCore::ServiceWorkerJobDataIdentifier jobDataIdentifier, WebCore::ServiceWorkerRegistrationKey registrationKey, WebCore::FetchOptions::Cache cachePolicy)
+    JobRejectedInServer(WebCore::ServiceWorkerJobIdentifier jobDataIdentifier, struct WebCore::ExceptionData exception)
+    RegistrationJobResolvedInServer(WebCore::ServiceWorkerJobIdentifier jobDataIdentifier, struct WebCore::ServiceWorkerRegistrationData registration, enum WebCore::ShouldNotifyWhenResolved shouldNotifyWhenResolved)
+    UnregistrationJobResolvedInServer(WebCore::ServiceWorkerJobIdentifier jobDataIdentifier, bool unregistrationResult)
+    StartScriptFetchForServer(WebCore::ServiceWorkerJobIdentifier jobDataIdentifier, WebCore::ServiceWorkerRegistrationKey registrationKey, WebCore::FetchOptions::Cache cachePolicy)
     UpdateRegistrationState(WebCore::ServiceWorkerRegistrationIdentifier identifier, enum WebCore::ServiceWorkerRegistrationState state, std::optional<WebCore::ServiceWorkerData> serviceWorkerIdentifier)
     UpdateWorkerState(WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, enum WebCore::ServiceWorkerState state)
     FireUpdateFoundEvent(WebCore::ServiceWorkerRegistrationIdentifier identifier)