Bug 119127

Summary: Make SuspendableTimer safer
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: WebCore Misc.Assignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, jeffrey+webkit, kangil.han
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch none

Description Alexey Proskuryakov 2013-07-25 22:18:04 PDT 
SuspendableTimer is fairly difficult to understand, or to use safely. It does not enforce the invariant that fired() cannot be called when the timer is suspended. And a debug-only m_suspended member variable that's used for assertions doesn't accurately track whether ActiveDOMObject is suspended.

Most ActiveDOMObjects are pretty sloppy about this, but timers are easiest to misuse, especially through DocumentEventQueue.
Comment 1 Alexey Proskuryakov 2013-07-25 23:33:47 PDT 
Created attachment 207510 [details]
proposed patch
Comment 2 WebKit Commit Bot 2013-07-25 23:36:43 PDT 
Attachment 207510 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/DocumentEventQueue.cpp', u'Source/WebCore/page/DOMTimer.cpp', u'Source/WebCore/page/DOMTimer.h', u'Source/WebCore/page/SuspendableTimer.cpp', u'Source/WebCore/page/SuspendableTimer.h']" exit_code: 1
Source/WebCore/page/SuspendableTimer.cpp:141:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Sam Weinig 2013-07-27 14:14:53 PDT 
Comment on attachment 207510 [details]
proposed patch

Nice. r=me
Comment 4 WebKit Commit Bot 2013-07-27 17:03:03 PDT 
Comment on attachment 207510 [details]
proposed patch

Clearing flags on attachment: 207510

Committed r153406: <http://trac.webkit.org/changeset/153406>
Comment 5 WebKit Commit Bot 2013-07-27 17:03:05 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 6 Alexey Proskuryakov 2013-07-31 11:23:34 PDT 
This caused bug 119345.