Changeset 153406 in webkit


Ignore:
Timestamp:
Jul 27, 2013 5:04:14 PM (11 years ago)
Author:
ap@apple.com
Message:

Make SuspendableTimer safer
https://bugs.webkit.org/show_bug.cgi?id=119127

Reviewed by Sam Weinig.

SuspendableTimer now enforces that it stays suspended until resumed (or until stopped
and started again). To ensure this, TimerBase is now a private base class, and parts of
its interface that clients use are reimplemented with suspend/resume in mind.

Derived classes are still allowed to override TimerBase virtual functions (notably
fired() and alignedFireTime()).

  • dom/DocumentEventQueue.cpp:

(WebCore::DocumentEventQueueTimer): Removed an extraneous WTF_MAKE_NONCOPYABLE,
TimerBase has it already.
(WebCore::DocumentEventQueueTimer::create): Use our normal create() pattern.
(WebCore::DocumentEventQueue::DocumentEventQueue): Made the constructor private, accordingly.
(WebCore::DocumentEventQueue::cancelEvent): Use SuspendableTimer::cancel(), which
is a new name to disambiguate TimerBase::stop() and ActiveDOMObject::stop().
(WebCore::DocumentEventQueue::close): Ditto.

  • page/DOMTimer.cpp:

(WebCore::DOMTimer::fired): Now that SuspendableTimer knows whether it's currently
suspended, assert that it's not.
(WebCore::DOMTimer::didStop): Separated ActiveDOMObject::stop() implementation from
additional cleanup, allowing for better SuspendableTimer encapsulation.

  • page/DOMTimer.h: Added FINAL and OVVERIDE specifiers as appropriate.
  • page/SuspendableTimer.h: Added FINAL (and OVERRIDE) qualifiers to ActiveDOMObject

methods. A derived class that wants to override current behavior is most likely not
a timer, and thus shouldn't be a derived class.
(WebCore::SuspendableTimer::isActive): SuspendableTimer with a next fire time is
active even if suspended, we shouldn't overwrite its saved data thinking that it's
inactive.
(WebCore::SuspendableTimer::isSuspended): Exposed to clients (m_suspended is no
longer debug only).

  • page/SuspendableTimer.cpp:

(WebCore::SuspendableTimer::SuspendableTimer): Updated for new variable names.
(WebCore::SuspendableTimer::stop): This is ActiveDOMObject::stop(), which is called
before final destruction. We don't track this state directly, but can approximate
with setting m_suspended, so even if someone tries to start the timer afterwards,
it won't fire.
(WebCore::SuspendableTimer::suspend): Updated for new names.
(WebCore::SuspendableTimer::resume): Ditto.
(WebCore::SuspendableTimer::didStop): No-op default implementation for client hook.
(WebCore::SuspendableTimer::cancel): Equivalent of TimerBase::stop(), which also
works when suspended.
(WebCore::SuspendableTimer::startRepeating): Replacement for TimerBase function with
the same name, which works correctly when suspended. We don't want to actually start
the timer in this case.
(WebCore::SuspendableTimer::startOneShot): Ditto.
(WebCore::SuspendableTimer::repeatInterval): Ditto.
(WebCore::SuspendableTimer::augmentFireInterval): Ditto.
(WebCore::SuspendableTimer::augmentRepeatInterval): Ditto.

Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r153401 r153406  
     12013-07-27  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Make SuspendableTimer safer
     4        https://bugs.webkit.org/show_bug.cgi?id=119127
     5
     6        Reviewed by Sam Weinig.
     7
     8        SuspendableTimer now enforces that it stays suspended until resumed (or until stopped
     9        and started again). To ensure this, TimerBase is now a private base class, and parts of
     10        its interface that clients use are reimplemented with suspend/resume in mind.
     11
     12        Derived classes are still allowed to override TimerBase virtual functions (notably
     13        fired() and alignedFireTime()).
     14
     15        * dom/DocumentEventQueue.cpp:
     16        (WebCore::DocumentEventQueueTimer): Removed an extraneous WTF_MAKE_NONCOPYABLE,
     17        TimerBase has it already.
     18        (WebCore::DocumentEventQueueTimer::create): Use our normal create() pattern.
     19        (WebCore::DocumentEventQueue::DocumentEventQueue): Made the constructor private, accordingly.
     20        (WebCore::DocumentEventQueue::cancelEvent): Use SuspendableTimer::cancel(), which
     21        is a new name to disambiguate TimerBase::stop() and ActiveDOMObject::stop().
     22        (WebCore::DocumentEventQueue::close): Ditto.
     23
     24        * page/DOMTimer.cpp:
     25        (WebCore::DOMTimer::fired): Now that SuspendableTimer knows whether it's currently
     26        suspended, assert that it's not.
     27        (WebCore::DOMTimer::didStop): Separated ActiveDOMObject::stop() implementation from
     28        additional cleanup, allowing for better SuspendableTimer encapsulation.
     29
     30        * page/DOMTimer.h: Added FINAL and OVVERIDE specifiers as appropriate.
     31
     32        * page/SuspendableTimer.h: Added FINAL (and OVERRIDE) qualifiers to ActiveDOMObject
     33        methods. A derived class that wants to override current behavior is most likely not
     34        a timer, and thus shouldn't be a derived class.
     35        (WebCore::SuspendableTimer::isActive): SuspendableTimer with a next fire time is
     36        active even if suspended, we shouldn't overwrite its saved data thinking that it's
     37        inactive.
     38        (WebCore::SuspendableTimer::isSuspended): Exposed to clients (m_suspended is no
     39        longer debug only).
     40
     41        * page/SuspendableTimer.cpp:
     42        (WebCore::SuspendableTimer::SuspendableTimer): Updated for new variable names.
     43        (WebCore::SuspendableTimer::stop): This is ActiveDOMObject::stop(), which is called
     44        before final destruction. We don't track this state directly, but can approximate
     45        with setting m_suspended, so even if someone tries to start the timer afterwards,
     46        it won't fire.
     47        (WebCore::SuspendableTimer::suspend): Updated for new names.
     48        (WebCore::SuspendableTimer::resume): Ditto.
     49        (WebCore::SuspendableTimer::didStop): No-op default implementation for client hook.
     50        (WebCore::SuspendableTimer::cancel): Equivalent of TimerBase::stop(), which also
     51        works when suspended.
     52        (WebCore::SuspendableTimer::startRepeating): Replacement for TimerBase function with
     53        the same name, which works correctly when suspended. We don't want to actually start
     54        the timer in this case.
     55        (WebCore::SuspendableTimer::startOneShot): Ditto.
     56        (WebCore::SuspendableTimer::repeatInterval): Ditto.
     57        (WebCore::SuspendableTimer::augmentFireInterval): Ditto.
     58        (WebCore::SuspendableTimer::augmentRepeatInterval): Ditto.
     59
    1602013-07-27  Sam Weinig  <sam@webkit.org>
    261
  • trunk/Source/WebCore/dom/DocumentEventQueue.cpp

    r150969 r153406  
    3838namespace WebCore {
    3939   
    40 class DocumentEventQueueTimer : public SuspendableTimer {
    41     WTF_MAKE_NONCOPYABLE(DocumentEventQueueTimer);
     40class DocumentEventQueueTimer FINAL : public SuspendableTimer {
    4241public:
     42    static PassOwnPtr<DocumentEventQueueTimer> create(DocumentEventQueue* eventQueue, ScriptExecutionContext* context)
     43    {
     44        return adoptPtr(new DocumentEventQueueTimer(eventQueue, context));
     45    }
     46
     47private:
    4348    DocumentEventQueueTimer(DocumentEventQueue* eventQueue, ScriptExecutionContext* context)
    4449        : SuspendableTimer(context)
    4550        , m_eventQueue(eventQueue) { }
    4651
    47 private:
    48     virtual void fired() { m_eventQueue->pendingEventTimerFired(); }
     52    virtual void fired() OVERRIDE
     53    {
     54        ASSERT(!isSuspended());
     55        m_eventQueue->pendingEventTimerFired();
     56    }
     57
    4958    DocumentEventQueue* m_eventQueue;
    5059};
     
    5665
    5766DocumentEventQueue::DocumentEventQueue(ScriptExecutionContext* context)
    58     : m_pendingEventTimer(adoptPtr(new DocumentEventQueueTimer(this, context)))
     67    : m_pendingEventTimer(DocumentEventQueueTimer::create(this, context))
    5968    , m_isClosed(false)
    6069{
     
    104113        m_queuedEvents.remove(it);
    105114    if (m_queuedEvents.isEmpty())
    106         m_pendingEventTimer->stop();
     115        m_pendingEventTimer->cancel();
    107116    return found;
    108117}
     
    111120{
    112121    m_isClosed = true;
    113     m_pendingEventTimer->stop();
     122    m_pendingEventTimer->cancel();
    114123    m_queuedEvents.clear();
    115124}
  • trunk/Source/WebCore/page/DOMTimer.cpp

    r152238 r153406  
    108108    ScriptExecutionContext* context = scriptExecutionContext();
    109109    timerNestingLevel = m_nestingLevel;
     110    ASSERT(!isSuspended());
    110111    ASSERT(!context->activeDOMObjectsAreSuspended());
    111112    UserGestureIndicator gestureIndicator(m_shouldForwardUserGesture ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
     
    151152}
    152153
    153 void DOMTimer::stop()
    154 {
    155     SuspendableTimer::stop();
     154void DOMTimer::didStop()
     155{
    156156    // Need to release JS objects potentially protected by ScheduledAction
    157157    // because they can form circular references back to the ScriptExecutionContext
  • trunk/Source/WebCore/page/DOMTimer.h

    r152238 r153406  
    3636    class ScheduledAction;
    3737
    38     class DOMTimer : public SuspendableTimer {
     38    class DOMTimer FINAL : public SuspendableTimer {
    3939    public:
    4040        virtual ~DOMTimer();
     
    4545
    4646        // ActiveDOMObject
    47         virtual void contextDestroyed();
    48         virtual void stop();
     47        virtual void contextDestroyed() OVERRIDE;
    4948
    5049        // Adjust to a change in the ScriptExecutionContext's minimum timer interval.
     
    5554    private:
    5655        DOMTimer(ScriptExecutionContext*, PassOwnPtr<ScheduledAction>, int interval, bool singleShot);
    57         virtual void fired();
     56        virtual void fired() OVERRIDE;
     57
     58        // SuspendableTimer
     59        virtual void didStop() OVERRIDE;
    5860
    5961        double intervalClampedToMinimum(int timeout, double minimumTimerInterval) const;
    6062
    6163        // Retuns timer fire time rounded to the next multiple of timer alignment interval.
    62         virtual double alignedFireTime(double) const;
     64        virtual double alignedFireTime(double) const OVERRIDE;
    6365
    6466        int m_timeoutId;
  • trunk/Source/WebCore/page/SuspendableTimer.cpp

    r146537 r153406  
    11/*
    2  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
     2 * Copyright (C) 2008, 2013 Apple Inc. All Rights Reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    3434SuspendableTimer::SuspendableTimer(ScriptExecutionContext* context)
    3535    : ActiveDOMObject(context)
    36     , m_nextFireInterval(0)
    37     , m_repeatInterval(0)
    38     , m_active(false)
    39 #if !ASSERT_DISABLED
    4036    , m_suspended(false)
    41 #endif
     37    , m_savedNextFireInterval(0)
     38    , m_savedRepeatInterval(0)
     39    , m_savedIsActive(false)
    4240{
    4341}
     
    5452void SuspendableTimer::stop()
    5553{
    56     TimerBase::stop();
     54    if (!m_suspended)
     55        TimerBase::stop();
     56
     57    // Make it less likely that SuspendableTimer is accidentally revived and fires after being stopped.
     58    // It is not allowed for an ActiveDOMObject to become active again after stop().
     59    m_suspended = true;
     60    m_savedIsActive = false;
     61
     62    didStop();
    5763}
    5864
    5965void SuspendableTimer::suspend(ReasonForSuspension)
    6066{
    61 #if !ASSERT_DISABLED
    6267    ASSERT(!m_suspended);
    6368    m_suspended = true;
    64 #endif
    65     m_active = isActive();
    66     if (m_active) {
    67         m_nextFireInterval = nextUnalignedFireInterval();
    68         m_repeatInterval = repeatInterval();
     69
     70    m_savedIsActive = TimerBase::isActive();
     71    if (m_savedIsActive) {
     72        m_savedNextFireInterval = nextUnalignedFireInterval();
     73        m_savedRepeatInterval = repeatInterval();
    6974        TimerBase::stop();
    7075    }
     
    7378void SuspendableTimer::resume()
    7479{
    75 #if !ASSERT_DISABLED
    7680    ASSERT(m_suspended);
    7781    m_suspended = false;
    78 #endif
    79     if (m_active)
    80         start(m_nextFireInterval, m_repeatInterval);
     82
     83    if (m_savedIsActive)
     84        start(m_savedNextFireInterval, m_savedRepeatInterval);
    8185}
    8286
     
    8690}
    8791
     92void SuspendableTimer::didStop()
     93{
     94}
     95
     96void SuspendableTimer::cancel()
     97{
     98    if (!m_suspended)
     99        TimerBase::stop();
     100    else
     101        m_suspended = false;
     102}
     103
     104void SuspendableTimer::startRepeating(double repeatInterval)
     105{
     106    if (!m_suspended)
     107        TimerBase::startRepeating(repeatInterval);
     108    else {
     109        m_savedIsActive = true;
     110        m_savedNextFireInterval = repeatInterval;
     111        m_savedRepeatInterval = repeatInterval;
     112    }
     113}
     114
     115void SuspendableTimer::startOneShot(double interval)
     116{
     117    if (!m_suspended)
     118        TimerBase::startOneShot(interval);
     119    else {
     120        m_savedIsActive = true;
     121        m_savedNextFireInterval = interval;
     122        m_savedRepeatInterval = 0;
     123    }
     124}
     125
     126double SuspendableTimer::repeatInterval() const
     127{
     128    if (!m_suspended)
     129        return TimerBase::repeatInterval();
     130    if (m_savedIsActive)
     131        return m_savedRepeatInterval;
     132    return 0;
     133}
     134
     135void SuspendableTimer::augmentFireInterval(double delta)
     136{
     137    if (!m_suspended)
     138        TimerBase::augmentFireInterval(delta);
     139    else if (m_savedIsActive) {
     140        m_savedNextFireInterval += delta;
     141    } else {
     142        m_savedIsActive = true;
     143        m_savedNextFireInterval = delta;
     144        m_savedRepeatInterval = 0;
     145    }
     146}
     147
     148void SuspendableTimer::augmentRepeatInterval(double delta)
     149{
     150    if (!m_suspended)
     151        TimerBase::augmentRepeatInterval(delta);
     152    else if (m_savedIsActive) {
     153        m_savedNextFireInterval += delta;
     154        m_savedRepeatInterval += delta;
     155    } else {
     156        m_savedIsActive = true;
     157        m_savedNextFireInterval = delta;
     158        m_savedRepeatInterval = delta;
     159    }
     160}
     161
    88162} // namespace WebCore
  • trunk/Source/WebCore/page/SuspendableTimer.h

    r77355 r153406  
    11/*
    2  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
     2 * Copyright (C) 2008, 2013 Apple Inc. All Rights Reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    3333namespace WebCore {
    3434
    35 class SuspendableTimer : public TimerBase, public ActiveDOMObject {
     35class SuspendableTimer : private TimerBase, public ActiveDOMObject {
    3636public:
    3737    explicit SuspendableTimer(ScriptExecutionContext*);
     
    3939
    4040    // ActiveDOMObject
    41     virtual bool hasPendingActivity() const;
    42     virtual void stop();
    43     virtual bool canSuspend() const;
    44     virtual void suspend(ReasonForSuspension);
    45     virtual void resume();
     41    virtual bool hasPendingActivity() const FINAL OVERRIDE;
     42    virtual void stop() FINAL OVERRIDE;
     43    virtual bool canSuspend() const FINAL OVERRIDE;
     44    virtual void suspend(ReasonForSuspension) FINAL OVERRIDE;
     45    virtual void resume() FINAL OVERRIDE;
     46
     47    // A hook for derived classes to perform cleanup.
     48    virtual void didStop();
     49
     50    // Part of TimerBase interface used by SuspendableTimer clients, modified to work when suspended.
     51    bool isActive() const { return TimerBase::isActive() || (m_suspended && m_savedIsActive); }
     52    bool isSuspended() const { return m_suspended; }
     53    void startRepeating(double repeatInterval);
     54    void startOneShot(double interval);
     55    double repeatInterval() const;
     56    void augmentFireInterval(double delta);
     57    void augmentRepeatInterval(double delta);
     58    using TimerBase::didChangeAlignmentInterval;
     59    using TimerBase::operator new;
     60    using TimerBase::operator delete;
     61
     62    void cancel(); // Equivalent to TimerBase::stop(), whose name conflicts with ActiveDOMObject::stop().
    4663
    4764private:
    4865    virtual void fired() = 0;
    4966
    50     double m_nextFireInterval;
    51     double m_repeatInterval;
    52     bool m_active;
    53 #if !ASSERT_DISABLED
    5467    bool m_suspended;
    55 #endif
     68
     69    double m_savedNextFireInterval;
     70    double m_savedRepeatInterval;
     71    bool m_savedIsActive;
    5672};
    5773
Note: See TracChangeset for help on using the changeset viewer.