[ Mac EWS ] imported/w3c/web-platform-tests/workers/semantics/multiple-workers/004.html is a flaky text failure
https://bugs.webkit.org/show_bug.cgi?id=237095
rdar://problem/89367636
Patch by Youenn Fablet <youennf@gmail.com> on 2022-06-23
Reviewed by Chris Dumez.
As per https://html.spec.whatwg.org/multipage/workers.html#concept-WorkerGlobalScope-owner-set,
a WorkerGlobalScope owner set should preserve the insertion order.
imported/w3c/web-platform-tests/workers/semantics/multiple-workers/004.html might be flakky as we sometimes do not run the shared worker synchronously.
In that case, we will send the connect event on the shared worker set.
Before the patch, the shared worker set would not be ordered so it might happen that the first connect event is related to an iframe SharedWorker.
After the patch, we ensure that the first SharedWorker (NetworkProcess being the place where insertion happens) will be the first to trigger the connect event.
This is done by changing the SharedWorker object set from a Map to a ListHashSet.
Covered by added LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering.html
* LayoutTests/http/wpt/service-workers/shared-workers: Added.
* LayoutTests/platform/mac-wk2/TestExpectations:
* Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp:
* Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h:
Canonical link: https://commits.webkit.org/251781@main
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@295776 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-expected.txt b/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-expected.txt
new file mode 100644
index 0000000..d78f6bc
--- /dev/null
+++ b/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-expected.txt
@@ -0,0 +1,3 @@
+
+PASS connect event should fire following SharedWorker creation order
+
diff --git a/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-sharedworker.js b/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-sharedworker.js
new file mode 100644
index 0000000..2d8f6ec
--- /dev/null
+++ b/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-sharedworker.js
@@ -0,0 +1,3 @@
+onconnect = (e) => {
+ e.ports[0].postMessage("got it");
+}
diff --git a/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering.html b/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering.html
new file mode 100644
index 0000000..d9529be
--- /dev/null
+++ b/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering.html
@@ -0,0 +1,30 @@
+<html>
+<head>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+if (window.testRunner)
+ testRunner.setUseSeparateServiceWorkerProcess(true);
+
+promise_test(async (test) => {
+ const worker1 = new SharedWorker('connect-event-ordering-sharedworker.js');
+ const worker2 = new SharedWorker('connect-event-ordering-sharedworker.js');
+
+ let result = '';
+ await Promise.all([
+ new Promise(resolve => { worker1.port.onmessage = () => {
+ result += 'worker1';
+ resolve();
+ }}),
+ new Promise(resolve => { worker2.port.onmessage = () => {
+ result += 'worker2';
+ resolve();
+ }})
+ ]);
+ assert_equals(result, 'worker1worker2');
+}, "connect event should fire following SharedWorker creation order");
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
index a7337e2..a03a19a 100644
--- a/LayoutTests/platform/mac-wk2/TestExpectations
+++ b/LayoutTests/platform/mac-wk2/TestExpectations
@@ -1618,8 +1618,6 @@
[ Monterey+ ] accessibility/model-element-attributes.html [ Skip ]
[ Monterey+ ] model-element/model-element-interactive.html [ Skip ]
-webkit.org/b/237095 imported/w3c/web-platform-tests/workers/semantics/multiple-workers/004.html [ Pass Failure ]
-
webkit.org/b/233621 http/tests/webgpu [ Skip ]
fast/text/install-font-style-recalc.html [ Pass ]
diff --git a/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp b/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp
index ec72b74..2af6075 100644
--- a/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp
+++ b/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp
@@ -54,8 +54,8 @@
WebSharedWorker::~WebSharedWorker()
{
if (auto* connection = contextConnection()) {
- for (auto& sharedWorkerObjectIdentifier : m_sharedWorkerObjects.keys())
- connection->removeSharedWorkerObject(sharedWorkerObjectIdentifier);
+ for (auto& sharedWorkerObject : m_sharedWorkerObjects)
+ connection->removeSharedWorkerObject(sharedWorkerObject.identifier);
}
ASSERT(allWorkers().get(m_identifier) == this);
@@ -79,8 +79,8 @@
void WebSharedWorker::didCreateContextConnection(WebSharedWorkerServerToContextConnection& contextConnection)
{
- for (auto& sharedWorkerObjectIdentifier : m_sharedWorkerObjects.keys())
- contextConnection.addSharedWorkerObject(sharedWorkerObjectIdentifier);
+ for (auto& sharedWorkerObject : m_sharedWorkerObjects)
+ contextConnection.addSharedWorkerObject(sharedWorkerObject.identifier);
if (didFinishFetching())
launch(contextConnection);
}
@@ -94,7 +94,8 @@
void WebSharedWorker::addSharedWorkerObject(WebCore::SharedWorkerObjectIdentifier sharedWorkerObjectIdentifier, const WebCore::TransferredMessagePort& port)
{
- m_sharedWorkerObjects.add(sharedWorkerObjectIdentifier, SharedWorkerObjectState { false, port });
+ ASSERT(!m_sharedWorkerObjects.contains({ sharedWorkerObjectIdentifier, { false, port } }));
+ m_sharedWorkerObjects.add({ sharedWorkerObjectIdentifier, { false, port } });
if (auto* connection = contextConnection())
connection->addSharedWorkerObject(sharedWorkerObjectIdentifier);
@@ -103,7 +104,8 @@
void WebSharedWorker::removeSharedWorkerObject(WebCore::SharedWorkerObjectIdentifier sharedWorkerObjectIdentifier)
{
- m_sharedWorkerObjects.remove(sharedWorkerObjectIdentifier);
+ ASSERT(m_sharedWorkerObjects.contains({ sharedWorkerObjectIdentifier, { } }));
+ m_sharedWorkerObjects.remove({ sharedWorkerObjectIdentifier, { } });
if (auto* connection = contextConnection())
connection->removeSharedWorkerObject(sharedWorkerObjectIdentifier);
@@ -112,13 +114,12 @@
void WebSharedWorker::suspend(WebCore::SharedWorkerObjectIdentifier sharedWorkerObjectIdentifier)
{
- auto iterator = m_sharedWorkerObjects.find(sharedWorkerObjectIdentifier);
+ auto iterator = m_sharedWorkerObjects.find({ sharedWorkerObjectIdentifier, { } });
if (iterator == m_sharedWorkerObjects.end())
return;
- iterator->value.isSuspended = true;
+ iterator->state.isSuspended = true;
ASSERT(!m_isSuspended);
-
suspendIfNeeded();
}
@@ -127,8 +128,8 @@
if (m_isSuspended)
return;
- for (auto& state : m_sharedWorkerObjects.values()) {
- if (!state.isSuspended)
+ for (auto& object : m_sharedWorkerObjects) {
+ if (!object.state.isSuspended)
return;
}
@@ -139,12 +140,11 @@
void WebSharedWorker::resume(WebCore::SharedWorkerObjectIdentifier sharedWorkerObjectIdentifier)
{
- auto iterator = m_sharedWorkerObjects.find(sharedWorkerObjectIdentifier);
+ auto iterator = m_sharedWorkerObjects.find({ sharedWorkerObjectIdentifier, { } });
if (iterator == m_sharedWorkerObjects.end())
return;
- iterator->value.isSuspended = false;
-
+ iterator->state.isSuspended = false;
resumeIfNeeded();
}
@@ -160,15 +160,15 @@
void WebSharedWorker::forEachSharedWorkerObject(const Function<void(WebCore::SharedWorkerObjectIdentifier, const WebCore::TransferredMessagePort&)>& apply) const
{
- for (auto& [sharedWorkerObjectIdentifier, state] : m_sharedWorkerObjects)
- apply(sharedWorkerObjectIdentifier, state.port);
+ for (auto& object : m_sharedWorkerObjects)
+ apply(object.identifier, object.state.port);
}
std::optional<WebCore::ProcessIdentifier> WebSharedWorker::firstSharedWorkerObjectProcess() const
{
if (m_sharedWorkerObjects.isEmpty())
return std::nullopt;
- return m_sharedWorkerObjects.begin()->key.processIdentifier();
+ return m_sharedWorkerObjects.first().identifier.processIdentifier();
}
WebSharedWorkerServerToContextConnection* WebSharedWorker::contextConnection() const
diff --git a/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h b/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h
index e0df69c..c5d763d 100644
--- a/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h
+++ b/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h
@@ -32,6 +32,7 @@
#include <WebCore/WorkerFetchResult.h>
#include <WebCore/WorkerInitializationData.h>
#include <WebCore/WorkerOptions.h>
+#include <wtf/ListHashSet.h>
#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -81,6 +82,16 @@
void launch(WebSharedWorkerServerToContextConnection&);
+ struct SharedWorkerObjectState {
+ bool isSuspended { false };
+ WebCore::TransferredMessagePort port;
+ };
+
+ struct Object {
+ WebCore::SharedWorkerObjectIdentifier identifier;
+ SharedWorkerObjectState state;
+ };
+
private:
WebSharedWorker(WebSharedWorker&&) = delete;
WebSharedWorker& operator=(WebSharedWorker&&) = delete;
@@ -90,16 +101,11 @@
void suspendIfNeeded();
void resumeIfNeeded();
- struct SharedWorkerObjectState {
- bool isSuspended { false };
- WebCore::TransferredMessagePort port;
- };
-
WebSharedWorkerServer& m_server;
WebCore::SharedWorkerIdentifier m_identifier;
WebCore::SharedWorkerKey m_key;
WebCore::WorkerOptions m_workerOptions;
- HashMap<WebCore::SharedWorkerObjectIdentifier, SharedWorkerObjectState> m_sharedWorkerObjects;
+ ListHashSet<Object> m_sharedWorkerObjects;
WebCore::WorkerFetchResult m_fetchResult;
WebCore::WorkerInitializationData m_initializationData;
bool m_isRunning { false };
@@ -107,3 +113,16 @@
};
} // namespace WebKit
+
+namespace WTF {
+
+struct WebSharedWorkerObjectHash {
+ static unsigned hash(const WebKit::WebSharedWorker::Object& object) { return DefaultHash<WebCore::SharedWorkerObjectIdentifier>::hash(object.identifier); }
+ static bool equal(const WebKit::WebSharedWorker::Object& a, const WebKit::WebSharedWorker::Object& b) { return a.identifier == b.identifier; }
+ static constexpr bool safeToCompareToEmptyOrDeleted = true;
+};
+
+template<> struct HashTraits<WebKit::WebSharedWorker::Object> : SimpleClassHashTraits<WebSharedWorkerObjectHash> { };
+template<> struct DefaultHash<WebKit::WebSharedWorker::Object> : WebSharedWorkerObjectHash { };
+
+} // namespace WTF
