Summary: | Implement SharedTimer on WorkerRunLoop | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dmitry Titov <dimich> | ||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, darin | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | 23488 | ||||||||||
Bug Blocks: | 22718 | ||||||||||
Attachments: |
|
Description
Dmitry Titov
2009-01-26 17:09:50 PST
Created attachment 27060 [details]
Proposed patch
+ class WorkerRunLoop : public SharedTimer { Is it really right to say that a run loop is an instance of SharedTimer? This results in awkward calls like runLoop.stop() that are difficult to interpret as being timer-related. (In reply to comment #2) > + class WorkerRunLoop : public SharedTimer { > > Is it really right to say that a run loop is an instance of SharedTimer? This > results in awkward calls like runLoop.stop() that are difficult to interpret as > being timer-related. In case it's not obvious, other options are to use private inheritance, or to have a shared timer as a data member. Thanks guys. I think having something like a ThreadSharedTimer as a member of WorkerRunLoop will remove those methods from the run loop itself. Updated patch is coming... Created attachment 27085 [details]
Updated patch
Removed SharedTimer implementation into a private class. WorkerRunLoop has an instance of it.
Comment on attachment 27085 [details] Updated patch > - task->performTask(context); > + if (result == MessageQueueTerminated) > + break; > + else if (result == MessageQueueMessageReceived) > + task->performTask(context); > + else > + m_sharedTimer->fire(); You don't need an "else" after the "break". Created attachment 27135 [details]
Updated patch
Thanks Mark!
Updated the patch.
bug 23488 was resolved, this bug is not blocked anymore. Same patch. Comment on attachment 27135 [details] Updated patch > + {} We usually put braces on separate lines in this context. > + bool isActive() { return m_sharedTimerFunction && m_nextFireTime != 0; } No need for "!= 0" here. r=me Committed revision 40532. + * dom/WorkerRunLoop.h: + Add member of type OwnPtr<WorkerSharedTimer> A great comment says why a change was made, not what the change was. But I know that it's difficult to find something to say about trivial changes. And committed JavaScriptCore parts in revision 40533. |