Integrate the chromium message loop into nsThread

RESOLVED FIXED in mozilla29

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Tracking

Trunk
mozilla29
Points:
---
Dependency tree / graph

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.
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)
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)
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+
Attachment #833009 - Flags: review?(benjamin) → review+
Attachment #833012 - Flags: review?(benjamin) → review+
Posted patch Fix timer replacements, v1 (obsolete) — Splinter Review
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 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-
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 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+
You need to log in before you can comment on or make changes to this bug.