Skip to content

Commit b5f2a68

Browse files
youennfyouennf
committed
BroadcastChannel is not always computing correctly its origin
https://bugs.webkit.org/show_bug.cgi?id=248869 rdar://102519898 Reviewed by Chris Dumez. Shared Workers document is also its own top document. This makes Document::topOrigin() computation wrong in case of third-party Shared Workers. Update BroadcastChannel to directly use ScriptExecutionContext::topOrigin instead. As a follow-up, we should probably update third-party SharedWorker/ServiceWorker document creation to ensure this mistake does not pop up again. * LayoutTests/http/wpt/shared-workers/resources/third-party-shared-worker-broadcast-channel-iframe.html: Added. * LayoutTests/http/wpt/shared-workers/resources/third-party-shared-worker-broadcast-channel-sharedworker.js: Added. * LayoutTests/http/wpt/shared-workers/third-party-shared-worker-broadcast-channel-expected.txt: Added. * LayoutTests/http/wpt/shared-workers/third-party-shared-worker-broadcast-channel.html: Added. * Source/WebCore/dom/BroadcastChannel.cpp: (WebCore::shouldPartitionOrigin): (WebCore::BroadcastChannel::MainThreadBridge::ensureOnMainThread): (WebCore::BroadcastChannel::MainThreadBridge::registerChannel): (WebCore::BroadcastChannel::MainThreadBridge::unregisterChannel): (WebCore::BroadcastChannel::MainThreadBridge::postMessage): * Source/WebCore/dom/BroadcastChannel.h: * Source/WebCore/page/PartitionedSecurityOrigin.h: (WebCore::PartitionedSecurityOrigin::isolatedCopy const): * Source/WebKit/NetworkProcess/NetworkBroadcastChannelRegistry.cpp: (WebKit::NetworkBroadcastChannelRegistry::registerChannel): (WebKit::NetworkBroadcastChannelRegistry::unregisterChannel): (WebKit::NetworkBroadcastChannelRegistry::postMessage): * Source/WebKit/WebProcess/WebCoreSupport/WebBroadcastChannelRegistry.cpp: (WebKit::WebBroadcastChannelRegistry::registerChannel): (WebKit::WebBroadcastChannelRegistry::unregisterChannel): (WebKit::WebBroadcastChannelRegistry::postMessage): (WebKit::WebBroadcastChannelRegistry::postMessageLocally): (WebKit::WebBroadcastChannelRegistry::postMessageToRemote): Canonical link: https://commits.webkit.org/257551@main
1 parent 65071a6 commit b5f2a68

7 files changed

+74
-23
lines changed

‎LayoutTests/http/wpt/shared-workers/resources/third-party-shared-worker-broadcast-channel-iframe.html

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<html>
2+
<head>
3+
<script src="/resources/testharness.js"></script>
4+
<script src="/resources/testharnessreport.js"></script>
5+
</head>
6+
<body>
7+
<script>
8+
const channel = new BroadcastChannel('test');
9+
10+
channel.onmessage = function (e) {
11+
parent.postMessage(e.data, "*");
12+
}
13+
14+
const worker = new SharedWorker("third-party-shared-worker-broadcast-channel-sharedworker.js");
15+
</script>
16+
</body>
17+
</html>

‎LayoutTests/http/wpt/shared-workers/resources/third-party-shared-worker-broadcast-channel-sharedworker.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
const channel = new BroadcastChannel('test');
2+
channel.postMessage('hello');

‎LayoutTests/http/wpt/shared-workers/third-party-shared-worker-broadcast-channel-expected.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
PASS BroadcastChannel between third-party iframe and shared worker should work properly
3+

‎LayoutTests/http/wpt/shared-workers/third-party-shared-worker-broadcast-channel.html

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<html><!-- webkit-test-runner [ BroadcastChannelOriginPartitioningEnabled=true ] -->
2+
<head>
3+
<script src="/resources/testharness.js"></script>
4+
<script src="/resources/testharnessreport.js"></script>
5+
<script src="/common/get-host-info.sub.js"></script>
6+
</head>
7+
<body>
8+
<script>
9+
function with_iframe(url) {
10+
return new Promise(function(resolve) {
11+
var frame = document.createElement('iframe');
12+
frame.className = 'test-iframe';
13+
frame.src = url;
14+
frame.onload = function() { resolve(frame); };
15+
document.body.appendChild(frame);
16+
});
17+
}
18+
19+
promise_test(async (t) => {
20+
const frame = await with_iframe(get_host_info().HTTP_REMOTE_ORIGIN + "/WebKit/shared-workers/resources/third-party-shared-worker-broadcast-channel-iframe.html");
21+
const data = await new Promise(resolve => window.onmessage = (e) => resolve(e.data));
22+
assert_equals(data, "hello");
23+
frame.remove();
24+
}, "BroadcastChannel between third-party iframe and shared worker should work properly");
25+
</script>
26+
</body>
27+
</html>

‎Source/WebCore/dom/BroadcastChannel.cpp

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,11 @@ static HashMap<BroadcastChannelIdentifier, ScriptExecutionContextIdentifier>& ch
6060
return map;
6161
}
6262

63-
static bool shouldPartitionOrigin(Document& document)
63+
static PartitionedSecurityOrigin partitionedSecurityOriginFromContext(ScriptExecutionContext& context)
6464
{
65-
return document.settings().broadcastChannelOriginPartitioningEnabled();
65+
Ref securityOrigin { *context.securityOrigin() };
66+
Ref topOrigin { context.settingsValues().broadcastChannelOriginPartitioningEnabled ? context.topOrigin() : securityOrigin.get() };
67+
return { WTFMove(topOrigin), WTFMove(securityOrigin) };
6668
}
6769

6870
class BroadcastChannel::MainThreadBridge : public ThreadSafeRefCounted<MainThreadBridge, WTF::DestructionThread::Main> {
@@ -82,22 +84,23 @@ class BroadcastChannel::MainThreadBridge : public ThreadSafeRefCounted<MainThrea
8284
private:
8385
MainThreadBridge(BroadcastChannel&, const String& name);
8486

85-
void ensureOnMainThread(Function<void(Document&)>&&);
87+
void ensureOnMainThread(Function<void(Page*)>&&);
8688

8789
WeakPtr<BroadcastChannel, WeakPtrImplWithEventTargetData> m_broadcastChannel;
8890
const BroadcastChannelIdentifier m_identifier;
8991
const String m_name; // Main thread only.
90-
std::optional<PartitionedSecurityOrigin> m_origin; // Main thread only.
92+
PartitionedSecurityOrigin m_origin; // Main thread only.
9193
};
9294

9395
BroadcastChannel::MainThreadBridge::MainThreadBridge(BroadcastChannel& channel, const String& name)
9496
: m_broadcastChannel(channel)
9597
, m_identifier(BroadcastChannelIdentifier::generateThreadSafe())
9698
, m_name(name.isolatedCopy())
99+
, m_origin(partitionedSecurityOriginFromContext(*channel.scriptExecutionContext()).isolatedCopy())
97100
{
98101
}
99102

100-
void BroadcastChannel::MainThreadBridge::ensureOnMainThread(Function<void(Document&)>&& task)
103+
void BroadcastChannel::MainThreadBridge::ensureOnMainThread(Function<void(Page*)>&& task)
101104
{
102105
ASSERT(m_broadcastChannel);
103106
if (!m_broadcastChannel)
@@ -109,44 +112,42 @@ void BroadcastChannel::MainThreadBridge::ensureOnMainThread(Function<void(Docume
109112
ASSERT(context->isContextThread());
110113

111114
Ref protectedThis { *this };
112-
if (auto document = dynamicDowncast<Document>(*context))
113-
task(*document);
114-
else {
115-
downcast<WorkerGlobalScope>(*context).thread().workerLoaderProxy().postTaskToLoader([protectedThis = WTFMove(protectedThis), task = WTFMove(task)](auto& context) {
116-
task(downcast<Document>(context));
117-
});
115+
if (auto* document = dynamicDowncast<Document>(*context)) {
116+
task(document->page());
117+
return;
118118
}
119+
120+
downcast<WorkerGlobalScope>(*context).thread().workerLoaderProxy().postTaskToLoader([protectedThis = WTFMove(protectedThis), task = WTFMove(task)](auto& context) {
121+
task(downcast<Document>(context).page());
122+
});
119123
}
120124

121125
void BroadcastChannel::MainThreadBridge::registerChannel()
122126
{
123-
auto securityOrigin = m_broadcastChannel->scriptExecutionContext()->securityOrigin()->isolatedCopy();
124-
ensureOnMainThread([this, contextIdentifier = m_broadcastChannel->scriptExecutionContext()->identifier(), securityOrigin = WTFMove(securityOrigin)](auto& document) {
125-
m_origin = PartitionedSecurityOrigin { shouldPartitionOrigin(document) ? Ref { document.topOrigin() } : securityOrigin.copyRef(), securityOrigin.copyRef() };
126-
if (auto* page = document.page())
127-
page->broadcastChannelRegistry().registerChannel(*m_origin, m_name, m_identifier);
127+
ensureOnMainThread([this, contextIdentifier = m_broadcastChannel->scriptExecutionContext()->identifier()](auto* page) mutable {
128+
if (page)
129+
page->broadcastChannelRegistry().registerChannel(m_origin, m_name, m_identifier);
128130
channelToContextIdentifier().add(m_identifier, contextIdentifier);
129131
});
130132
}
131133

132134
void BroadcastChannel::MainThreadBridge::unregisterChannel()
133135
{
134-
ensureOnMainThread([this](auto& document) {
135-
if (auto* page = document.page())
136-
page->broadcastChannelRegistry().unregisterChannel(*m_origin, m_name, m_identifier);
136+
ensureOnMainThread([this](auto* page) {
137+
if (page)
138+
page->broadcastChannelRegistry().unregisterChannel(m_origin, m_name, m_identifier);
137139
channelToContextIdentifier().remove(m_identifier);
138140
});
139141
}
140142

141143
void BroadcastChannel::MainThreadBridge::postMessage(Ref<SerializedScriptValue>&& message)
142144
{
143-
ensureOnMainThread([this, message = WTFMove(message)](auto& document) mutable {
144-
auto* page = document.page();
145+
ensureOnMainThread([this, message = WTFMove(message)](auto* page) mutable {
145146
if (!page)
146147
return;
147148

148149
auto blobHandles = message->blobHandles();
149-
page->broadcastChannelRegistry().postMessage(*m_origin, m_name, m_identifier, WTFMove(message), [blobHandles = WTFMove(blobHandles)] {
150+
page->broadcastChannelRegistry().postMessage(m_origin, m_name, m_identifier, WTFMove(message), [blobHandles = WTFMove(blobHandles)] {
150151
// Keeps Blob data inside messageData alive until the message has been delivered.
151152
});
152153
});

‎Source/WebCore/dom/BroadcastChannel.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ class BroadcastChannel : public RefCounted<BroadcastChannel>, public EventTarget
6868
BroadcastChannel(ScriptExecutionContext&, const String& name);
6969

7070
void dispatchMessage(Ref<SerializedScriptValue>&&);
71-
void ensureOnMainThread(Function<void(Document&)>&&);
7271

7372
bool isEligibleForMessaging() const;
7473

‎Source/WebCore/page/PartitionedSecurityOrigin.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ struct PartitionedSecurityOrigin {
5151
bool isHashTableDeletedValue() const { return topOrigin.isHashTableDeletedValue(); }
5252
bool isHashTableEmptyValue() const { return topOrigin.isHashTableEmptyValue(); }
5353

54+
PartitionedSecurityOrigin isolatedCopy() const { return { topOrigin->isolatedCopy(), clientOrigin->isolatedCopy() }; }
55+
5456
Ref<SecurityOrigin> topOrigin;
5557
Ref<SecurityOrigin> clientOrigin;
5658
};

0 commit comments

Comments
 (0)