Commit a2e319e6 authored by ap@apple.com's avatar ap@apple.com

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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153406 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 636fb120
2013-07-27 Alexey Proskuryakov <ap@apple.com>
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.
2013-07-27 Sam Weinig <sam@webkit.org>
Add assertions for CSSPrimitiveValue's m_value.valueID accessor
......@@ -37,15 +37,24 @@
namespace WebCore {
class DocumentEventQueueTimer : public SuspendableTimer {
WTF_MAKE_NONCOPYABLE(DocumentEventQueueTimer);
class DocumentEventQueueTimer FINAL : public SuspendableTimer {
public:
static PassOwnPtr<DocumentEventQueueTimer> create(DocumentEventQueue* eventQueue, ScriptExecutionContext* context)
{
return adoptPtr(new DocumentEventQueueTimer(eventQueue, context));
}
private:
DocumentEventQueueTimer(DocumentEventQueue* eventQueue, ScriptExecutionContext* context)
: SuspendableTimer(context)
, m_eventQueue(eventQueue) { }
private:
virtual void fired() { m_eventQueue->pendingEventTimerFired(); }
virtual void fired() OVERRIDE
{
ASSERT(!isSuspended());
m_eventQueue->pendingEventTimerFired();
}
DocumentEventQueue* m_eventQueue;
};
......@@ -55,7 +64,7 @@ PassRefPtr<DocumentEventQueue> DocumentEventQueue::create(ScriptExecutionContext
}
DocumentEventQueue::DocumentEventQueue(ScriptExecutionContext* context)
: m_pendingEventTimer(adoptPtr(new DocumentEventQueueTimer(this, context)))
: m_pendingEventTimer(DocumentEventQueueTimer::create(this, context))
, m_isClosed(false)
{
m_pendingEventTimer->suspendIfNeeded();
......@@ -103,14 +112,14 @@ bool DocumentEventQueue::cancelEvent(Event* event)
if (found)
m_queuedEvents.remove(it);
if (m_queuedEvents.isEmpty())
m_pendingEventTimer->stop();
m_pendingEventTimer->cancel();
return found;
}
void DocumentEventQueue::close()
{
m_isClosed = true;
m_pendingEventTimer->stop();
m_pendingEventTimer->cancel();
m_queuedEvents.clear();
}
......
......@@ -107,6 +107,7 @@ void DOMTimer::fired()
{
ScriptExecutionContext* context = scriptExecutionContext();
timerNestingLevel = m_nestingLevel;
ASSERT(!isSuspended());
ASSERT(!context->activeDOMObjectsAreSuspended());
UserGestureIndicator gestureIndicator(m_shouldForwardUserGesture ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
// Only the first execution of a multi-shot timer should get an affirmative user gesture indicator.
......@@ -150,9 +151,8 @@ void DOMTimer::contextDestroyed()
delete this;
}
void DOMTimer::stop()
void DOMTimer::didStop()
{
SuspendableTimer::stop();
// Need to release JS objects potentially protected by ScheduledAction
// because they can form circular references back to the ScriptExecutionContext
// which will cause a memory leak.
......
......@@ -35,7 +35,7 @@ namespace WebCore {
class ScheduledAction;
class DOMTimer : public SuspendableTimer {
class DOMTimer FINAL : public SuspendableTimer {
public:
virtual ~DOMTimer();
// Creates a new timer owned by specified ScriptExecutionContext, starts it
......@@ -44,8 +44,7 @@ namespace WebCore {
static void removeById(ScriptExecutionContext*, int timeoutId);
// ActiveDOMObject
virtual void contextDestroyed();
virtual void stop();
virtual void contextDestroyed() OVERRIDE;
// Adjust to a change in the ScriptExecutionContext's minimum timer interval.
// This allows the minimum allowable interval time to be changed in response
......@@ -54,12 +53,15 @@ namespace WebCore {
private:
DOMTimer(ScriptExecutionContext*, PassOwnPtr<ScheduledAction>, int interval, bool singleShot);
virtual void fired();
virtual void fired() OVERRIDE;
// SuspendableTimer
virtual void didStop() OVERRIDE;
double intervalClampedToMinimum(int timeout, double minimumTimerInterval) const;
// Retuns timer fire time rounded to the next multiple of timer alignment interval.
virtual double alignedFireTime(double) const;
virtual double alignedFireTime(double) const OVERRIDE;
int m_timeoutId;
int m_nestingLevel;
......
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
* Copyright (C) 2008, 2013 Apple Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
......@@ -33,12 +33,10 @@ namespace WebCore {
SuspendableTimer::SuspendableTimer(ScriptExecutionContext* context)
: ActiveDOMObject(context)
, m_nextFireInterval(0)
, m_repeatInterval(0)
, m_active(false)
#if !ASSERT_DISABLED
, m_suspended(false)
#endif
, m_savedNextFireInterval(0)
, m_savedRepeatInterval(0)
, m_savedIsActive(false)
{
}
......@@ -53,31 +51,37 @@ bool SuspendableTimer::hasPendingActivity() const
void SuspendableTimer::stop()
{
TimerBase::stop();
if (!m_suspended)
TimerBase::stop();
// Make it less likely that SuspendableTimer is accidentally revived and fires after being stopped.
// It is not allowed for an ActiveDOMObject to become active again after stop().
m_suspended = true;
m_savedIsActive = false;
didStop();
}
void SuspendableTimer::suspend(ReasonForSuspension)
{
#if !ASSERT_DISABLED
ASSERT(!m_suspended);
m_suspended = true;
#endif
m_active = isActive();
if (m_active) {
m_nextFireInterval = nextUnalignedFireInterval();
m_repeatInterval = repeatInterval();
m_savedIsActive = TimerBase::isActive();
if (m_savedIsActive) {
m_savedNextFireInterval = nextUnalignedFireInterval();
m_savedRepeatInterval = repeatInterval();
TimerBase::stop();
}
}
void SuspendableTimer::resume()
{
#if !ASSERT_DISABLED
ASSERT(m_suspended);
m_suspended = false;
#endif
if (m_active)
start(m_nextFireInterval, m_repeatInterval);
if (m_savedIsActive)
start(m_savedNextFireInterval, m_savedRepeatInterval);
}
bool SuspendableTimer::canSuspend() const
......@@ -85,4 +89,74 @@ bool SuspendableTimer::canSuspend() const
return true;
}
void SuspendableTimer::didStop()
{
}
void SuspendableTimer::cancel()
{
if (!m_suspended)
TimerBase::stop();
else
m_suspended = false;
}
void SuspendableTimer::startRepeating(double repeatInterval)
{
if (!m_suspended)
TimerBase::startRepeating(repeatInterval);
else {
m_savedIsActive = true;
m_savedNextFireInterval = repeatInterval;
m_savedRepeatInterval = repeatInterval;
}
}
void SuspendableTimer::startOneShot(double interval)
{
if (!m_suspended)
TimerBase::startOneShot(interval);
else {
m_savedIsActive = true;
m_savedNextFireInterval = interval;
m_savedRepeatInterval = 0;
}
}
double SuspendableTimer::repeatInterval() const
{
if (!m_suspended)
return TimerBase::repeatInterval();
if (m_savedIsActive)
return m_savedRepeatInterval;
return 0;
}
void SuspendableTimer::augmentFireInterval(double delta)
{
if (!m_suspended)
TimerBase::augmentFireInterval(delta);
else if (m_savedIsActive) {
m_savedNextFireInterval += delta;
} else {
m_savedIsActive = true;
m_savedNextFireInterval = delta;
m_savedRepeatInterval = 0;
}
}
void SuspendableTimer::augmentRepeatInterval(double delta)
{
if (!m_suspended)
TimerBase::augmentRepeatInterval(delta);
else if (m_savedIsActive) {
m_savedNextFireInterval += delta;
m_savedRepeatInterval += delta;
} else {
m_savedIsActive = true;
m_savedNextFireInterval = delta;
m_savedRepeatInterval = delta;
}
}
} // namespace WebCore
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
* Copyright (C) 2008, 2013 Apple Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
......@@ -32,27 +32,43 @@
namespace WebCore {
class SuspendableTimer : public TimerBase, public ActiveDOMObject {
class SuspendableTimer : private TimerBase, public ActiveDOMObject {
public:
explicit SuspendableTimer(ScriptExecutionContext*);
virtual ~SuspendableTimer();
// ActiveDOMObject
virtual bool hasPendingActivity() const;
virtual void stop();
virtual bool canSuspend() const;
virtual void suspend(ReasonForSuspension);
virtual void resume();
virtual bool hasPendingActivity() const FINAL OVERRIDE;
virtual void stop() FINAL OVERRIDE;
virtual bool canSuspend() const FINAL OVERRIDE;
virtual void suspend(ReasonForSuspension) FINAL OVERRIDE;
virtual void resume() FINAL OVERRIDE;
// A hook for derived classes to perform cleanup.
virtual void didStop();
// Part of TimerBase interface used by SuspendableTimer clients, modified to work when suspended.
bool isActive() const { return TimerBase::isActive() || (m_suspended && m_savedIsActive); }
bool isSuspended() const { return m_suspended; }
void startRepeating(double repeatInterval);
void startOneShot(double interval);
double repeatInterval() const;
void augmentFireInterval(double delta);
void augmentRepeatInterval(double delta);
using TimerBase::didChangeAlignmentInterval;
using TimerBase::operator new;
using TimerBase::operator delete;
void cancel(); // Equivalent to TimerBase::stop(), whose name conflicts with ActiveDOMObject::stop().
private:
virtual void fired() = 0;
double m_nextFireInterval;
double m_repeatInterval;
bool m_active;
#if !ASSERT_DISABLED
bool m_suspended;
#endif
double m_savedNextFireInterval;
double m_savedRepeatInterval;
bool m_savedIsActive;
};
} // namespace WebCore
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment