Make sure BroadcastChannel are separated by userContextId

RESOLVED FIXED in Firefox 49

Status

()

Core
DOM: Security
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jhao, Assigned: jhao)

Tracking

(Blocks: 1 bug)

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [userContextId][OA][domsecurity-active])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
I tried to write a test for bug 1260907, but later I realized my test actually tests BroadcastChannel's postMessage, instead of window-to-window postMessage. Therefore, I open this bug for that.
(Assignee)

Updated

2 years ago
Whiteboard: [userContextId][OA]
(Assignee)

Comment 1

2 years ago
Created attachment 8746991 [details] [diff] [review]
Make sure BroadcastChannels are separated by userContextId

Hi baku, may I ask you to review this patch? Thanks.
Attachment #8746991 - Flags: review?(amarchesini)
(Assignee)

Comment 2

2 years ago
Created attachment 8746992 [details] [diff] [review]
Make sure BroadcastChannels are separated by userContextId

Removed trailing spaces.
Attachment #8746992 - Flags: review?(amarchesini)
(Assignee)

Updated

2 years ago
Attachment #8746991 - Attachment is obsolete: true
Attachment #8746991 - Flags: review?(amarchesini)
Whiteboard: [userContextId][OA] → [userContextId][OA][domsecurity-active]
Comment on attachment 8746992 [details] [diff] [review]
Make sure BroadcastChannels are separated by userContextId

Review of attachment 8746992 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/contextualidentity/test/browser/browser_postMessage.js
@@ +21,5 @@
> +
> +add_task(function* setup() {
> +  // make sure userContext is enabled.
> +  SpecialPowers.pushPrefEnv({"set": [
> +    ["privacy.userContext.enabled", true]

There is a better way to do it. What about this:

add_task(function* setup() {
  yield new Promise((resolve) => {
    SpecialPowers.pushPrefEnv({"set": [
      ["privacy.userContext.enabled", true],
    ]}, resolve);
  });
});

Maybe file a bug to fix all the existing contextual identity browser tests.
If we do that, we can remove cleanup().

@@ +28,5 @@
> +
> +add_task(function* cleanup() {
> +  // make sure we don't leave any prefs set for the next tests
> +  registerCleanupFunction(function() {
> +    SpecialPowers.popPrefEnv();

remove this one. We don't need it if we use pushPrefEnv.
Attachment #8746992 - Flags: review?(amarchesini) → review+
(Assignee)

Comment 4

2 years ago
Created attachment 8748040 [details] [diff] [review]
Make sure BroadcastChannels are separated by userContextId

Thanks, baku.
I addressed your comments. I also added some comments in the test, and change the filename from browser_postMessage.js to browser_broadcastchannel.js to avoid confusion.
(Assignee)

Updated

2 years ago
Attachment #8746992 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8748040 - Flags: review+
(Assignee)

Comment 5

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f54e7472baa2
(Assignee)

Updated

2 years ago
See Also: → bug 1269660
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 6

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/afc86294a605
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/afc86294a605
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49

Updated

2 years ago
Blocks: 1191418
Duplicate of this bug: 1270683
You need to log in before you can comment on or make changes to this bug.