1092444 - [e10s] Add shim support for sandboxes with expanded principals

Status: RESOLVED FIXED

(Firefox :: Extension Compatibility, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: greasemonkey-ep-sandbox

Greasemonkey sometimes creates sandboxes with "expanded principals". See here:
https://developer.mozilla.org/en-US/docs/Components.utils.Sandbox#The_Expanded_Principal

We need to support these in the shims in order for certain greasemonkey scripts to work. This patch does that.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: sandbox-ep-test

Here is a test.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8515393 [details] [diff] [review]
greasemonkey-ep-sandbox

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

::: toolkit/components/addoncompat/RemoteAddonsParent.jsm
@@ +648,5 @@
> +      let chromeGlobal = chromeGlobalForContentWindow(principals);
> +      return SandboxParent.makeContentSandbox(chromeGlobal, principals, ...rest);
> +    } else if (principals &&
> +               typeof(principals) == "object" &&
> +               "every" in principals &&

Duck-typing gives me the willies. :/

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8515394 [details] [diff] [review]
sandbox-ep-test

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

::: toolkit/components/addoncompat/tests/addon/bootstrap.js
@@ +85,2 @@
>          loadWithRemoveCount++;
>          is(event.target.documentURI, url1, "only fire for first url");

How was this passing before? Was it?

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab201881acaf

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #3)
> Comment on attachment 8515394 [details] [diff] [review]
> sandbox-ep-test
> 
> Review of attachment 8515394 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/addoncompat/tests/addon/bootstrap.js
> @@ +85,2 @@
> >          loadWithRemoveCount++;
> >          is(event.target.documentURI, url1, "only fire for first url");
> 
> How was this passing before? Was it?

Well, it threw an exception. Since it was from within an event handler, it was ignored. I'm not sure how the test harness normally deals with that sort of thing.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a819db6a34a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ab201881acaf
https://hg.mozilla.org/mozilla-central/rev/9a819db6a34a