1276310 - Merge XPCContext into XPCJSRuntime

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I think once bug 1276276 is done, we can just do this.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Need to remember to ask Bobby whether there are any issues he sees with this, but he's not accepting needinfo right now.

Comment 2

[Bobby Holley (:bholley)]

Seems fine to me.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Move XPCContext's mPendingResult member into XPCJSRuntime

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Get rid of Components.lastResult and the corresponding XPCContext member

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Get rid of the mException member of XPCContext

The member is only set by SetException(), which is called with non-null in
precisely one place.  This place is always immediately followed by a call to
nsXPCWrappedJSClass::CheckForException which proceeds to grab the exception from
the XPCContext and call SetException(null).  So basically, it's all a weird
out-of-band argument to CheckForException.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Store an XPCJSRuntime* in XPCCallContext instead of storing an XPCContext*

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Remove the now-unused XPCContext

The XPCShellImpl change is needed because we're now unifying XPCShellImpl with
some file that does "using namespace xpc" at toplevel, so the "Atob" and "Btoa"
barewords in its JSFunctionSpecs become ambiguous.  Luckily, the file-static
versions of those in XPCShellImpl are completely identical to the xpc-namespaced
versions anyway.

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 8764104 [details] [diff] [review]
part 2.  Get rid of Components.lastResult and the corresponding XPCContext member

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

\o/

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 8764107 [details] [diff] [review]
part 4.  Store an XPCJSRuntime* in XPCCallContext instead of storing an XPCContext*

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

This is all a bit overblown at this point but I'm fine with leaving it for now.

Comment 10

[Bobby Holley (:bholley)]

Comment on attachment 8764108 [details] [diff] [review]
part 5.  Remove the now-unused XPCContext

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

::: js/xpconnect/src/XPCContext.cpp
@@ -18,5 @@
> -        mJSContext(aJSContext)
> -{
> -    MOZ_COUNT_CTOR(XPCContext);
> -
> -    MOZ_ASSERT(!JS_GetSecondContextPrivate(mJSContext), "Must be null");

Can you file a followup to remove the second context private from JSAPI?

Comment 11

[Jan de Mooij [:jandem]]

(In reply to Bobby Holley (busy) from comment #10)
> Can you file a followup to remove the second context private from JSAPI?

We can remove both context privates actually - I landed a patch that removes the context private from the JS shell. Removing both will help the JSContext/JSRuntime refactoring.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Filed bug 1281582 on removing JSContext privates.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa060d1404ad
part 1.  Move XPCContext's mPendingResult member into XPCJSRuntime.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3d3a8915a3a
part 2.  Get rid of Components.lastResult and the corresponding XPCContext member.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/a59ed6a08cb8
part 3.  Get rid of the mException member of XPCContext.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/15159f86c546
part 4.  Store an XPCJSRuntime* in XPCCallContext instead of storing an XPCContext*.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fbb064a7bcb
part 5.  Remove the now-unused XPCContext.  r=bholley

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fa060d1404ad
https://hg.mozilla.org/mozilla-central/rev/e3d3a8915a3a
https://hg.mozilla.org/mozilla-central/rev/a59ed6a08cb8
https://hg.mozilla.org/mozilla-central/rev/15159f86c546
https://hg.mozilla.org/mozilla-central/rev/5fbb064a7bcb