Modified: trunk/Source/WebCore/ChangeLog (242371 => 242372)
--- trunk/Source/WebCore/ChangeLog 2019-03-04 20:26:06 UTC (rev 242371)
+++ trunk/Source/WebCore/ChangeLog 2019-03-04 20:30:58 UTC (rev 242372)
@@ -1,3 +1,28 @@
+2019-03-04 Youenn Fablet <you...@apple.com>
+
+ Make sure to correctly notify of end of a ServiceWorkerJob when the context is stopped
+ https://bugs.webkit.org/show_bug.cgi?id=195195
+
+ Reviewed by Chris Dumez.
+
+ Before the patch, we were notifying that some jobs were finished too aggressively at context stop time.
+ This was confusing the Network Process.
+ Only notify such jobs that have pending loads.
+ Improve the tracking of jobs doing registration resolution to ensure the Network Process gets notified
+ in case of a registration promise being resolved but the settling callback being not yet called while the context is stopped.
+
+ Covered by existing tests not crashing anymore, in particular imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting.https.html.
+
+ * workers/service/ServiceWorkerContainer.cpp:
+ (WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):
+ (WebCore::ServiceWorkerContainer::notifyRegistrationIsSettled):
+ (WebCore::ServiceWorkerContainer::stop):
+ * workers/service/ServiceWorkerContainer.h:
+ * workers/service/ServiceWorkerJob.cpp:
+ (WebCore::ServiceWorkerJob::cancelPendingLoad):
+ * workers/service/ServiceWorkerJob.h:
+ (WebCore::ServiceWorkerJob::isLoading const):
+
2019-03-04 Chris Dumez <cdu...@apple.com>
[iOS] Improve our file picker
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp (242371 => 242372)
--- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp 2019-03-04 20:26:06 UTC (rev 242371)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp 2019-03-04 20:30:58 UTC (rev 242372)
@@ -421,10 +421,6 @@
#endif
ASSERT_WITH_MESSAGE(job.hasPromise() || job.data().type == ServiceWorkerJobType::Update, "Only soft updates have no promise");
- auto guard = WTF::makeScopeExit([this, &job] {
- destroyJob(job);
- });
-
if (job.data().type == ServiceWorkerJobType::Register)
CONTAINER_RELEASE_LOG_IF_ALLOWED("jobResolvedWithRegistration: Registration job %" PRIu64 " succeeded", job.identifier().toUInt64());
else {
@@ -432,32 +428,28 @@
CONTAINER_RELEASE_LOG_IF_ALLOWED("jobResolvedWithRegistration: Update job %" PRIu64 " succeeded", job.identifier().toUInt64());
}
- std::function<void()> notifyWhenResolvedIfNeeded;
- if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes) {
- notifyWhenResolvedIfNeeded = [connection = m_swConnection, registrationKey = data.key]() mutable {
- callOnMainThread([connection = WTFMove(connection), registrationKey = registrationKey.isolatedCopy()] {
- connection->didResolveRegistrationPromise(registrationKey);
- });
- };
- }
+ auto guard = WTF::makeScopeExit([this, &job] {
+ destroyJob(job);
+ });
- if (isStopped()) {
- if (notifyWhenResolvedIfNeeded)
- notifyWhenResolvedIfNeeded();
+ auto notifyIfExitEarly = WTF::makeScopeExit([this, &data, &shouldNotifyWhenResolved] {
+ if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
+ notifyRegistrationIsSettled(data.key);
+ });
+
+ if (isStopped())
return;
- }
auto promise = job.takePromise();
- if (!promise) {
- if (notifyWhenResolvedIfNeeded)
- notifyWhenResolvedIfNeeded();
+ if (!promise)
return;
- }
- scriptExecutionContext()->postTask([this, protectedThis = makeRef(*this), promise = WTFMove(promise), jobIdentifier = job.identifier(), data = "" notifyWhenResolvedIfNeeded = WTFMove(notifyWhenResolvedIfNeeded)](ScriptExecutionContext& context) mutable {
+ notifyIfExitEarly.release();
+
+ scriptExecutionContext()->postTask([this, protectedThis = RefPtr<ServiceWorkerContainer>(this), promise = WTFMove(promise), jobIdentifier = job.identifier(), data = "" shouldNotifyWhenResolved](ScriptExecutionContext& context) mutable {
if (isStopped() || !context.sessionID().isValid()) {
- if (notifyWhenResolvedIfNeeded)
- notifyWhenResolvedIfNeeded();
+ if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
+ notifyRegistrationIsSettled(data.key);
return;
}
@@ -465,9 +457,10 @@
CONTAINER_RELEASE_LOG_IF_ALLOWED("jobResolvedWithRegistration: Resolving promise for job %" PRIu64 ". Registration ID: %" PRIu64, jobIdentifier.toUInt64(), registration->identifier().toUInt64());
- if (notifyWhenResolvedIfNeeded) {
- promise->whenSettled([notifyWhenResolvedIfNeeded = WTFMove(notifyWhenResolvedIfNeeded)] {
- notifyWhenResolvedIfNeeded();
+ if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes) {
+ m_ongoingSettledRegistrations.add(++m_lastOngoingSettledRegistrationIdentifier, registration->data().key);
+ promise->whenSettled([this, protectedThis = WTFMove(protectedThis), identifier = m_lastOngoingSettledRegistrationIdentifier] {
+ notifyRegistrationIsSettled(m_ongoingSettledRegistrations.take(identifier));
});
}
@@ -475,6 +468,13 @@
});
}
+void ServiceWorkerContainer::notifyRegistrationIsSettled(const ServiceWorkerRegistrationKey& registrationKey)
+{
+ callOnMainThread([connection = m_swConnection, registrationKey = registrationKey.isolatedCopy()] {
+ connection->didResolveRegistrationPromise(registrationKey);
+ });
+}
+
void ServiceWorkerContainer::jobResolvedWithUnregistrationResult(ServiceWorkerJob& job, bool unregistrationResult)
{
#ifndef NDEBUG
@@ -639,9 +639,13 @@
m_readyPromise = nullptr;
auto jobMap = WTFMove(m_jobMap);
for (auto& ongoingJob : jobMap.values()) {
- notifyFailedFetchingScript(*ongoingJob.job.get(), ResourceError { errorDomainWebKitInternal, 0, ongoingJob.job->data().scriptURL, "Job cancelled"_s, ResourceError::Type::Cancellation });
- ongoingJob.job->cancelPendingLoad();
+ if (ongoingJob.job->cancelPendingLoad())
+ notifyFailedFetchingScript(*ongoingJob.job.get(), ResourceError { errorDomainWebKitInternal, 0, ongoingJob.job->data().scriptURL, "Job cancelled"_s, ResourceError::Type::Cancellation });
}
+
+ auto registrationMap = WTFMove(m_ongoingSettledRegistrations);
+ for (auto& registration : registrationMap.values())
+ notifyRegistrationIsSettled(registration);
}
DocumentOrWorkerIdentifier ServiceWorkerContainer::contextIdentifier()
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h (242371 => 242372)
--- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h 2019-03-04 20:26:06 UTC (rev 242371)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h 2019-03-04 20:30:58 UTC (rev 242372)
@@ -114,6 +114,8 @@
void derefEventTarget() final;
void stop() final;
+ void notifyRegistrationIsSettled(const ServiceWorkerRegistrationKey&);
+
std::unique_ptr<ReadyPromise> m_readyPromise;
NavigatorBase& m_navigator;
@@ -145,6 +147,10 @@
uint64_t m_lastPendingPromiseIdentifier { 0 };
HashMap<uint64_t, std::unique_ptr<PendingPromise>> m_pendingPromises;
+
+ uint64_t m_lastOngoingSettledRegistrationIdentifier { 0 };
+ HashMap<uint64_t, ServiceWorkerRegistrationKey> m_ongoingSettledRegistrations;
+
};
} // namespace WebCore
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerJob.cpp (242371 => 242372)
--- trunk/Source/WebCore/workers/service/ServiceWorkerJob.cpp 2019-03-04 20:26:06 UTC (rev 242371)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerJob.cpp 2019-03-04 20:30:58 UTC (rev 242372)
@@ -166,12 +166,14 @@
m_client.jobFailedLoadingScript(*this, error, Exception { error.isAccessControl() ? SecurityError : TypeError, "Script load failed"_s });
}
-void ServiceWorkerJob::cancelPendingLoad()
+bool ServiceWorkerJob::cancelPendingLoad()
{
if (!m_scriptLoader)
- return;
+ return false;
+
m_scriptLoader->cancel();
m_scriptLoader = nullptr;
+ return true;
}
} // namespace WebCore
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerJob.h (242371 => 242372)
--- trunk/Source/WebCore/workers/service/ServiceWorkerJob.h 2019-03-04 20:26:06 UTC (rev 242371)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerJob.h 2019-03-04 20:30:58 UTC (rev 242372)
@@ -69,7 +69,7 @@
const DocumentOrWorkerIdentifier& contextIdentifier() { return m_contextIdentifier; }
- void cancelPendingLoad();
+ bool cancelPendingLoad();
private:
// WorkerScriptLoaderClient
