Title: [175830] trunk/Source/WebCore
- Revision
- 175830
- Author
- [email protected]
- Date
- 2014-11-10 14:05:52 -0800 (Mon, 10 Nov 2014)
Log Message
Support throttling of DOMTimers using nested setTimeout() calls
https://bugs.webkit.org/show_bug.cgi?id=138262
Reviewed by Gavin Barraclough.
Extend DOMTimers throttling support to timers that are using nested
setTimeout() calls instead of a setInterval(). To achieve this, this
patch introduces a NestedTimersMap singleton class where nested timers
are added, and for which we potentially update the next firing time,
after the parent timer is done executing.
I have verified this helps on ebay.com for example, which has timers
interacting with non-visible plugins that are scheduled using nested
setTimeout() calls with a frequency of 150 / 200 ms.
This is a second take on r175441, which caused intermittent crashes.
This time, nested timers are removed from the NestedTimersMap when
DOMTimer::removeById() is called. It would be unsafe to use the nested
timer afterwards because we don't hold a strong reference to it and
the ScriptExecutionContext is unref'ing the DOMTimer when
ScriptExecutionContext::removeTimeout() is called from
DOMTimer::removeById().
* page/DOMTimer.cpp:
(WebCore::NestedTimersMap::NestedTimersMap):
(WebCore::NestedTimersMap::~NestedTimersMap):
(WebCore::NestedTimersMap::add):
(WebCore::NestedTimersMap::remove):
(WebCore::NestedTimersMap::begin):
(WebCore::NestedTimersMap::end):
(WebCore::DOMTimer::install):
(WebCore::DOMTimer::removeById):
(WebCore::DOMTimer::fired):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (175829 => 175830)
--- trunk/Source/WebCore/ChangeLog 2014-11-10 22:01:59 UTC (rev 175829)
+++ trunk/Source/WebCore/ChangeLog 2014-11-10 22:05:52 UTC (rev 175830)
@@ -1,3 +1,39 @@
+2014-11-10 Chris Dumez <[email protected]>
+
+ Support throttling of DOMTimers using nested setTimeout() calls
+ https://bugs.webkit.org/show_bug.cgi?id=138262
+
+ Reviewed by Gavin Barraclough.
+
+ Extend DOMTimers throttling support to timers that are using nested
+ setTimeout() calls instead of a setInterval(). To achieve this, this
+ patch introduces a NestedTimersMap singleton class where nested timers
+ are added, and for which we potentially update the next firing time,
+ after the parent timer is done executing.
+
+ I have verified this helps on ebay.com for example, which has timers
+ interacting with non-visible plugins that are scheduled using nested
+ setTimeout() calls with a frequency of 150 / 200 ms.
+
+ This is a second take on r175441, which caused intermittent crashes.
+ This time, nested timers are removed from the NestedTimersMap when
+ DOMTimer::removeById() is called. It would be unsafe to use the nested
+ timer afterwards because we don't hold a strong reference to it and
+ the ScriptExecutionContext is unref'ing the DOMTimer when
+ ScriptExecutionContext::removeTimeout() is called from
+ DOMTimer::removeById().
+
+ * page/DOMTimer.cpp:
+ (WebCore::NestedTimersMap::NestedTimersMap):
+ (WebCore::NestedTimersMap::~NestedTimersMap):
+ (WebCore::NestedTimersMap::add):
+ (WebCore::NestedTimersMap::remove):
+ (WebCore::NestedTimersMap::begin):
+ (WebCore::NestedTimersMap::end):
+ (WebCore::DOMTimer::install):
+ (WebCore::DOMTimer::removeById):
+ (WebCore::DOMTimer::fired):
+
2014-11-10 Jer Noble <[email protected]>
[EME][Mac] Add a systemCode to distinguish when no expired sessions were found in response to a "keyrelease" message.
Modified: trunk/Source/WebCore/page/DOMTimer.cpp (175829 => 175830)
--- trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-10 22:01:59 UTC (rev 175829)
+++ trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-10 22:05:52 UTC (rev 175830)
@@ -36,6 +36,7 @@
#include <wtf/CurrentTime.h>
#include <wtf/HashSet.h>
#include <wtf/MathExtras.h>
+#include <wtf/NeverDestroyed.h>
#include <wtf/StdLibExtras.h>
#if PLATFORM(IOS)
@@ -85,6 +86,61 @@
DOMTimerFireState* DOMTimerFireState::current = nullptr;
+struct NestedTimersMap {
+ typedef HashMap<int, DOMTimer*>::const_iterator const_iterator;
+
+ static NestedTimersMap* instanceForContext(ScriptExecutionContext* context)
+ {
+ // For worker threads, we don't use NestedTimersMap as doing so would not
+ // be thread safe.
+ if (context->isDocument())
+ return &instance();
+ return nullptr;
+ }
+
+ void startTracking()
+ {
+ // Make sure we start with an empty HashMap. In theory, it is possible the HashMap is not
+ // empty if a timer fires during the execution of another timer (may happen with the
+ // in-process Web Inspector).
+ nestedTimers.clear();
+ isTrackingNestedTimers = true;
+ }
+
+ void stopTracking()
+ {
+ isTrackingNestedTimers = false;
+ nestedTimers.clear();
+ }
+
+ void add(int timeoutId, DOMTimer* timer)
+ {
+ if (isTrackingNestedTimers)
+ nestedTimers.add(timeoutId, timer);
+ }
+
+ void remove(int timeoutId)
+ {
+ if (isTrackingNestedTimers)
+ nestedTimers.remove(timeoutId);
+ }
+
+ const_iterator begin() const { return nestedTimers.begin(); }
+ const_iterator end() const { return nestedTimers.end(); }
+
+private:
+ static NestedTimersMap& instance()
+ {
+ static NeverDestroyed<NestedTimersMap> map;
+ return map;
+ }
+
+ static bool isTrackingNestedTimers;
+ HashMap<int /* timeoutId */, DOMTimer*> nestedTimers;
+};
+
+bool NestedTimersMap::isTrackingNestedTimers = false;
+
static inline bool shouldForwardUserGesture(int interval, int nestingLevel)
{
return UserGestureIndicator::processingUserGesture()
@@ -134,6 +190,10 @@
timer->suspendIfNeeded();
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);
+
return timer->m_timeoutId;
}
@@ -145,6 +205,9 @@
if (timeoutId <= 0)
return;
+ if (NestedTimersMap* nestedTimers = NestedTimersMap::instanceForContext(context))
+ nestedTimers->remove(timeoutId);
+
InspectorInstrumentation::didRemoveTimer(context, timeoutId);
context->removeTimeout(timeoutId);
}
@@ -238,6 +301,11 @@
}
#endif
+ // Keep track nested timer installs.
+ NestedTimersMap* nestedTimers = NestedTimersMap::instanceForContext(context);
+ if (nestedTimers)
+ nestedTimers->startTracking();
+
m_action->execute(context);
#if PLATFORM(IOS)
@@ -252,6 +320,16 @@
InspectorInstrumentation::didFireTimer(cookie);
+ // Check if we should throttle nested single-shot timers.
+ if (nestedTimers) {
+ for (auto& keyValue : *nestedTimers) {
+ auto* timer = keyValue.value;
+ if (timer->isActive() && !timer->repeatInterval())
+ timer->updateThrottlingStateIfNecessary(fireState);
+ }
+ nestedTimers->stopTracking();
+ }
+
context->setTimerNestingLevel(0);
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
