Last Comment Bug 763856 - Need way to leave a JSAutoEnterCompartment without waiting for the destructor to be called
: Need way to leave a JSAutoEnterCompartment without waiting for the destructor...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-12 02:07 PDT by Bobby Holley (:bholley) (busy with Stylo)
Modified: 2012-06-13 13:29 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Introduce JSAutoEnterCompartment::leave(). v1 (1.82 KB, patch)
2012-06-12 02:08 PDT, Bobby Holley (:bholley) (busy with Stylo)
luke: review+
Details | Diff | Splinter Review
Part 1 - Clarify compartment semantics for ExposedPropertiesOnly. v1 (7.15 KB, patch)
2012-06-12 04:55 PDT, Bobby Holley (:bholley) (busy with Stylo)
no flags Details | Diff | Splinter Review
Part 2 - Clarify compartment situation in Xray wrapper. v1 (3.26 KB, patch)
2012-06-12 04:56 PDT, Bobby Holley (:bholley) (busy with Stylo)
no flags Details | Diff | Splinter Review
Part 1 - Clarify compartment semantics for ExposedPropertiesOnly. v2 (7.24 KB, patch)
2012-06-12 05:00 PDT, Bobby Holley (:bholley) (busy with Stylo)
no flags Details | Diff | Splinter Review

Description Bobby Holley (:bholley) (busy with Stylo) 2012-06-12 02:07:11 PDT
This has been annoying me. In general, it's great to use scoping. But sometimes, it unnecessarily complicates the code. For example, sometimes we want to leave the compartment before doing a tail function call. Currently, we have to unnecessarily brace/indent a lot of things to make this happen.

Patch coming up.
Comment 1 Bobby Holley (:bholley) (busy with Stylo) 2012-06-12 02:08:00 PDT
Created attachment 632171 [details] [diff] [review]
Introduce JSAutoEnterCompartment::leave(). v1

Attaching a patch, flagging luke for review.
Comment 2 Bobby Holley (:bholley) (busy with Stylo) 2012-06-12 04:55:22 PDT
Created attachment 632209 [details] [diff] [review]
Part 1 - Clarify compartment semantics for ExposedPropertiesOnly. v1
Comment 3 Bobby Holley (:bholley) (busy with Stylo) 2012-06-12 04:56:04 PDT
Created attachment 632210 [details] [diff] [review]
Part 2 - Clarify compartment situation in Xray wrapper. v1

Adding to the mess of the NodePrincipal (et al) check isn't great, but I'm refactoring that in bug 761704.
Comment 4 Bobby Holley (:bholley) (busy with Stylo) 2012-06-12 05:00:32 PDT
Created attachment 632211 [details] [diff] [review]
Part 1 - Clarify compartment semantics for ExposedPropertiesOnly. v2

Added an updated comment to clarify why two JSAutoEnterCompartments are necessary.
Comment 5 Bobby Holley (:bholley) (busy with Stylo) 2012-06-12 05:01:14 PDT
Gah! Wrong bug number for those other patches. Sorry blake. :-(
Comment 6 Bobby Holley (:bholley) (busy with Stylo) 2012-06-12 10:07:22 PDT
One thing I realized about this patch is that it allows the consumer to pooch the compartment situation by leaving compartments out of order. I suspect it's not worth worrying about, but maybe we should add some asserts somehow?
Comment 7 Bobby Holley (:bholley) (busy with Stylo) 2012-06-13 02:45:20 PDT
Pushed to m-i:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f195d6de172f
Comment 8 Matt Brubeck (:mbrubeck) 2012-06-13 13:29:40 PDT
https://hg.mozilla.org/mozilla-central/rev/f195d6de172f

Note You need to log in before you can comment on or make changes to this bug.