594455 - [meta] DefaultCompartment fixes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: patch

Asserting at the API boundary that we don't enter with the defaultCompartment.

Comment 2

[Jason Orendorff [:jorendorff]]

Gregor, I think the assertion in JS_IsAboutToBeCollected is bogus. Please rip that out and later today I'll remove any JSAutoCrossCompartmentCalls we've added in GC callbacks.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Don't use JSAutoCompartment to appease bogus assertion in JS_IsAboutToBeFinalized

Hope everyone agrees this isn't a productive thing to be doing.

Comment 4

[Gregor Wagner [:gwagner]]

Attachment: Asserts in API functions

refresh of api asserts:
JS_ASSERT(cx->compartment != cx->runtime->defaultCompartment);

Comment 5

[Gregor Wagner [:gwagner]]

I pass now all the xpcshell tests on my machine (Mac) with all the various defaultCompartment fixes. 
Tryserver for windows and linux indicates another mixup in 
TEST-INFO | e:\builds\moz2_slave\tryserver-win32-debug-unittest-xpcshell\build\xpcshell\tests\chrome\test\unit_ipc\test_resolve_uris_ipc.js |

It looks like the last one :)

Comment 6

[Andreas Gal :gal]

file it, I can fix

Comment 7

[Gregor Wagner [:gwagner]]

I pushed the API assertion patch to tryserver and I don't see assertions any more.

Looks like we fixed all defaultCompartment mixups that are covered by the testcases!

Comment 8

[Gregor Wagner [:gwagner]]

Attachment: patch

Maybe we should land some assertions in jsapi for the future.

Comment 9

[Jason Orendorff [:jorendorff]]

Comment on attachment 479972 [details] [diff] [review]
patch

The original idea was that non-JS_THREADSAFE embeddings could ignore compartments entirely and use the default compartment for everything. That way we only break JS_THREADSAFE embedders.

If you think that would work, please make the asserts JS_THREADSAFE-only.  (I think all JS_THREADSAFE embeddings will want to avoid the default compartment for the same reasons we do.)

r=me with that.

Comment 10

[Gregor Wagner [:gwagner]]

Attachment: patch

Something like this with JS_THREADSAFE_ASSERT?

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/1283831c28bf