939182 - Integrate the chromium message loop into nsThread

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

We need the ability to process chromium IPC messages on regular nsThreads for workers. The attached patch integrates the chromium message loop into nsThread.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Pre-patch cleanup, v1

This is not the main patch, just some cleanup that I did before. It basically adds MOZ_OVERRIDE and MOZ_ASSERT to stuff, and tightens up the inheritance rules.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Integrate message loop, v1

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Followup nsIThreadObserver change, v1

I needed to change nsIThreadObserver slightly so that LazyIdleThread can know if an event actually ran. This bug already existed but was triggered reliably with the message loop patch.

Comment 4

[Benjamin Smedberg]

Comment on attachment 833004 [details] [diff] [review]
Pre-patch cleanup, v1

What's up with the anonymous namespace? `static` is preferred when possible for debugging reasons, and it looks possible here.

r=me with that reverted or explained

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Fix timer replacements, v1

Hrm, I hate this... 

toolkit/components/places/tests/expiration/test_pref_interval.js (and presumably other things) mock the timer implementation. Since the mock is written in JS it crashes when being called off the main thread. I'm not sure that this is something we really should support, but the attached patch works around mocks by always using the builtin implementation for the delayed work timers.

What do you think?

Comment 6

[Benjamin Smedberg]

Comment on attachment 8344474 [details] [diff] [review]
Fix timer replacements, v1

I don't understand how TimerCID always uses the builtin implementation. Are you relying on ordering to ensure that TimerCID is always called before the mock takes over?

The normal/better way to use a specific implementation is to expose its CID directly and call doCreateInstance with it. I would prefer that here.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Fix timer replacements, v2

Well, I was thinking that some addon could replace our timer implementation with a legitimate C++ implementation, and that we should use whatever it is... But this works too. Do you prefer this?

Comment 8

[Benjamin Smedberg]

Comment on attachment 8344805 [details] [diff] [review]
Fix timer replacements, v2

Yes quite. Any addon attempting to replace timers with a c++ replacement would be insane, especially because we call directly into the timer C++ code from XPCOM startup/shutdown.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/b2g-inbound/rev/475f1db6b910
https://hg.mozilla.org/integration/b2g-inbound/rev/1d2659a098c2
https://hg.mozilla.org/integration/b2g-inbound/rev/fa74710fdeef
https://hg.mozilla.org/integration/b2g-inbound/rev/072161a22749

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/475f1db6b910
https://hg.mozilla.org/mozilla-central/rev/1d2659a098c2
https://hg.mozilla.org/mozilla-central/rev/fa74710fdeef
https://hg.mozilla.org/mozilla-central/rev/072161a22749