adoptRef DOMTimer in install instead of its constructor
https://bugs.webkit.org/show_bug.cgi?id=203016
Reviewed by Simon Fraser.
Moved the code to add DOMTimer to ScriptExecutionContext's map to DOMTimer::install
instead of its constructor so that we can adoptRef there instead for clarity & simplicity.
* dom/ScriptExecutionContext.h:
(WebCore::ScriptExecutionContext::addTimeout):
* page/DOMTimer.cpp:
(WebCore::DOMTimer::DOMTimer):
(WebCore::DOMTimer::install):
(WebCore::DOMTimer::fired):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251177 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0376b04..3751a10 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2019-10-15 Ryosuke Niwa <rniwa@webkit.org>
+
+ adoptRef DOMTimer in install instead of its constructor
+ https://bugs.webkit.org/show_bug.cgi?id=203016
+
+ Reviewed by Simon Fraser.
+
+ Moved the code to add DOMTimer to ScriptExecutionContext's map to DOMTimer::install
+ instead of its constructor so that we can adoptRef there instead for clarity & simplicity.
+
+ * dom/ScriptExecutionContext.h:
+ (WebCore::ScriptExecutionContext::addTimeout):
+ * page/DOMTimer.cpp:
+ (WebCore::DOMTimer::DOMTimer):
+ (WebCore::DOMTimer::install):
+ (WebCore::DOMTimer::fired):
+
2019-10-15 Simon Fraser <simon.fraser@apple.com>
ScrollingTreeScrollingNodeDelegateMac::stretchAmount() should not have side effects
diff --git a/Source/WebCore/dom/ScriptExecutionContext.h b/Source/WebCore/dom/ScriptExecutionContext.h
index 397e18c..ea4b57e 100644
--- a/Source/WebCore/dom/ScriptExecutionContext.h
+++ b/Source/WebCore/dom/ScriptExecutionContext.h
@@ -202,7 +202,7 @@
// Gets the next id in a circular sequence from 1 to 2^31-1.
int circularSequentialID();
- bool addTimeout(int timeoutId, DOMTimer& timer) { return m_timeouts.add(timeoutId, &timer).isNewEntry; }
+ bool addTimeout(int timeoutId, DOMTimer& timer) { return m_timeouts.add(timeoutId, timer).isNewEntry; }
void removeTimeout(int timeoutId) { m_timeouts.remove(timeoutId); }
DOMTimer* findTimeout(int timeoutId) { return m_timeouts.get(timeoutId); }
@@ -307,7 +307,7 @@
HashSet<ContextDestructionObserver*> m_destructionObservers;
HashSet<ActiveDOMObject*> m_activeDOMObjects;
- HashMap<int, RefPtr<DOMTimer>> m_timeouts;
+ HashMap<int, Ref<DOMTimer>> m_timeouts;
struct PendingException;
std::unique_ptr<Vector<std::unique_ptr<PendingException>>> m_pendingExceptions;
diff --git a/Source/WebCore/page/DOMTimer.cpp b/Source/WebCore/page/DOMTimer.cpp
index 8e6271c..b213a99 100644
--- a/Source/WebCore/page/DOMTimer.cpp
+++ b/Source/WebCore/page/DOMTimer.cpp
@@ -169,13 +169,6 @@
, m_currentTimerInterval(intervalClampedToMinimum())
, m_userGestureTokenToForward(UserGestureIndicator::currentUserGesture())
{
- RefPtr<DOMTimer> reference = adoptRef(this);
-
- // Keep asking for the next id until we're given one that we don't already have.
- do {
- m_timeoutId = context.circularSequentialID();
- } while (!context.addTimeout(m_timeoutId, *this));
-
if (singleShot)
startOneShot(m_currentTimerInterval);
else
@@ -186,22 +179,25 @@
int DOMTimer::install(ScriptExecutionContext& context, std::unique_ptr<ScheduledAction> action, Seconds timeout, bool singleShot)
{
- // DOMTimer constructor passes ownership of the initial ref on the object to the constructor.
- // This reference will be released automatically when a one-shot timer fires, when the context
- // is destroyed, or if explicitly cancelled by removeById.
- DOMTimer* timer = new DOMTimer(context, WTFMove(action), timeout, singleShot);
+ Ref<DOMTimer> timer = adoptRef(*new DOMTimer(context, WTFMove(action), timeout, singleShot));
timer->suspendIfNeeded();
+
+ // Keep asking for the next id until we're given one that we don't already have.
+ do {
+ timer->m_timeoutId = context.circularSequentialID();
+ } while (!context.addTimeout(timer->m_timeoutId, timer.get()));
+
InspectorInstrumentation::didInstallTimer(context, timer->m_timeoutId, timeout, singleShot);
// Keep track of nested timer installs.
if (NestedTimersMap* nestedTimers = NestedTimersMap::instanceForContext(context))
- nestedTimers->add(timer->m_timeoutId, *timer);
+ nestedTimers->add(timer->m_timeoutId, timer.get());
#if PLATFORM(IOS_FAMILY)
if (is<Document>(context)) {
auto& document = downcast<Document>(context);
- document.contentChangeObserver().didInstallDOMTimer(*timer, timeout, singleShot);
+ document.contentChangeObserver().didInstallDOMTimer(timer.get(), timeout, singleShot);
if (DeferDOMTimersForScope::isDeferring())
- document.domTimerHoldingTank().add(*timer);
+ document.domTimerHoldingTank().add(timer.get());
}
#endif
return timer->m_timeoutId;
@@ -287,7 +283,7 @@
// Retain this - if the timer is cancelled while this function is on the stack (implicitly and always
// for one-shot timers, or if removeById is called on itself from within an interval timer fire) then
// wait unit the end of this function to delete DOMTimer.
- RefPtr<DOMTimer> reference = this;
+ Ref<DOMTimer> protectedThis(*this);
ASSERT(scriptExecutionContext());
ScriptExecutionContext& context = *scriptExecutionContext();
