986107 - Make new sandboxes inherit the metadata of the creating sandbox by default

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Dave Townsend [:mossop]]

Bug 898559 added the notion of metadata to tag a sandbox for the add-on debugger but it requires add-ons to set it correctly when creating custom sandboxes. This patch makes the sandbox use the same metadata as the sandbox that is creating it by default.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 8395005 [details] [diff] [review]
patch rev 1

Failry straightforward I think but I don't know my way around xpconnect so I mostly cribbed from other places. This just tries to get the global from the JS context and then get its metadata and use that in the event that there was no metadata passed in.

Comment 3

[Bobby Holley (:bholley)]

Comment on attachment 8395005 [details] [diff] [review]
patch rev 1

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

::: js/xpconnect/src/Sandbox.cpp
@@ +1574,5 @@
>      if (NS_FAILED(AssembleSandboxMemoryReporterName(cx, options.sandboxName)))
>          return ThrowAndFail(NS_ERROR_INVALID_ARG, cx, _retval);
>  
> +    if (options.metadata.isNullOrUndefined())
> +        xpc::GetContextMetadata(cx, &options.metadata);

This should just be:

if (options.metadata.isNullOrUndefined()) {
  // If the caller is running in a sandbox, inherit.
  RootedObject callerGlobal(cx, CurrentGlobalOrNull(cx));
  if (IsSandbox(callerGlobal)) {
    rv = GetSandboxMetadata(cx, callerGlobal, &options.metadata);
    if (NS_WARN_IF(NS_FAILED(rv))
      return rv;
  }
}

Or somesuch.

@@ +1834,5 @@
> +
> +    targetObject = JS_GetGlobalForObject(cx, targetObject);
> +
> +    RootedObject sandbox(cx, targetObject);
> +    return xpc::GetSandboxMetadata(cx, sandbox, rval);

How do we know that the global here is a sandbox at all? GetSandboxMetadata isn't safe to call on non-sandbox globals.

::: js/xpconnect/tests/unit/test_sandbox_metadata.js
@@ +43,5 @@
>    do_check_eq(thrown, true);
> +
> +  let systemPrincipal = Components.classes['@mozilla.org/systemprincipal;1']
> +                                  .createInstance(Components.interfaces.nsIPrincipal);
> +  sandbox = Components.utils.Sandbox(systemPrincipal, {

You can just pass |this| here, which is a system-principaled global (BackstagePass) instead of explicitly grabbing the system principal.

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Ah yes, that's much simpler

Comment 5

[Dave Townsend [:mossop]]

https://hg.mozilla.org/integration/fx-team/rev/099f98a960a9

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/099f98a960a9