1461292 - Rename *AutoCompartment to *AutoRealm

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

It would be nice to rename JSAutoCompartment to JS::AutoRealm, but we also have our internal js::AutoCompartment and merging these is not trivial. So I'll just rename to JSAutoRealm and js::AutoRealm.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Rename JSAutoCompartment to JSAutoRealm

The hardest part here is what to do with some of the comments mentioning "compartment". I changed the obvious ones to "realm", but I didn't touch the ones I was less confident about.

Boris, I expect you will have to audit these at some point anyway... Another option is to keep JSAutoCompartment as a typedef and try to limit my changes to js/src for now and leave the rest to you, but that might be more confusing to people.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Rename JSAutoNullableCompartment to JSAutoNullableRealm

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 3 - Rename AutoCompartment (and friends) to AutoRealm

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8975448 [details] [diff] [review]
Part 2 - Rename JSAutoNullableCompartment to JSAutoNullableRealm

r=me

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8975441 [details] [diff] [review]
Part 1 - Rename JSAutoCompartment to JSAutoRealm

> CustomElementRegistry::Define

There's all sorts of weirdness here that should just go away; I'll do that on top of your patch.  Filed bug 1461711 to track that.

>+++ b/dom/base/nsFrameMessageManager.cpp

The comments here about leaking until shutdown should probably s/compartment/global/ or s/compartment/realm/.

>+++ b/dom/base/nsObjectLoadingContent.cpp
>+  // NB: We need a JSAutoRealm because we can be called from nsPluginFrame

We can't be called from nsPluginFrame, and afaict the only caller is nsObjectLoadingContent::SetupProtoChain which already has cx in the obj realm.  So I think we can remove this comment and this JSAutoRealm.

>+++ b/dom/bindings/BindingUtils.cpp
>+  JSAutoRealm newAc(aCx, newParent);

newAr?

>+++ b/dom/bindings/CallbackObject.h
>+    // pop the JSContext. Though in practice we'll often manually order

There is no more JSContext stack.  This should probably say:

  // pop the script settings stack. Though in practice ....

>+++ b/dom/bindings/Codegen.py
>@@ -16679,27 +16679,27 @@ class CGMaplikeOrSetlikeHelperFunctionGe
>+            JSAutoRealm tempCompartment(cx, UnprivilegedJunkScopeOrWorkerGlobal());

tempRealm?

>+            JSAutoRealm reflectorCompartment(cx, obj);

reflectorRealm.

>+++ b/js/xpconnect/src/xpcprivate.h
>+    mozilla::Maybe<JSAutoRealm> mAutoCompartment;

mAutoRealm?

>+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
>     // The expando lives in the target's compartment, so do our installation there.

"target's realm"?

r=me on "the bits outside js/" + js/xpconnect

Comment 6

[Jan de Mooij [:jandem]]

Thanks for the reviews!

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #5)
> We can't be called from nsPluginFrame, and afaict the only caller is
> nsObjectLoadingContent::SetupProtoChain which already has cx in the obj
> realm.  So I think we can remove this comment and this JSAutoRealm.

I'll post a follow-up patch for this one to minimize risk.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b6365bdb4c2
part 1 - Rename JSAutoCompartment to JSAutoRealm. r=bz,luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/1096b51e73ed
part 2 - Rename JSAutoNullableCompartment to JSAutoNullableRealm. r=bz

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Part 4 - Remove JSAutoRealm from GetPluginJSObject

I also removed the now-unused |obj| argument.

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/6b6365bdb4c2
https://hg.mozilla.org/mozilla-central/rev/1096b51e73ed

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf1b00c73d57
part 3 - Rename AutoCompartment to AutoRealm. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/f050b09ee268
part 4 - Remove unnecessary JSAutoRealm from GetPluginJSObject. r=bz

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/cf1b00c73d57
https://hg.mozilla.org/mozilla-central/rev/f050b09ee268