Bug 229814 – Implement origin partitioning (top-origin/frame-origin) for BroadcastChannel

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Implement origin partitioning (top-origin/frame-origin) for BroadcastChannel to address privacy concerns, as discussed here:
- https://github.com/whatwg/html/issues/5803

[Chris Dumez]

Created attachment 437163 [details]
Patch

[Chris Dumez]

Created attachment 437164 [details]
Patch

[Chris Dumez]

Created attachment 437166 [details]
Patch

[Sam Weinig]

Comment on attachment 437166 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437166&action=review

> Tools/TestRunnerShared/TestFeatures.cpp:128
> +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL)
> +{
> +    return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/");
> +}

We'd like to avoid adding more of these path based filters (the goal was to remove them all eventually).

Can we just disable it by default for all tests instead and use header comments to enable it where needed?

[Chris Dumez]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 437166 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437166&action=review
> 
> > Tools/TestRunnerShared/TestFeatures.cpp:128
> > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL)
> > +{
> > +    return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/");
> > +}
> 
> We'd like to avoid adding more of these path based filters (the goal was to
> remove them all eventually).
> 
> Can we just disable it by default for all tests instead and use header
> comments to enable it where needed?

We could. But since it would be enabled in the shipping configuration, ideally, I'd want it to be the default in our tests I think.

[Chris Dumez]

Created attachment 437189 [details]
Patch

[Chris Dumez]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 437166 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437166&action=review
> 
> > Tools/TestRunnerShared/TestFeatures.cpp:128
> > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL)
> > +{
> > +    return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/");
> > +}
> 
> We'd like to avoid adding more of these path based filters (the goal was to
> remove them all eventually).
> 
> Can we just disable it by default for all tests instead and use header
> comments to enable it where needed?

I made this change.

[Chris Dumez]

Created attachment 437220 [details]
Patch

[Sam Weinig]

(In reply to Chris Dumez from comment #5)
> (In reply to Sam Weinig from comment #4)
> > Comment on attachment 437166 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=437166&action=review
> > 
> > > Tools/TestRunnerShared/TestFeatures.cpp:128
> > > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL)
> > > +{
> > > +    return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/");
> > > +}
> > 
> > We'd like to avoid adding more of these path based filters (the goal was to
> > remove them all eventually).
> > 
> > Can we just disable it by default for all tests instead and use header
> > comments to enable it where needed?
> 
> We could. But since it would be enabled in the shipping configuration,
> ideally, I'd want it to be the default in our tests I think.

The opposite would also be fine (enable by default, disable explicitly where needed).

[Chris Dumez]

(In reply to Sam Weinig from comment #9)
> (In reply to Chris Dumez from comment #5)
> > (In reply to Sam Weinig from comment #4)
> > > Comment on attachment 437166 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=437166&action=review
> > > 
> > > > Tools/TestRunnerShared/TestFeatures.cpp:128
> > > > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL)
> > > > +{
> > > > +    return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/");
> > > > +}
> > > 
> > > We'd like to avoid adding more of these path based filters (the goal was to
> > > remove them all eventually).
> > > 
> > > Can we just disable it by default for all tests instead and use header
> > > comments to enable it where needed?
> > 
> > We could. But since it would be enabled in the shipping configuration,
> > ideally, I'd want it to be the default in our tests I think.
> 
> The opposite would also be fine (enable by default, disable explicitly where
> needed).

Put I cannot add header comments to disable the feature in WPT tests, which is why I was doing it some other way.

[EWS]

Committed r282105 (241403@main): <https://commits.webkit.org/241403@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437220 [details].

[Radar WebKit Bug Importer]

<rdar://problem/82834757>

[Chris Dumez]

Follow-up fix in <https://commits.webkit.org/r282106>.

[Chris Dumez]

Follow-up fix in <https://commits.webkit.org/r282246>.