- Revision
- 295776
- Author
- commit-qu...@webkit.org
- Date
- 2022-06-23 05:43:18 -0700 (Thu, 23 Jun 2022)
Log Message
[ 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 <youe...@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
Modified Paths
Added Paths
Diff
Added: trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-expected.txt (0 => 295776)
--- trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-expected.txt (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-expected.txt 2022-06-23 12:43:18 UTC (rev 295776)
@@ -0,0 +1,3 @@
+
+PASS connect event should fire following SharedWorker creation order
+
Added: trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-sharedworker.js (0 => 295776)
--- trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-sharedworker.js (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering-sharedworker.js 2022-06-23 12:43:18 UTC (rev 295776)
@@ -0,0 +1,3 @@
+_onconnect_ = (e) => {
+ e.ports[0].postMessage("got it");
+}
Added: trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering.html (0 => 295776)
--- trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering.html (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/shared-workers/connect-event-ordering.html 2022-06-23 12:43:18 UTC (rev 295776)
@@ -0,0 +1,30 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</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>
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (295775 => 295776)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2022-06-23 12:40:44 UTC (rev 295775)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2022-06-23 12:43:18 UTC (rev 295776)
@@ -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 ]
Modified: trunk/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp (295775 => 295776)
--- trunk/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp 2022-06-23 12:40:44 UTC (rev 295775)
+++ trunk/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp 2022-06-23 12:43:18 UTC (rev 295776)
@@ -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,8 +160,8 @@
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
@@ -168,7 +168,7 @@
{
if (m_sharedWorkerObjects.isEmpty())
return std::nullopt;
- return m_sharedWorkerObjects.begin()->key.processIdentifier();
+ return m_sharedWorkerObjects.first().identifier.processIdentifier();
}
WebSharedWorkerServerToContextConnection* WebSharedWorker::contextConnection() const
Modified: trunk/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h (295775 => 295776)
--- trunk/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h 2022-06-23 12:40:44 UTC (rev 295775)
+++ trunk/Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h 2022-06-23 12:43:18 UTC (rev 295776)
@@ -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
