Closed Bug 868110 Opened 7 years ago Closed 7 years ago

Remove JS_GetGlobalObject

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(3 files)

This function tends to be a total footgun, and people generally want JS_GetGlobalForScopeChain. There may still end up being a few consumers of this, but we should rename the resulting function to JS_GetDefaultGlobal or somesuch.
Yes, please.  IIUC, this takes us closer to bug 604813 and the bright and shiny future of JSContext-per-JSRuntime (which would then allow us to merge the two into one thing), right?
Blocks: 604813
(In reply to Luke Wagner [:luke] from comment #1)
> Yes, please.  IIUC, this takes us closer to bug 604813 and the bright and
> shiny future of JSContext-per-JSRuntime (which would then allow us to merge
> the two into one thing), right?

Yes, though as discussed on IRC, there will still be some consumers of that stuff for a while.
Here's the first batch of work on this:
https://tbpl.mozilla.org/?tree=Try&rev=b0d3e16ed3f3
Depends on: 868633
Depends on: 868634
Depends on: 868635
Depends on: 868637
Depends on: 869800
Depends on: 871301
Depends on: 871303
Depends on: 871306
Depends on: 873698
This one is easy to infer, because we subsequently call JS_CallFunctionValue,
which asserts that cx, obj, and fval are all same-compartment. So assuming
this code doesn't compartment mismatch right now, this should be equivalent.
Attachment #751789 - Flags: review?(luke)
The primary consumer of this is the whole inner/outer DOM window setup, which
uses the default global to track the current inner. But there are few other
random ones as well.

We use this as an opportunity to convert a bunch of consumers from the two-step
GetNativeContext() -> JS_GetGlobalObject() into just |GetNativeGlobal()|. This
will make things much easier to convert when we start tracking the current inner
explicitly.
Attachment #751790 - Flags: review?(luke)
Attachment #751791 - Flags: review?(luke)
Attachment #751789 - Flags: review?(luke) → review+
Attachment #751790 - Flags: review?(luke) → review+
Attachment #751791 - Flags: review?(luke) → review+
(In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #13)
> remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bc3d298479a5

There's a bunch of cx -> mContext changes that weren't in the reviewed patch.
(In reply to :Ms2ger from comment #15)
> (In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #13)
> > remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bc3d298479a5
> 
> There's a bunch of cx -> mContext changes that weren't in the reviewed patch.

Hm, not sure how that happened - probably happened while dealing with merge conflicts with bug 868130.

Anyway, those should all be ok - good catch though!
You need to log in before you can comment on or make changes to this bug.