Integrate the chromium message loop into nsThread

RESOLVED FIXED in mozilla29

Status

()

Core
XPCOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Ben Turner (not reading bugmail, use the needinfo flag!), Assigned: Ben Turner (not reading bugmail, use the needinfo flag!))

Tracking

Trunk
mozilla29
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

We need the ability to process chromium IPC messages on regular nsThreads for workers. The attached patch integrates the chromium message loop into nsThread.
Created attachment 833004 [details] [diff] [review]
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.
Attachment #833004 - Flags: review?(benjamin)
Created attachment 833009 [details] [diff] [review]
Integrate message loop, v1
Attachment #833009 - Flags: review?(benjamin)
Created attachment 833012 [details] [diff] [review]
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.
Attachment #833012 - Flags: review?(benjamin)
Blocks: 914762

Comment 4

4 years ago
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
Attachment #833004 - Flags: review?(benjamin) → review+

Updated

4 years ago
Attachment #833009 - Flags: review?(benjamin) → review+

Updated

4 years ago
Attachment #833012 - Flags: review?(benjamin) → review+
Created attachment 8344474 [details] [diff] [review]
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?
Attachment #8344474 - Flags: review?(benjamin)
Attachment #8344474 - Attachment description: Fix timer replacements → Fix timer replacements, v1

Comment 6

4 years ago
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.
Attachment #8344474 - Flags: review?(benjamin) → review-
Created attachment 8344805 [details] [diff] [review]
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?
Attachment #8344474 - Attachment is obsolete: true
Attachment #8344805 - Flags: review?(benjamin)

Comment 8

4 years ago
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.
Attachment #8344805 - Flags: review?(benjamin) → review+
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
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
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.