Bug 119127 - Make SuspendableTimer safer
Summary: Make SuspendableTimer safer
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
Depends on:
Reported: 2013-07-25 22:18 PDT by Alexey Proskuryakov
Modified: 2013-07-31 11:23 PDT (History)
4 users (show)

See Also:

proposed patch (14.35 KB, patch)
2013-07-25 23:33 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.