Title: [225308] trunk
Revision
225308
Author
commit-qu...@webkit.org
Date
2017-11-29 17:19:25 -0800 (Wed, 29 Nov 2017)

Log Message

ServiceWorkerClient objects should be reused if there is already one existing with the same identifier
https://bugs.webkit.org/show_bug.cgi?id=180143

Patch by Youenn Fablet <you...@apple.com> on 2017-11-29
Reviewed by Chris Dumez.

Source/WebCore:

Covered by updated tests.

ServiceWorkerGlobalScope keeps a map of all live ServiceWorkerClient objects.
Before creating a new client, it checks whether the map has one such object with the same identifier.
If so, it reuses this object. Otherwise it creates either a ServiceWorkerWindowClient or ServiceWorkerClient.

Add support for using a ServiceWorkerClientIdentifier as a HashMap key.

* workers/service/ServiceWorkerClient.cpp:
(WebCore::ServiceWorkerClient::ServiceWorkerClient):
(WebCore::ServiceWorkerClient::~ServiceWorkerClient):
* workers/service/ServiceWorkerClient.h:
(WebCore::ServiceWorkerClient::getOrCreate):
* workers/service/ServiceWorkerClientIdentifier.h:
(WebCore::ServiceWorkerClientIdentifier::hash const):
(WTF::ServiceWorkerClientIdentifierHash::hash):
(WTF::ServiceWorkerClientIdentifierHash::equal):
(WTF::HashTraits<WebCore::ServiceWorkerClientIdentifier>::emptyValue):
(WTF::HashTraits<WebCore::ServiceWorkerClientIdentifier>::constructDeletedValue):
(WTF::HashTraits<WebCore::ServiceWorkerClientIdentifier>::isDeletedValue):
* workers/service/ServiceWorkerGlobalScope.cpp:
(WebCore::ServiceWorkerGlobalScope::serviceWorkerClient):
(WebCore::ServiceWorkerGlobalScope::addServiceWorkerClient):
(WebCore::ServiceWorkerGlobalScope::removeServiceWorkerClient):
* workers/service/ServiceWorkerGlobalScope.h:
* workers/service/ServiceWorkerWindowClient.cpp:
(WebCore::ServiceWorkerWindowClient::ServiceWorkerWindowClient):
* workers/service/ServiceWorkerWindowClient.h:
* workers/service/context/ServiceWorkerThread.cpp:
(WebCore::ServiceWorkerThread::postMessageToServiceWorkerGlobalScope):

LayoutTests:

* http/tests/workers/service/resources/basic-ServiceWorker-postMessage-worker.js:
(event.else):
* http/tests/workers/service/resources/basic-ServiceWorker-postMessage.js:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (225307 => 225308)


--- trunk/LayoutTests/ChangeLog	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/LayoutTests/ChangeLog	2017-11-30 01:19:25 UTC (rev 225308)
@@ -1,3 +1,14 @@
+2017-11-29  Youenn Fablet  <you...@apple.com>
+
+        ServiceWorkerClient objects should be reused if there is already one existing with the same identifier
+        https://bugs.webkit.org/show_bug.cgi?id=180143
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/workers/service/resources/basic-ServiceWorker-postMessage-worker.js:
+        (event.else):
+        * http/tests/workers/service/resources/basic-ServiceWorker-postMessage.js:
+
 2017-11-29  Ryan Haddad  <ryanhad...@apple.com>
 
         Mark imported/w3c/web-platform-tests/XMLHttpRequest/firing-events-http-no-content-length.html as flaky.

Modified: trunk/LayoutTests/http/tests/workers/service/resources/basic-ServiceWorker-postMessage-worker.js (225307 => 225308)


--- trunk/LayoutTests/http/tests/workers/service/resources/basic-ServiceWorker-postMessage-worker.js	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/LayoutTests/http/tests/workers/service/resources/basic-ServiceWorker-postMessage-worker.js	2017-11-30 01:19:25 UTC (rev 225308)
@@ -1,4 +1,15 @@
+var client = null;
 self.addEventListener("message", (event) => {
-    event.source.postMessage("Service worker received message '" + event.data + "' from origin '" + event.origin + "'");
+    if (!client) {
+        client = event.source;
+        if (!(client instanceof WindowClient)) {
+            event.source.postMessage("FAIL: client source is not a WindowClient");
+            return;
+        }
+    } else if (client !== event.source) {
+        event.source.postMessage("FAIL: client source of the second message is not the same as the first message");
+        return;
+    }
+    event.source.postMessage("PASS: Service worker received message '" + event.data + "' from origin '" + event.origin + "'");
 });
 

Modified: trunk/LayoutTests/http/tests/workers/service/resources/basic-ServiceWorker-postMessage.js (225307 => 225308)


--- trunk/LayoutTests/http/tests/workers/service/resources/basic-ServiceWorker-postMessage.js	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/LayoutTests/http/tests/workers/service/resources/basic-ServiceWorker-postMessage.js	2017-11-30 01:19:25 UTC (rev 225308)
@@ -1,7 +1,7 @@
 var messageNumber = 1;
 navigator.serviceWorker.addEventListener("message", function(event) {
     log("PASS: Client received message from service worker, origin: " + event.origin);
-    log("PASS: " + event.data);
+    log(event.data);
     if (messageNumber == 1) {
         event.source.postMessage("Message 2");
         messageNumber++;

Modified: trunk/Source/WebCore/ChangeLog (225307 => 225308)


--- trunk/Source/WebCore/ChangeLog	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/ChangeLog	2017-11-30 01:19:25 UTC (rev 225308)
@@ -1,3 +1,41 @@
+2017-11-29  Youenn Fablet  <you...@apple.com>
+
+        ServiceWorkerClient objects should be reused if there is already one existing with the same identifier
+        https://bugs.webkit.org/show_bug.cgi?id=180143
+
+        Reviewed by Chris Dumez.
+
+        Covered by updated tests.
+
+        ServiceWorkerGlobalScope keeps a map of all live ServiceWorkerClient objects.
+        Before creating a new client, it checks whether the map has one such object with the same identifier.
+        If so, it reuses this object. Otherwise it creates either a ServiceWorkerWindowClient or ServiceWorkerClient.
+
+        Add support for using a ServiceWorkerClientIdentifier as a HashMap key.
+
+        * workers/service/ServiceWorkerClient.cpp:
+        (WebCore::ServiceWorkerClient::ServiceWorkerClient):
+        (WebCore::ServiceWorkerClient::~ServiceWorkerClient):
+        * workers/service/ServiceWorkerClient.h:
+        (WebCore::ServiceWorkerClient::getOrCreate):
+        * workers/service/ServiceWorkerClientIdentifier.h:
+        (WebCore::ServiceWorkerClientIdentifier::hash const):
+        (WTF::ServiceWorkerClientIdentifierHash::hash):
+        (WTF::ServiceWorkerClientIdentifierHash::equal):
+        (WTF::HashTraits<WebCore::ServiceWorkerClientIdentifier>::emptyValue):
+        (WTF::HashTraits<WebCore::ServiceWorkerClientIdentifier>::constructDeletedValue):
+        (WTF::HashTraits<WebCore::ServiceWorkerClientIdentifier>::isDeletedValue):
+        * workers/service/ServiceWorkerGlobalScope.cpp:
+        (WebCore::ServiceWorkerGlobalScope::serviceWorkerClient):
+        (WebCore::ServiceWorkerGlobalScope::addServiceWorkerClient):
+        (WebCore::ServiceWorkerGlobalScope::removeServiceWorkerClient):
+        * workers/service/ServiceWorkerGlobalScope.h:
+        * workers/service/ServiceWorkerWindowClient.cpp:
+        (WebCore::ServiceWorkerWindowClient::ServiceWorkerWindowClient):
+        * workers/service/ServiceWorkerWindowClient.h:
+        * workers/service/context/ServiceWorkerThread.cpp:
+        (WebCore::ServiceWorkerThread::postMessageToServiceWorkerGlobalScope):
+
 2017-11-29  Zalan Bujtas  <za...@apple.com>
 
         Add missing WTF_MAKE_ISO_ALLOCATED macros

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerClient.cpp (225307 => 225308)


--- trunk/Source/WebCore/workers/service/ServiceWorkerClient.cpp	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerClient.cpp	2017-11-30 01:19:25 UTC (rev 225308)
@@ -34,18 +34,33 @@
 #include "SerializedScriptValue.h"
 #include "ServiceWorkerGlobalScope.h"
 #include "ServiceWorkerThread.h"
+#include "ServiceWorkerWindowClient.h"
 
 namespace WebCore {
 
-ServiceWorkerClient::ServiceWorkerClient(ScriptExecutionContext& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
+Ref<ServiceWorkerClient> ServiceWorkerClient::getOrCreate(ServiceWorkerGlobalScope& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
+{
+    if (auto* client = context.serviceWorkerClient(identifier))
+        return *client;
+
+    if (data.type == ServiceWorkerClientType::Window)
+        return ServiceWorkerWindowClient::create(context, identifier, WTFMove(data));
+
+    return adoptRef(*new ServiceWorkerClient { context, identifier, WTFMove(data) });
+}
+
+ServiceWorkerClient::ServiceWorkerClient(ServiceWorkerGlobalScope& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
     : ContextDestructionObserver(&context)
     , m_identifier(identifier)
     , m_data(WTFMove(data))
 {
+    context.addServiceWorkerClient(*this);
 }
 
 ServiceWorkerClient::~ServiceWorkerClient()
 {
+    if (auto* context = scriptExecutionContext())
+        downcast<ServiceWorkerGlobalScope>(*context).removeServiceWorkerClient(*this);
 }
 
 const URL& ServiceWorkerClient::url() const

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerClient.h (225307 => 225308)


--- trunk/Source/WebCore/workers/service/ServiceWorkerClient.h	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerClient.h	2017-11-30 01:19:25 UTC (rev 225308)
@@ -39,6 +39,8 @@
 
 namespace WebCore {
 
+class ServiceWorkerGlobalScope;
+
 class ServiceWorkerClient : public RefCounted<ServiceWorkerClient>, public ContextDestructionObserver {
 public:
     using Identifier = ServiceWorkerClientIdentifier;
@@ -46,10 +48,7 @@
     using Type = ServiceWorkerClientType;
     using FrameType = ServiceWorkerClientFrameType;
 
-    static Ref<ServiceWorkerClient> create(ScriptExecutionContext& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
-    {
-        return adoptRef(*new ServiceWorkerClient(context, identifier, WTFMove(data)));
-    }
+    static Ref<ServiceWorkerClient> getOrCreate(ServiceWorkerGlobalScope&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&);
 
     ~ServiceWorkerClient();
 
@@ -63,7 +62,7 @@
     ExceptionOr<void> postMessage(ScriptExecutionContext&, JSC::JSValue message, Vector<JSC::Strong<JSC::JSObject>>&& transfer);
 
 protected:
-    ServiceWorkerClient(ScriptExecutionContext&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&);
+    ServiceWorkerClient(ServiceWorkerGlobalScope&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&);
 
     ServiceWorkerClientIdentifier m_identifier;
     ServiceWorkerClientData m_data;

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerClientIdentifier.h (225307 => 225308)


--- trunk/Source/WebCore/workers/service/ServiceWorkerClientIdentifier.h	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerClientIdentifier.h	2017-11-30 01:19:25 UTC (rev 225308)
@@ -37,6 +37,8 @@
     SWServerConnectionIdentifier serverConnectionIdentifier;
     DocumentIdentifier contextIdentifier;
 
+    unsigned hash() const;
+
     String toString() const { return String::number(serverConnectionIdentifier.toUInt64()) + "-" +  String::number(contextIdentifier.toUInt64()); }
 
     template<class Encoder> void encode(Encoder&) const;
@@ -70,6 +72,37 @@
     return { { WTFMove(*serverConnectionIdentifier), WTFMove(*contextIdentifier) } };
 }
 
+inline unsigned ServiceWorkerClientIdentifier::hash() const
+{
+    unsigned hashes[2];
+    hashes[0] = WTF::intHash(serverConnectionIdentifier.toUInt64());
+    hashes[1] = WTF::intHash(contextIdentifier.toUInt64());
+
+    return StringHasher::hashMemory(hashes, sizeof(hashes));
+}
+
 } // namespace WebCore
 
+namespace WTF {
+
+struct ServiceWorkerClientIdentifierHash {
+    static unsigned hash(const WebCore::ServiceWorkerClientIdentifier& key) { return key.hash(); }
+    static bool equal(const WebCore::ServiceWorkerClientIdentifier& a, const WebCore::ServiceWorkerClientIdentifier& b) { return a == b; }
+    static const bool safeToCompareToEmptyOrDeleted = true;
+};
+
+template<> struct HashTraits<WebCore::ServiceWorkerClientIdentifier> : GenericHashTraits<WebCore::ServiceWorkerClientIdentifier> {
+    static WebCore::ServiceWorkerClientIdentifier emptyValue() { return { }; }
+
+    static void constructDeletedValue(WebCore::ServiceWorkerClientIdentifier& slot) { slot.serverConnectionIdentifier = makeObjectIdentifier<WebCore::SWServerConnectionIdentifierType>(std::numeric_limits<uint64_t>::max()); }
+
+    static bool isDeletedValue(const WebCore::ServiceWorkerClientIdentifier& slot) { return slot.serverConnectionIdentifier.toUInt64() == std::numeric_limits<uint64_t>::max(); }
+};
+
+template<> struct DefaultHash<WebCore::ServiceWorkerClientIdentifier> {
+    typedef ServiceWorkerClientIdentifierHash Hash;
+};
+
+}
+
 #endif // ENABLE(SERVICE_WORKER)

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp (225307 => 225308)


--- trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp	2017-11-30 01:19:25 UTC (rev 225308)
@@ -28,8 +28,10 @@
 
 #if ENABLE(SERVICE_WORKER)
 
+#include "ServiceWorkerClient.h"
 #include "ServiceWorkerClients.h"
 #include "ServiceWorkerThread.h"
+#include "ServiceWorkerWindowClient.h"
 #include "WorkerNavigator.h"
 
 namespace WebCore {
@@ -58,6 +60,23 @@
     return static_cast<ServiceWorkerThread&>(WorkerGlobalScope::thread());
 }
 
+ServiceWorkerClient* ServiceWorkerGlobalScope::serviceWorkerClient(ServiceWorkerClientIdentifier identifier)
+{
+    return m_clientMap.get(identifier);
+}
+
+void ServiceWorkerGlobalScope::addServiceWorkerClient(ServiceWorkerClient& client)
+{
+    auto result = m_clientMap.add(client.identifier(), &client);
+    ASSERT_UNUSED(result, result.isNewEntry);
+}
+
+void ServiceWorkerGlobalScope::removeServiceWorkerClient(ServiceWorkerClient& client)
+{
+    auto isRemoved = m_clientMap.remove(client.identifier());
+    ASSERT_UNUSED(isRemoved, isRemoved);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(SERVICE_WORKER)

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h (225307 => 225308)


--- trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h	2017-11-30 01:19:25 UTC (rev 225308)
@@ -27,6 +27,7 @@
 
 #if ENABLE(SERVICE_WORKER)
 
+#include "ServiceWorkerClientIdentifier.h"
 #include "ServiceWorkerContextData.h"
 #include "ServiceWorkerRegistration.h"
 #include "WorkerGlobalScope.h"
@@ -34,6 +35,8 @@
 namespace WebCore {
 
 class DeferredPromise;
+struct ServiceWorkerClientData;
+class ServiceWorkerClient;
 class ServiceWorkerClients;
 class ServiceWorkerThread;
 
@@ -57,6 +60,10 @@
 
     ServiceWorkerThread& thread();
 
+    ServiceWorkerClient* serviceWorkerClient(ServiceWorkerClientIdentifier);
+    void addServiceWorkerClient(ServiceWorkerClient&);
+    void removeServiceWorkerClient(ServiceWorkerClient&);
+
 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);
 
@@ -63,6 +70,7 @@
     ServiceWorkerContextData m_contextData;
     Ref<ServiceWorkerRegistration> m_registration;
     Ref<ServiceWorkerClients> m_clients;
+    HashMap<ServiceWorkerClientIdentifier, ServiceWorkerClient*> m_clientMap;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerWindowClient.cpp (225307 => 225308)


--- trunk/Source/WebCore/workers/service/ServiceWorkerWindowClient.cpp	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerWindowClient.cpp	2017-11-30 01:19:25 UTC (rev 225308)
@@ -32,7 +32,7 @@
 
 namespace WebCore {
 
-ServiceWorkerWindowClient::ServiceWorkerWindowClient(ScriptExecutionContext& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
+ServiceWorkerWindowClient::ServiceWorkerWindowClient(ServiceWorkerGlobalScope& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
     : ServiceWorkerClient(context, identifier, WTFMove(data))
 {
 }

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerWindowClient.h (225307 => 225308)


--- trunk/Source/WebCore/workers/service/ServiceWorkerWindowClient.h	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerWindowClient.h	2017-11-30 01:19:25 UTC (rev 225308)
@@ -33,10 +33,11 @@
 namespace WebCore {
 
 class DeferredPromise;
+class ServiceWorkerGlobalScope;
 
 class ServiceWorkerWindowClient final : public ServiceWorkerClient {
 public:
-    static Ref<ServiceWorkerWindowClient> create(ScriptExecutionContext& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
+    static Ref<ServiceWorkerWindowClient> create(ServiceWorkerGlobalScope& context, ServiceWorkerClientIdentifier identifier, ServiceWorkerClientData&& data)
     {
         return adoptRef(*new ServiceWorkerWindowClient(context, identifier, WTFMove(data)));
     }
@@ -48,7 +49,7 @@
     void navigate(const String& url, Ref<DeferredPromise>&&);
 
 private:
-    ServiceWorkerWindowClient(ScriptExecutionContext&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&);
+    ServiceWorkerWindowClient(ServiceWorkerGlobalScope&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&);
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp (225307 => 225308)


--- trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp	2017-11-30 01:05:01 UTC (rev 225307)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp	2017-11-30 01:19:25 UTC (rev 225308)
@@ -109,7 +109,7 @@
     ScriptExecutionContext::Task task([this, channels = WTFMove(channels), message = WTFMove(message), sourceIdentifier, sourceData = sourceData.isolatedCopy()] (ScriptExecutionContext& context) mutable {
         auto& serviceWorkerGlobalScope = downcast<ServiceWorkerGlobalScope>(context);
         auto ports = MessagePort::entanglePorts(serviceWorkerGlobalScope, WTFMove(channels));
-        RefPtr<ServiceWorkerClient> source = ServiceWorkerWindowClient::create(context, sourceIdentifier, WTFMove(sourceData));
+        RefPtr<ServiceWorkerClient> source = ServiceWorkerClient::getOrCreate(serviceWorkerGlobalScope, sourceIdentifier, WTFMove(sourceData));
         auto sourceOrigin = SecurityOrigin::create(source->url());
         auto messageEvent = ExtendableMessageEvent::create(WTFMove(ports), WTFMove(message), sourceOrigin->toString(), { }, ExtendableMessageEventSource { source });
         serviceWorkerGlobalScope.dispatchEvent(messageEvent);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to