- Revision
- 225248
- Author
- [email protected]
- Date
- 2017-11-28 16:28:42 -0800 (Tue, 28 Nov 2017)
Log Message
Get rid of ServiceWorker::allWorkers() hashmap
https://bugs.webkit.org/show_bug.cgi?id=180111
Reviewed by Brady Eidson.
Get rid of ServiceWorker::allWorkers() hashmap as it is not thread safe and we'll soon have
ServiceWorker objects living in various service worker threads.
Instead, we now have a per-ScriptExecutionContext map, which is inherently thread-safe.
No new tests, no web-facing behavior change.
* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::registerServiceWorker):
(WebCore::ScriptExecutionContext::unregisterServiceWorker):
* dom/ScriptExecutionContext.h:
(WebCore::ScriptExecutionContext::serviceWorker):
* workers/service/ServiceWorker.cpp:
(WebCore::ServiceWorker::getOrCreate):
(WebCore::ServiceWorker::ServiceWorker):
(WebCore::ServiceWorker::~ServiceWorker):
(WebCore::ServiceWorker::stop):
* workers/service/ServiceWorker.h:
* workers/service/server/SWClientConnection.cpp:
(WebCore::SWClientConnection::updateWorkerState):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (225247 => 225248)
--- trunk/Source/WebCore/ChangeLog 2017-11-29 00:23:29 UTC (rev 225247)
+++ trunk/Source/WebCore/ChangeLog 2017-11-29 00:28:42 UTC (rev 225248)
@@ -1,3 +1,31 @@
+2017-11-28 Chris Dumez <[email protected]>
+
+ Get rid of ServiceWorker::allWorkers() hashmap
+ https://bugs.webkit.org/show_bug.cgi?id=180111
+
+ Reviewed by Brady Eidson.
+
+ Get rid of ServiceWorker::allWorkers() hashmap as it is not thread safe and we'll soon have
+ ServiceWorker objects living in various service worker threads.
+
+ Instead, we now have a per-ScriptExecutionContext map, which is inherently thread-safe.
+
+ No new tests, no web-facing behavior change.
+
+ * dom/ScriptExecutionContext.cpp:
+ (WebCore::ScriptExecutionContext::registerServiceWorker):
+ (WebCore::ScriptExecutionContext::unregisterServiceWorker):
+ * dom/ScriptExecutionContext.h:
+ (WebCore::ScriptExecutionContext::serviceWorker):
+ * workers/service/ServiceWorker.cpp:
+ (WebCore::ServiceWorker::getOrCreate):
+ (WebCore::ServiceWorker::ServiceWorker):
+ (WebCore::ServiceWorker::~ServiceWorker):
+ (WebCore::ServiceWorker::stop):
+ * workers/service/ServiceWorker.h:
+ * workers/service/server/SWClientConnection.cpp:
+ (WebCore::SWClientConnection::updateWorkerState):
+
2017-11-28 Said Abou-Hallawa <[email protected]>
[CG] PostScript images should be supported if they are sub-resource images
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (225247 => 225248)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2017-11-29 00:23:29 UTC (rev 225247)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2017-11-29 00:28:42 UTC (rev 225248)
@@ -560,6 +560,17 @@
connection.serviceWorkerStartedControllingClient(m_activeServiceWorker->identifier(), m_activeServiceWorker->registrationIdentifier(), downcast<Document>(*this).identifier());
}
+void ScriptExecutionContext::registerServiceWorker(ServiceWorker& serviceWorker)
+{
+ auto addResult = m_serviceWorkers.add(serviceWorker.identifier(), &serviceWorker);
+ ASSERT_UNUSED(addResult, addResult.isNewEntry);
+}
+
+void ScriptExecutionContext::unregisterServiceWorker(ServiceWorker& serviceWorker)
+{
+ m_serviceWorkers.remove(serviceWorker.identifier());
+}
+
ServiceWorkerContainer* ScriptExecutionContext::serviceWorkerContainer()
{
NavigatorBase* navigator = nullptr;
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.h (225247 => 225248)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.h 2017-11-29 00:23:29 UTC (rev 225247)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.h 2017-11-29 00:28:42 UTC (rev 225248)
@@ -30,6 +30,7 @@
#include "ActiveDOMObject.h"
#include "DOMTimer.h"
#include "SecurityContext.h"
+#include "ServiceWorkerIdentifier.h"
#include <heap/HandleTypes.h>
#include <runtime/ConsoleTypes.h>
#include <wtf/CrossThreadTask.h>
@@ -240,6 +241,10 @@
ServiceWorker* activeServiceWorker() const;
void setActiveServiceWorker(RefPtr<ServiceWorker>&&);
+ void registerServiceWorker(ServiceWorker&);
+ void unregisterServiceWorker(ServiceWorker&);
+ ServiceWorker* serviceWorker(ServiceWorkerIdentifier identifier) { return m_serviceWorkers.get(identifier); }
+
ServiceWorkerContainer* serviceWorkerContainer();
#endif
@@ -313,6 +318,7 @@
#if ENABLE(SERVICE_WORKER)
RefPtr<ServiceWorker> m_activeServiceWorker;
+ HashMap<ServiceWorkerIdentifier, ServiceWorker*> m_serviceWorkers;
#endif
};
Modified: trunk/Source/WebCore/workers/service/ServiceWorker.cpp (225247 => 225248)
--- trunk/Source/WebCore/workers/service/ServiceWorker.cpp 2017-11-29 00:23:29 UTC (rev 225247)
+++ trunk/Source/WebCore/workers/service/ServiceWorker.cpp 2017-11-29 00:28:42 UTC (rev 225248)
@@ -41,31 +41,10 @@
namespace WebCore {
-const HashMap<ServiceWorkerIdentifier, HashSet<ServiceWorker*>>& ServiceWorker::allWorkers()
-{
- return mutableAllWorkers();
-}
-
-HashMap<ServiceWorkerIdentifier, HashSet<ServiceWorker*>>& ServiceWorker::mutableAllWorkers()
-{
- // FIXME: Once we support service workers from workers, this will need to change.
- RELEASE_ASSERT(isMainThread());
-
- static NeverDestroyed<HashMap<ServiceWorkerIdentifier, HashSet<ServiceWorker*>>> allWorkersMap;
- return allWorkersMap;
-}
-
Ref<ServiceWorker> ServiceWorker::getOrCreate(ScriptExecutionContext& context, ServiceWorkerData&& data)
{
- auto it = allWorkers().find(data.identifier);
- if (it != allWorkers().end()) {
- for (auto& worker : it->value) {
- if (worker->scriptExecutionContext() == &context) {
- ASSERT(!worker->m_isStopped);
- return *worker;
- }
- }
- }
+ if (auto existingServiceWorker = context.serviceWorker(data.identifier))
+ return *existingServiceWorker;
return adoptRef(*new ServiceWorker(context, WTFMove(data)));
}
@@ -75,10 +54,7 @@
{
suspendIfNeeded();
- auto result = mutableAllWorkers().ensure(identifier(), [] {
- return HashSet<ServiceWorker*>();
- });
- result.iterator->value.add(this);
+ context.registerServiceWorker(*this);
relaxAdoptionRequirement();
updatePendingActivityForEventDispatch();
@@ -86,13 +62,8 @@
ServiceWorker::~ServiceWorker()
{
- auto iterator = mutableAllWorkers().find(identifier());
-
- ASSERT(iterator->value.contains(this));
- iterator->value.remove(this);
-
- if (iterator->value.isEmpty())
- mutableAllWorkers().remove(iterator);
+ if (auto* context = scriptExecutionContext())
+ context->unregisterServiceWorker(*this);
}
void ServiceWorker::scheduleTaskToUpdateState(State state)
@@ -178,6 +149,7 @@
void ServiceWorker::stop()
{
m_isStopped = true;
+ scriptExecutionContext()->unregisterServiceWorker(*this);
updatePendingActivityForEventDispatch();
}
Modified: trunk/Source/WebCore/workers/service/ServiceWorker.h (225247 => 225248)
--- trunk/Source/WebCore/workers/service/ServiceWorker.h 2017-11-29 00:23:29 UTC (rev 225247)
+++ trunk/Source/WebCore/workers/service/ServiceWorker.h 2017-11-29 00:28:42 UTC (rev 225248)
@@ -64,11 +64,8 @@
using RefCounted::ref;
using RefCounted::deref;
- static const HashMap<ServiceWorkerIdentifier, HashSet<ServiceWorker*>>& allWorkers();
-
private:
ServiceWorker(ScriptExecutionContext&, ServiceWorkerData&&);
- static HashMap<ServiceWorkerIdentifier, HashSet<ServiceWorker*>>& mutableAllWorkers();
void updatePendingActivityForEventDispatch();
EventTargetInterface eventTargetInterface() const final;
Modified: trunk/Source/WebCore/workers/service/server/SWClientConnection.cpp (225247 => 225248)
--- trunk/Source/WebCore/workers/service/server/SWClientConnection.cpp 2017-11-29 00:23:29 UTC (rev 225247)
+++ trunk/Source/WebCore/workers/service/server/SWClientConnection.cpp 2017-11-29 00:28:42 UTC (rev 225248)
@@ -154,8 +154,11 @@
void SWClientConnection::updateWorkerState(ServiceWorkerIdentifier identifier, ServiceWorkerState state)
{
- for (auto* worker : ServiceWorker::allWorkers().get(identifier))
- worker->scheduleTaskToUpdateState(state);
+ // FIXME: We should iterate over all service worker clients, not only documents.
+ for (auto* document : Document::allDocuments()) {
+ if (auto* serviceWorker = document->serviceWorker(identifier))
+ serviceWorker->scheduleTaskToUpdateState(state);
+ }
}
void SWClientConnection::fireUpdateFoundEvent(ServiceWorkerRegistrationIdentifier identifier)
