Source/WebCore/ChangeLog

 12014-08-25 Gavin Barraclough <barraclough@apple.com>
 2
 3 DOMTimer may be deleted during timer fire
 4 https://bugs.webkit.org/show_bug.cgi?id=136198
 5
 6 Reviewed by NOBODY (OOPS!).
 7
 8 Consequentially ScheduledActions may also be deleted mid execution.
 9 This is fairly surprising & fragile, let's make this simpler.
 10
 11 Currently DOMTimer instances are effectively owned by the ScriptExecutionContext.
 12 There is a 1-1 mapping between timers and contexts, all timers are help in a map
 13 on the context and if the context goes away all timers are deleted. Rather than
 14 being implemented in a straightforward fashion (a smart pointer type for the map
 15 value) this is currently implemented by having the timer objects listen for the
 16 context going away using contextDestroyed, and deleting themselves if so.
 17
 18 Switch to using a smart pointer for values of m_timeouts in ScriptExecutionContext.
 19 By using a RefCounted object we can also extend the lifetime of the DOMTimer instance
 20 from within the DOMTimer::fired method, so the object is not destroyed while the
 21 member function is still on the stack.
 22
 23 * WebCore.xcodeproj/project.pbxproj:
 24 - project -> private since DOMTimer could no longer be a forward declare in ScriptExecutionContext.h.
 25 * dom/ScriptExecutionContext.cpp:
 26 (WebCore::ScriptExecutionContext::adjustMinimumTimerInterval):
 27 (WebCore::ScriptExecutionContext::didChangeTimerAlignmentInterval):
 28 - auto* -> auto& since value type in map is no longer a raw pointer.
 29 * dom/ScriptExecutionContext.h:
 30 (WebCore::ScriptExecutionContext::addTimeout):
 31 - DOMTimer* -> PassRefPtr<DOMTimer>
 32 * page/DOMTimer.cpp:
 33 (WebCore::DOMTimer::DOMTimer):
 34 - adopt the initial ref, and pass to addTimeout.
 35 (WebCore::DOMTimer::install):
 36 - updated comment.
 37 (WebCore::DOMTimer::removeById):
 38 - instead of explicitly deleting the timer and assuming this will implicitly remove it from the context map,
 39 we explicitly remove it from the context map and assume this will implicitly deleting the timer!
 40 (WebCore::DOMTimer::fired):
 41 - Add a RefPtr to keep the DOMTimer object alive until the fired method completes; to cancel a one-shot timer
 42 just remove it from the context's map, rather than explicitly deleting it.
 43 (WebCore::DOMTimer::~DOMTimer): Deleted.
 44 (WebCore::DOMTimer::contextDestroyed): Deleted.
 45 - no need! object lifetime management now handled by smart pointers.
 46 * page/DOMTimer.h:
 47 - added parent class RefCounted<DOMTimer>.
 48
1492014-08-25 Zalan Bujtas <zalan@apple.com>
250
351 Subpixel layout: remove roundedLayoutPoint/roundedLayoutSize functions.
172950

Source/WebCore/WebCore.xcodeproj/project.pbxproj

647647 185BCF280F3279CE000EA262 /* ThreadTimers.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 185BCF260F3279CE000EA262 /* ThreadTimers.cpp */; };
648648 185BCF290F3279CE000EA262 /* ThreadTimers.h in Headers */ = {isa = PBXBuildFile; fileRef = 185BCF270F3279CE000EA262 /* ThreadTimers.h */; };
649649 188604B30F2E654A000B6443 /* DOMTimer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 188604B10F2E654A000B6443 /* DOMTimer.cpp */; };
650  188604B40F2E654A000B6443 /* DOMTimer.h in Headers */ = {isa = PBXBuildFile; fileRef = 188604B20F2E654A000B6443 /* DOMTimer.h */; };
 650 188604B40F2E654A000B6443 /* DOMTimer.h in Headers */ = {isa = PBXBuildFile; fileRef = 188604B20F2E654A000B6443 /* DOMTimer.h */; settings = {ATTRIBUTES = (Private, ); }; };
651651 18F831B80FD48C7800D8C56B /* WorkerLoaderProxy.h in Headers */ = {isa = PBXBuildFile; fileRef = 18F831B70FD48C7800D8C56B /* WorkerLoaderProxy.h */; };
652652 1921327411C0E6BB00456238 /* SVGFEConvolveMatrixElement.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1921327111C0E6BB00456238 /* SVGFEConvolveMatrixElement.cpp */; };
653653 1921327511C0E6BB00456238 /* SVGFEConvolveMatrixElement.h in Headers */ = {isa = PBXBuildFile; fileRef = 1921327211C0E6BB00456238 /* SVGFEConvolveMatrixElement.h */; };

21252125 5DB1BC6B10715A6400EFAA49 /* TransformSourceLibxslt.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5DB1BC6910715A6400EFAA49 /* TransformSourceLibxslt.cpp */; };
21262126 5DF7F5C20F01F92A00526B4B /* CSSPropertyNames.h in Copy Generated Headers */ = {isa = PBXBuildFile; fileRef = 656580EF09D12B20000E61D7 /* CSSPropertyNames.h */; };
21272127 5DFE8F560D16477B0076E937 /* ScheduledAction.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BCA378BA0D15F64200B793D6 /* ScheduledAction.cpp */; };
2128  5DFE8F570D16477C0076E937 /* ScheduledAction.h in Headers */ = {isa = PBXBuildFile; fileRef = BCA378BB0D15F64200B793D6 /* ScheduledAction.h */; };
 2128 5DFE8F570D16477C0076E937 /* ScheduledAction.h in Headers */ = {isa = PBXBuildFile; fileRef = BCA378BB0D15F64200B793D6 /* ScheduledAction.h */; settings = {ATTRIBUTES = (Private, ); }; };
21292129 5DFEBAB718592B6D00C75BEB /* WebKitAvailability.h in Headers */ = {isa = PBXBuildFile; fileRef = 5DFEBAB618592B6D00C75BEB /* WebKitAvailability.h */; settings = {ATTRIBUTES = (Private, ); }; };
21302130 5F2DBBE9178E3C8100141486 /* CertificateInfoMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5F2DBBE7178E332D00141486 /* CertificateInfoMac.mm */; };
21312131 5FA904CA178E61F5004C8A2D /* CertificateInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 5F2DBBE8178E336900141486 /* CertificateInfo.h */; settings = {ATTRIBUTES = (Private, ); }; };

21362136 628D214C12131ED10055DCFC /* NetworkingContext.h in Headers */ = {isa = PBXBuildFile; fileRef = 628D214B12131ED10055DCFC /* NetworkingContext.h */; settings = {ATTRIBUTES = (Private, ); }; };
21372137 628D214E12131EF40055DCFC /* FrameNetworkingContext.h in Headers */ = {isa = PBXBuildFile; fileRef = 628D214D12131EF40055DCFC /* FrameNetworkingContext.h */; settings = {ATTRIBUTES = (Private, ); }; };
21382138 62C1217C11AB9E77003C462C /* SuspendableTimer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 62C1217A11AB9E76003C462C /* SuspendableTimer.cpp */; };
2139  62C1217D11AB9E77003C462C /* SuspendableTimer.h in Headers */ = {isa = PBXBuildFile; fileRef = 62C1217B11AB9E77003C462C /* SuspendableTimer.h */; };
 2139 62C1217D11AB9E77003C462C /* SuspendableTimer.h in Headers */ = {isa = PBXBuildFile; fileRef = 62C1217B11AB9E77003C462C /* SuspendableTimer.h */; settings = {ATTRIBUTES = (Private, ); }; };
21402140 62CD32591157E57C0063B0A7 /* CustomEvent.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 62CD32561157E57C0063B0A7 /* CustomEvent.cpp */; };
21412141 62CD325A1157E57C0063B0A7 /* CustomEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 62CD32571157E57C0063B0A7 /* CustomEvent.h */; };
21422142 63189AE30E83A33300012E41 /* NodeRareData.h in Headers */ = {isa = PBXBuildFile; fileRef = 63189AE20E83A33300012E41 /* NodeRareData.h */; settings = {ATTRIBUTES = (); }; };
172600

Source/WebCore/dom/ScriptExecutionContext.cpp

@@PublicURLManager& ScriptExecutionContext
416416void ScriptExecutionContext::adjustMinimumTimerInterval(double oldMinimumTimerInterval)
417417{
418418 if (minimumTimerInterval() != oldMinimumTimerInterval) {
419  for (auto* timer : m_timeouts.values())
 419 for (auto& timer : m_timeouts.values())
420420 timer->adjustMinimumTimerInterval(oldMinimumTimerInterval);
421421 }
422422}

@@double ScriptExecutionContext::minimumTi
433433
434434void ScriptExecutionContext::didChangeTimerAlignmentInterval()
435435{
436  for (auto* timer : m_timeouts.values())
 436 for (auto& timer : m_timeouts.values())
437437 timer->didChangeAlignmentInterval();
438438}
439439
172600

Source/WebCore/dom/ScriptExecutionContext.h

2929#define ScriptExecutionContext_h
3030
3131#include "ActiveDOMObject.h"
 32#include "DOMTimer.h"
 33#include "ScheduledAction.h"
3234#include "SecurityContext.h"
3335#include "Supplementable.h"
3436#include <runtime/ConsoleTypes.h>

@@namespace WebCore {
4749
4850class CachedScript;
4951class DatabaseContext;
50 class DOMTimer;
5152class EventQueue;
5253class EventTarget;
5354class MessagePort;

@@public:
150151 // Gets the next id in a circular sequence from 1 to 2^31-1.
151152 int circularSequentialID();
152153
153  bool addTimeout(int timeoutId, DOMTimer* timer) { return m_timeouts.add(timeoutId, timer).isNewEntry; }
 154 bool addTimeout(int timeoutId, PassRefPtr<DOMTimer> timer) { return m_timeouts.add(timeoutId, timer).isNewEntry; }
154155 void removeTimeout(int timeoutId) { m_timeouts.remove(timeoutId); }
155156 DOMTimer* findTimeout(int timeoutId) { return m_timeouts.get(timeoutId); }
156157

@@private:
205206 HashSet<ActiveDOMObject*> m_activeDOMObjects;
206207
207208 int m_circularSequentialID;
208  HashMap<int, DOMTimer*> m_timeouts;
 209 HashMap<int, RefPtr<DOMTimer>> m_timeouts;
209210
210211 bool m_inDispatchErrorEvent;
211212 class PendingException;
172600

Source/WebCore/page/DOMTimer.cpp

@@DOMTimer::DOMTimer(ScriptExecutionContex
6565 , m_originalInterval(interval)
6666 , m_shouldForwardUserGesture(shouldForwardUserGesture(interval, m_nestingLevel))
6767{
 68 RefPtr<DOMTimer> reference = adoptRef(this);
 69
6870 // Keep asking for the next id until we're given one that we don't already have.
6971 do {
7072 m_timeoutId = context->circularSequentialID();
71  } while (!context->addTimeout(m_timeoutId, this));
 73 } while (!context->addTimeout(m_timeoutId, reference));
7274
7375 double intervalMilliseconds = intervalClampedToMinimum(interval, context->minimumTimerInterval());
7476 if (singleShot)

@@DOMTimer::DOMTimer(ScriptExecutionContex
7779 startRepeating(intervalMilliseconds);
7880}
7981
80 DOMTimer::~DOMTimer()
81 {
82  if (scriptExecutionContext())
83  scriptExecutionContext()->removeTimeout(m_timeoutId);
84 }
85 
8682int DOMTimer::install(ScriptExecutionContext* context, std::unique_ptr<ScheduledAction> action, int timeout, bool singleShot)
8783{
88  // DOMTimer constructor links the new timer into a list of ActiveDOMObjects held by the 'context'.
89  // The timer is deleted when context is deleted (DOMTimer::contextDestroyed) or explicitly via DOMTimer::removeById(),
90  // or if it is a one-time timer and it has fired (DOMTimer::fired).
 84 // DOMTimer constructor passes ownership of the initial ref on the object to the constructor.
 85 // This reference will be released automatically when a one-shot timer fires, when the context
 86 // is destroyed, or if explicitly cancelled by removeById.
9187 DOMTimer* timer = new DOMTimer(context, WTF::move(action), timeout, singleShot);
9288#if PLATFORM(IOS)
9389 if (context->isDocument()) {

@@void DOMTimer::removeById(ScriptExecutio
115111 return;
116112
117113 InspectorInstrumentation::didRemoveTimer(context, timeoutId);
118 
119  delete context->findTimeout(timeoutId);
 114 context->removeTimeout(timeoutId);
120115}
121116
122117void DOMTimer::fired()
123118{
 119 // Retain this - if the timer is cancelled while this function is on the stack (implicitly and always
 120 // for one-shot timers, or if removeById is called on itself from within an interval timer fire) then
 121 // wait unit the end of this function to delete DOMTimer.
 122 RefPtr<DOMTimer> reference = this;
 123
124124 ScriptExecutionContext* context = scriptExecutionContext();
125125 ASSERT(context);
126126#if PLATFORM(IOS)

@@void DOMTimer::fired()
148148 augmentRepeatInterval(minimumInterval - repeatInterval());
149149 }
150150
151  // No access to member variables after this point, it can delete the timer.
152151 m_action->execute(context);
153152
154153 InspectorInstrumentation::didFireTimer(cookie);

@@void DOMTimer::fired()
156155 return;
157156 }
158157
159  // Delete timer before executing the action for one-shot timers.
160  std::unique_ptr<ScheduledAction> action = WTF::move(m_action);
161 
162  // No access to member variables after this point.
163  delete this;
 158 context->removeTimeout(m_timeoutId);
164159
165160#if PLATFORM(IOS)
166161 bool shouldReportLackOfChanges;

@@void DOMTimer::fired()
179174 }
180175#endif
181176
182  action->execute(context);
 177 m_action->execute(context);
183178
184179#if PLATFORM(IOS)
185180 if (shouldBeginObservingChanges) {

@@void DOMTimer::fired()
196191 timerNestingLevel = 0;
197192}
198193
199 void DOMTimer::contextDestroyed()
200 {
201  SuspendableTimer::contextDestroyed();
202  delete this;
203 }
204 
205194void DOMTimer::didStop()
206195{
207196 // Need to release JS objects potentially protected by ScheduledAction
172600

Source/WebCore/page/DOMTimer.h

2828#define DOMTimer_h
2929
3030#include "SuspendableTimer.h"
 31#include <WTF/RefCounted.h>
3132#include <memory>
3233
3334namespace WebCore {
3435
3536 class ScheduledAction;
3637
37  class DOMTimer final : public SuspendableTimer {
 38 class DOMTimer final : public RefCounted<DOMTimer>, public SuspendableTimer {
 39 WTF_MAKE_NONCOPYABLE(DOMTimer);
 40 WTF_MAKE_FAST_ALLOCATED;
3841 public:
39  virtual ~DOMTimer();
4042 // Creates a new timer owned by specified ScriptExecutionContext, starts it
4143 // and returns its Id.
4244 static int install(ScriptExecutionContext*, std::unique_ptr<ScheduledAction>, int timeout, bool singleShot);

@@namespace WebCore {
4951
5052 private:
5153 DOMTimer(ScriptExecutionContext*, std::unique_ptr<ScheduledAction>, int interval, bool singleShot);
52  virtual void fired() override;
53 
54  // ActiveDOMObject
55  virtual void contextDestroyed() override;
 54 double intervalClampedToMinimum(int timeout, double minimumTimerInterval) const;
5655
5756 // SuspendableTimer
 57 virtual void fired() override;
5858 virtual void didStop() override;
59 
60  double intervalClampedToMinimum(int timeout, double minimumTimerInterval) const;
61 
62  // Retuns timer fire time rounded to the next multiple of timer alignment interval.
6359 virtual double alignedFireTime(double) const override;
6460
6561 int m_timeoutId;
172600