Closed Bug 1275999 Opened 4 years ago Closed 4 years ago

Stop saving frame chains in xpconnect

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(5 files)

I believe nothing relies on this anymore once bug 1274915 lands.
Oh, I guess technically JS_IsRunning pays attention to this, but I think that's a bug.
Blocks: 1255874
This can all go away because no one is paying attention to the saved frame chain state anymore.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
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?
Attachment #8756989 - Flags: review+
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.
Attachment #8756990 - Flags: review+
Attachment #8756991 - Flags: review+
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.
Attachment #8756992 - Flags: review+
Attachment #8756993 - Flags: review+
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 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
> Remove comment

No, the comment was for the next class, not the helper class I removed.
Blocks: 1276276
You need to log in before you can comment on or make changes to this bug.