1275999 - Stop saving frame chains in xpconnect

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I believe nothing relies on this anymore once bug 1274915 lands.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Oh, I guess technically JS_IsRunning pays attention to this, but I think that's a bug.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Stop saving frame chains in XPCJSContextStack::Push

This can all go away because no one is paying attention to the saved frame chain state anymore.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Change XPCJSContextStack to just store JSContext*

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Remove the totally unused XPCJSContextStack::HasJSContext function

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Change XPCJSContextStack::Pop to return void

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Get rid of GetDefaultScopeFromJSContext

Comment 7

[Bobby Holley (:bholley)]

Comment on attachment 8756989 [details] [diff] [review]
part 1.  Stop saving frame chains in XPCJSContextStack::Push

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

::: js/xpconnect/src/XPCJSContextStack.cpp
@@ -44,5 @@
>  
>      --idx; // Advance to new top of the stack
>  
>      XPCJSContextInfo& e = mStack[idx];
> -    if (e.cx && e.savedFrameChain) {

Can you remove this boolean?

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 8756990 [details] [diff] [review]
part 2.  Change XPCJSContextStack to just store JSContext*

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

Oh I see. ignore that comment.

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 8756992 [details] [diff] [review]
part 4.  Change XPCJSContextStack::Pop to return void

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

Commit message should be push, not pop.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Try found a bug in part 2, yay Werror=unused-but-set-variable.  js::Debug_SetActiveJSContext(mRuntime->Runtime(), nullptr) should be js::Debug_SetActiveJSContext(mRuntime->Runtime(), newTop) instead.  Fixed locally.

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8756990 [details] [diff] [review]
part 2.  Change XPCJSContextStack to just store JSContext*

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

::: js/xpconnect/src/xpcprivate.h
@@ +2772,5 @@
>  };
>  
>  
>  /***************************************************************************/
>  // XPCJSContextStack is not actually an xpcom object, but xpcom calls are

Remove comment

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c17c414e300
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed57ef99f5e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d9e42ddcd59
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cab033c0e0b
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce020eaf0e1e

Comment 13

[Boris Zbarsky [:bzbarsky]]

> Remove comment

No, the comment was for the next class, not the helper class I removed.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8c17c414e300
https://hg.mozilla.org/mozilla-central/rev/ed57ef99f5e1
https://hg.mozilla.org/mozilla-central/rev/1d9e42ddcd59
https://hg.mozilla.org/mozilla-central/rev/8cab033c0e0b
https://hg.mozilla.org/mozilla-central/rev/ce020eaf0e1e