Skip to content
Permalink
Browse files
[ Mac EWS ] imported/w3c/web-platform-tests/workers/semantics/multipl…
…e-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: https://svn.webkit.org/repository/webkit/trunk@295776 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
youennf authored and webkit-commit-queue committed Jun 23, 2022
1 parent 072e834 commit 4d3602000572bf9c9f374922ac4e2a7f628f4a03
Showing 6 changed files with 78 additions and 25 deletions.
@@ -0,0 +1,3 @@

PASS connect event should fire following SharedWorker creation order

@@ -0,0 +1,3 @@
onconnect = (e) => {
e.ports[0].postMessage("got it");
}
@@ -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>
@@ -1618,8 +1618,6 @@ http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404.
[ 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 ]
@@ -54,8 +54,8 @@ WebSharedWorker::WebSharedWorker(WebSharedWorkerServer& server, const WebCore::S
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::setFetchResult(WebCore::WorkerFetchResult&& fetchResult)

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::launch(WebSharedWorkerServerToContextConnection& connectio

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::addSharedWorkerObject(WebCore::SharedWorkerObjectIdentifie

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::removeSharedWorkerObject(WebCore::SharedWorkerObjectIdenti

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 @@ void WebSharedWorker::suspendIfNeeded()
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::suspendIfNeeded()

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::resumeIfNeeded()

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
@@ -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 @@ class WebSharedWorker : public CanMakeWeakPtr<WebSharedWorker> {

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,20 +101,28 @@ class WebSharedWorker : public CanMakeWeakPtr<WebSharedWorker> {
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 };
bool m_isSuspended { false };
};

} // 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

0 comments on commit 4d36020

Please sign in to comment.