[webkit-reviews] review denied: [Bug 227924] Add initial support for BroadcastChannel behind a runtime flag : [Attachment 433515] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jul 14 13:52:14 PDT 2021
Alex Christensen <achristensen at apple.com> has denied Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 227924: Add initial support for BroadcastChannel behind a runtime flag
https://bugs.webkit.org/show_bug.cgi?id=227924
Attachment 433515: Patch
https://bugs.webkit.org/attachment.cgi?id=433515&action=review
--- Comment #7 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 433515
--> https://bugs.webkit.org/attachment.cgi?id=433515
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=433515&action=review
> Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:96
> + default: false
It might be nice to add a comment saying that it should stay off until
https://github.com/whatwg/html/issues/5803 is resolved.
> Source/WebCore/dom/BroadcastChannelRegistry.cpp:35
> +static BroadcastChannelRegistry* globalRegistry;
Is this null-initialized? I can never remember.
> Source/WebCore/dom/BroadcastChannelRegistry.cpp:94
> + globalRegistry = new LocalBroadcastChannelRegistry;
When would this be called? WebKitLegacy?
> Source/WebCore/dom/DeviceOrientationAndMotionAccessController.h:32
> +#include "SecurityOriginData.h"
Aren't unified source files fun?
> Source/WebKit/NetworkProcess/NetworkBroadcastChannelRegistry.cpp:76
> + IPC::Connection::send(globalIdentifier.connectionIdentifier,
Messages::WebBroadcastChannelRegistry::PostMessageToRemote(globalIdentifier.cha
nnelIndentifierInProcess, message), 0);
If I'm reading this correctly, this makes a communication channel between pages
using ephemeral and non-ephemeral data stores. There's a lot of globals that
should probably be owned by something, and we should probably add a test
verifying that pages with different data stores cannot communicate with each
other.
> Source/WebKit/NetworkProcess/NetworkBroadcastChannelRegistry.h:64
> + using NameToChannelIdentifiersMap = HashMap<String,
Vector<GlobalBroadcastChannelIdentifier>>;
Why a Vector? Don't we iterate this or remove from it? Aren't they all
unique? Do we need order?
More information about the webkit-reviews
mailing list