- Revision
- 225351
- Author
- [email protected]
- Date
- 2017-11-30 14:35:25 -0800 (Thu, 30 Nov 2017)
Log Message
ServiceWorkerThread's m_extendedEvents gets destroyed on the wrong thread
https://bugs.webkit.org/show_bug.cgi?id=180216
Reviewed by Brady Eidson.
ServiceWorkerThread's m_extendedEvents gets destroyed on the wrong thread. Those events live on the worker
thread so we should destroy them on the worker thread, not the main thread. To address the issue, m_extendedEvents
was moved to ServiceWorkerGlobalScope, which actually lives on the right thread.
* workers/service/ServiceWorkerGlobalScope.cpp:
(WebCore::ServiceWorkerGlobalScope::updateExtendedEventsSet):
* workers/service/ServiceWorkerGlobalScope.h:
* workers/service/context/ServiceWorkerThread.cpp:
(WebCore::ServiceWorkerThread::postFetchTask):
(WebCore::ServiceWorkerThread::postMessageToServiceWorkerGlobalScope):
(WebCore::ServiceWorkerThread::updateExtendedEventsSet): Deleted.
* workers/service/context/ServiceWorkerThread.h:
(WebCore::ServiceWorkerThread::hasPendingEvents const): Deleted.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (225350 => 225351)
--- trunk/Source/WebCore/ChangeLog 2017-11-30 22:25:52 UTC (rev 225350)
+++ trunk/Source/WebCore/ChangeLog 2017-11-30 22:35:25 UTC (rev 225351)
@@ -1,5 +1,26 @@
2017-11-30 Chris Dumez <[email protected]>
+ ServiceWorkerThread's m_extendedEvents gets destroyed on the wrong thread
+ https://bugs.webkit.org/show_bug.cgi?id=180216
+
+ Reviewed by Brady Eidson.
+
+ ServiceWorkerThread's m_extendedEvents gets destroyed on the wrong thread. Those events live on the worker
+ thread so we should destroy them on the worker thread, not the main thread. To address the issue, m_extendedEvents
+ was moved to ServiceWorkerGlobalScope, which actually lives on the right thread.
+
+ * workers/service/ServiceWorkerGlobalScope.cpp:
+ (WebCore::ServiceWorkerGlobalScope::updateExtendedEventsSet):
+ * workers/service/ServiceWorkerGlobalScope.h:
+ * workers/service/context/ServiceWorkerThread.cpp:
+ (WebCore::ServiceWorkerThread::postFetchTask):
+ (WebCore::ServiceWorkerThread::postMessageToServiceWorkerGlobalScope):
+ (WebCore::ServiceWorkerThread::updateExtendedEventsSet): Deleted.
+ * workers/service/context/ServiceWorkerThread.h:
+ (WebCore::ServiceWorkerThread::hasPendingEvents const): Deleted.
+
+2017-11-30 Chris Dumez <[email protected]>
+
SWServerToContextConnection / SWServerWorker do not need to be ThreadSafeRefCounted
https://bugs.webkit.org/show_bug.cgi?id=180214
Modified: trunk/Source/WebCore/bindings/js/JSDOMGuardedObject.cpp (225350 => 225351)
--- trunk/Source/WebCore/bindings/js/JSDOMGuardedObject.cpp 2017-11-30 22:25:52 UTC (rev 225350)
+++ trunk/Source/WebCore/bindings/js/JSDOMGuardedObject.cpp 2017-11-30 22:35:25 UTC (rev 225351)
@@ -53,6 +53,7 @@
m_globalObject->guardedObjects(locker).remove(this);
}
m_guarded.clear();
+ m_globalObject.clear();
}
void DOMGuardedObject::contextDestroyed()
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp (225350 => 225351)
--- trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp 2017-11-30 22:25:52 UTC (rev 225350)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp 2017-11-30 22:35:25 UTC (rev 225351)
@@ -28,6 +28,8 @@
#if ENABLE(SERVICE_WORKER)
+#include "ExtendableEvent.h"
+#include "SWContextManager.h"
#include "ServiceWorkerClient.h"
#include "ServiceWorkerClients.h"
#include "ServiceWorkerThread.h"
@@ -79,6 +81,36 @@
ASSERT_UNUSED(isRemoved, isRemoved);
}
+// https://w3c.github.io/ServiceWorker/#update-service-worker-extended-events-set-algorithm
+void ServiceWorkerGlobalScope::updateExtendedEventsSet(ExtendableEvent* newEvent)
+{
+ ASSERT(!isMainThread());
+ ASSERT(!newEvent || !newEvent->isBeingDispatched());
+ bool hadPendingEvents = hasPendingEvents();
+ m_extendedEvents.removeAllMatching([](auto& event) {
+ return !event->pendingPromiseCount();
+ });
+
+ if (newEvent && newEvent->pendingPromiseCount()) {
+ m_extendedEvents.append(*newEvent);
+ newEvent->whenAllExtendLifetimePromisesAreSettled([this](auto&&) {
+ updateExtendedEventsSet();
+ });
+ // Clear out the event's target as it is the WorkerGlobalScope and we do not want to keep it
+ // alive unnecessarily.
+ newEvent->setTarget(nullptr);
+ }
+
+ bool hasPendingEvents = this->hasPendingEvents();
+ if (hasPendingEvents == hadPendingEvents)
+ return;
+
+ callOnMainThread([threadIdentifier = thread().identifier(), hasPendingEvents] {
+ if (auto* connection = SWContextManager::singleton().connection())
+ connection->setServiceWorkerHasPendingEvents(threadIdentifier, hasPendingEvents);
+ });
+}
+
} // namespace WebCore
#endif // ENABLE(SERVICE_WORKER)
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h (225350 => 225351)
--- trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h 2017-11-30 22:25:52 UTC (rev 225350)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h 2017-11-30 22:35:25 UTC (rev 225351)
@@ -35,6 +35,7 @@
namespace WebCore {
class DeferredPromise;
+class ExtendableEvent;
struct ServiceWorkerClientData;
class ServiceWorkerClient;
class ServiceWorkerClients;
@@ -64,13 +65,18 @@
void addServiceWorkerClient(ServiceWorkerClient&);
void removeServiceWorkerClient(ServiceWorkerClient&);
+ void updateExtendedEventsSet(ExtendableEvent* newEvent = nullptr);
+
private:
ServiceWorkerGlobalScope(const ServiceWorkerContextData&, const URL&, const String& identifier, const String& userAgent, bool isOnline, ServiceWorkerThread&, bool shouldBypassMainWorldContentSecurityPolicy, Ref<SecurityOrigin>&& topOrigin, MonotonicTime timeOrigin, IDBClient::IDBConnectionProxy*, SocketProvider*, PAL::SessionID);
+ bool hasPendingEvents() const { return !m_extendedEvents.isEmpty(); }
+
ServiceWorkerContextData m_contextData;
Ref<ServiceWorkerRegistration> m_registration;
Ref<ServiceWorkerClients> m_clients;
HashMap<ServiceWorkerClientIdentifier, ServiceWorkerClient*> m_clientMap;
+ Vector<Ref<ExtendableEvent>> m_extendedEvents;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp (225350 => 225351)
--- trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp 2017-11-30 22:25:52 UTC (rev 225350)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp 2017-11-30 22:35:25 UTC (rev 225351)
@@ -99,8 +99,9 @@
// FIXME: instead of directly using runLoop(), we should be using something like WorkerGlobalScopeProxy.
// FIXME: request and options come straigth from IPC so are already isolated. We should be able to take benefit of that.
runLoop().postTaskForMode([this, client = WTFMove(client), clientId, request = request.isolatedCopy(), options = options.isolatedCopy()] (ScriptExecutionContext& context) mutable {
- auto fetchEvent = ServiceWorkerFetch::dispatchFetchEvent(WTFMove(client), downcast<WorkerGlobalScope>(context), clientId, WTFMove(request), WTFMove(options));
- updateExtendedEventsSet(fetchEvent.ptr());
+ auto& serviceWorkerGlobalScope = downcast<ServiceWorkerGlobalScope>(context);
+ auto fetchEvent = ServiceWorkerFetch::dispatchFetchEvent(WTFMove(client), serviceWorkerGlobalScope, clientId, WTFMove(request), WTFMove(options));
+ serviceWorkerGlobalScope.updateExtendedEventsSet(fetchEvent.ptr());
}, WorkerRunLoop::defaultMode());
}
@@ -114,7 +115,7 @@
auto messageEvent = ExtendableMessageEvent::create(WTFMove(ports), WTFMove(message), sourceOrigin->toString(), { }, ExtendableMessageEventSource { source });
serviceWorkerGlobalScope.dispatchEvent(messageEvent);
serviceWorkerGlobalScope.thread().workerObjectProxy().confirmMessageFromWorkerObject(serviceWorkerGlobalScope.hasPendingActivity());
- updateExtendedEventsSet(messageEvent.ptr());
+ serviceWorkerGlobalScope.updateExtendedEventsSet(messageEvent.ptr());
});
runLoop().postTask(WTFMove(task));
}
@@ -164,36 +165,6 @@
runLoop().postTask(WTFMove(task));
}
-// https://w3c.github.io/ServiceWorker/#update-service-worker-extended-events-set-algorithm
-void ServiceWorkerThread::updateExtendedEventsSet(ExtendableEvent* newEvent)
-{
- ASSERT(!isMainThread());
- ASSERT(!newEvent || !newEvent->isBeingDispatched());
- bool hadPendingEvents = hasPendingEvents();
- m_extendedEvents.removeAllMatching([](auto& event) {
- return !event->pendingPromiseCount();
- });
-
- if (newEvent && newEvent->pendingPromiseCount()) {
- m_extendedEvents.append(*newEvent);
- newEvent->whenAllExtendLifetimePromisesAreSettled([this](auto&&) {
- updateExtendedEventsSet();
- });
- // Clear out the event's target as it is the WorkerGlobalScope and we do not want to keep it
- // alive unnecessarily.
- newEvent->setTarget(nullptr);
- }
-
- bool hasPendingEvents = this->hasPendingEvents();
- if (hasPendingEvents == hadPendingEvents)
- return;
-
- callOnMainThread([identifier = this->identifier(), hasPendingEvents] {
- if (auto* connection = SWContextManager::singleton().connection())
- connection->setServiceWorkerHasPendingEvents(identifier, hasPendingEvents);
- });
-}
-
} // namespace WebCore
#endif // ENABLE(SERVICE_WORKER)
Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.h (225350 => 225351)
--- trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.h 2017-11-30 22:25:52 UTC (rev 225350)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.h 2017-11-30 22:35:25 UTC (rev 225351)
@@ -74,12 +74,8 @@
private:
WEBCORE_EXPORT ServiceWorkerThread(const ServiceWorkerContextData&, PAL::SessionID, WorkerLoaderProxy&, WorkerDebuggerProxy&);
- void updateExtendedEventsSet(ExtendableEvent* newEvent = nullptr);
- bool hasPendingEvents() const { return !m_extendedEvents.isEmpty(); }
-
ServiceWorkerContextData m_data;
WorkerObjectProxy& m_workerObjectProxy;
- Vector<Ref<ExtendableEvent>> m_extendedEvents;
};
} // namespace WebCore
