937152 - XPCWrappedJS::mMainThread and mMainThreadOnly are always true

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Andrew McCreight [:mccr8]]

Thanks to the work to make XPCWrappedJS only used on the main thread, mMainThread and mMainThreadOnly are always true.  We can take advantage of that to simplify some code.  I've been touching this code recently, and having this old gunk around is annoying.

Comment 1

[Bobby Holley (:bholley)]

I wrote some patches last year to remove all the threading goop, but got blocked on the XPCWrappedJS Stuff. I'm going to dupe this to the older bug - if you want to write those patches, by all means please do so. I promise speedy reviews. :-)

Comment 2

[Bobby Holley (:bholley)]

(unless you had something of more limited scope in mind, in which case feel free to re-open this and mark it as a dep of the other bug)

Comment 3

[Andrew McCreight [:mccr8]]

Yes, this is just a limited thing.  Better to incrementally improve the state than to wait on some uber patch. ;)

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: remove the flags and simplify some code

Comment 5

[Andrew McCreight [:mccr8]]

Comment on attachment 830225 [details] [diff] [review]
remove the flags and simplify some code

Well, I should actually test this first...

The constructor for XPCWJS does an AddRef, so it must always be constructed on the main thread, so these flags are (I think) always going to be true.  I guess technically something where root is null could have it be false.

I also replaced a few instances of NS_IsMainThread() with true in functions where we check and crash at the start of the function.

Comment 6

[Andrew McCreight [:mccr8]]

mMainThreadOnly may not evaluate to true, but under the assumption that the code I am removing is only executed on the main thread (which is enforced by the CRASH guards), then it will end up behaving the same either way.  I should still test this...

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 830225 [details] [diff] [review]
remove the flags and simplify some code

This can at least run mochitests for 10 minutes, so it is probably okay.

Comment 8

[Andrew McCreight [:mccr8]]

l64 debug try run was green: https://tbpl.mozilla.org/?tree=Try&rev=8ab149fedc09

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cb70c98cbb6

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5cb70c98cbb6

Comment 10

[Bobby Holley (:bholley)]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/a4ce87454575

Comment 11

[Bobby Holley (:bholley)]

https://hg.mozilla.org/releases/mozilla-esr24/rev/7682a673c015

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1aa44c9f128d