Diff
Modified: trunk/LayoutTests/ChangeLog (176495 => 176496)
--- trunk/LayoutTests/ChangeLog 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/LayoutTests/ChangeLog 2014-11-22 03:29:58 UTC (rev 176496)
@@ -1,5 +1,21 @@
2014-11-21 Chris Dumez <[email protected]>
+ Throttled DOMTimers can prevent their document from being garbage collected
+ https://bugs.webkit.org/show_bug.cgi?id=138915
+
+ Reviewed by Andreas Kling.
+
+ Improve fast/dom/throttled-timer-running-on-document-destruction.html
+ layout test to cover the case where the throttled timer is changing the
+ style of an element on the *same* document when the document is
+ destroyed.
+
+ * fast/dom/resources/frame-with-throttled-timer-animating-element-other-document.html: Renamed from LayoutTests/fast/dom/resources/frame-with-throttled-timer.html.
+ * fast/dom/resources/frame-with-throttled-timer-animating-element-same-document.html: Added.
+ * fast/dom/throttled-timer-running-on-document-destruction.html:
+
+2014-11-21 Chris Dumez <[email protected]>
+
[iOS] Regression(r176202): line-height is wrong on marco.org
https://bugs.webkit.org/show_bug.cgi?id=138970
Copied: trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer-animating-element-other-document.html (from rev 176495, trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html) (0 => 176496)
--- trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer-animating-element-other-document.html (rev 0)
+++ trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer-animating-element-other-document.html 2014-11-22 03:29:58 UTC (rev 176496)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<body>
+<iframe id="testFrame"></iframe>
+<script>
+document.getElementById('testFrame').contentDocument.body.innerHTML = "<div id='testElement' style='display: none'>TEST</div>"
+
+setInterval(function() {
+ // Change the style of a display:none element.
+ var testFrame = document.getElementById("testFrame");
+ var testElement = testFrame.contentDocument.getElementById("testElement");
+ testElement.style["left"] = "" + Math.floor((Math.random() * 10) + 1) + "px";
+}, 0);
+</script>
+</body>
Added: trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer-animating-element-same-document.html (0 => 176496)
--- trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer-animating-element-same-document.html (rev 0)
+++ trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer-animating-element-same-document.html 2014-11-22 03:29:58 UTC (rev 176496)
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<body>
+<div id='testElement' style='display: none'>TEST</div>
+<script>
+setInterval(function() {
+ // Change the style of a display:none element.
+ var testElement = document.getElementById("testElement");
+ testElement.style["left"] = "" + Math.floor((Math.random() * 10) + 1) + "px";
+}, 0);
+</script>
+</body>
Deleted: trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html (176495 => 176496)
--- trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html 2014-11-22 03:29:58 UTC (rev 176496)
@@ -1,14 +0,0 @@
-<!DOCTYPE html>
-<body>
-<iframe id="testFrame"></iframe>
-<script>
-document.getElementById('testFrame').contentDocument.body.innerHTML = "<div id='testElement' style='display: none'>TEST</div>"
-
-setInterval(function() {
- // Change the style of a display:none element.
- var testFrame = document.getElementById("testFrame");
- var testElement = testFrame.contentDocument.getElementById("testElement");
- testElement.style["left"] = "" + Math.floor((Math.random() * 10) + 1) + "px";
-}, 5);
-</script>
-</body>
Modified: trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html (176495 => 176496)
--- trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html 2014-11-22 03:29:58 UTC (rev 176496)
@@ -1,22 +1,31 @@
<!DOCTYPE html>
<body>
<script src=""
-<iframe id="testFrame" src=""
-
<script>
description("Test that we don't crash if a throttled timer is still running when the document is destroyed.");
jsTestIsAsync = true;
-function removeFrame()
+var frameLoadedCount = 0;
+
+function removeFrames()
{
- document.body.removeChild(document.getElementById("testFrame"));
+ document.body.removeChild(document.getElementById("testFrame1"));
+ document.body.removeChild(document.getElementById("testFrame2"));
gc();
testPassed("Did not crash.");
finishJSTest();
}
-setTimeout(removeFrame, 300);
+function frameLoaded()
+{
+ ++frameLoadedCount;
+ if (frameLoadedCount == 2)
+ setTimeout(removeFrames, 100);
+}
</script>
+<iframe id="testFrame1" src="" _onload_="frameLoaded()"></iframe>
+<iframe id="testFrame2" src="" _onload_="frameLoaded()"></iframe>
+
<script src=""
</body>
Modified: trunk/Source/WebCore/ChangeLog (176495 => 176496)
--- trunk/Source/WebCore/ChangeLog 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/ChangeLog 2014-11-22 03:29:58 UTC (rev 176496)
@@ -1,3 +1,37 @@
+2014-11-21 Chris Dumez <[email protected]>
+
+ Throttled DOMTimers can prevent their document from being garbage collected
+ https://bugs.webkit.org/show_bug.cgi?id=138915
+
+ Reviewed by Andreas Kling.
+
+ Throttled DOMTimers whose interval depend on viewport changes would
+ keep a Vector of elements outside viewport causing them to be throttled
+ so that we could check later on (upon scroll or layout) if those
+ elements are still outside viewport. The issue is that these elements
+ could potentially be removed from the document (and destroyed) after
+ the timer has fired. To handle this, DOMTimer was ref'ing the
+ elements. Unfortunately, this was causing us to leak the document
+ as the elements in the Vector would keep the document alive.
+
+ To handle this issue, this patch updates the DOMTimer Vector to use
+ weak pointers. The WeakPtrFactory is stored in ElementRareData to
+ avoid wasting memory for all kinds of Elements (it is a fair assumption
+ that the number of elements whose style is animated via timers is low).
+
+ Test: fast/dom/throttled-timer-running-on-document-destruction.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::createWeakPtr):
+ * dom/Element.h:
+ * dom/ElementRareData.cpp:
+ * dom/ElementRareData.h:
+ (WebCore::ElementRareData::weakPtrFactory):
+ * page/DOMTimer.cpp:
+ (WebCore::DOMTimerFireState::elementsChangedOutsideViewport):
+ (WebCore::DOMTimer::updateThrottlingStateAfterViewportChange):
+ * page/DOMTimer.h:
+
2014-11-21 Gyuyoung Kim <[email protected]>
Unreviewed, EFL build fix since r176459.
Modified: trunk/Source/WebCore/dom/Element.cpp (176495 => 176496)
--- trunk/Source/WebCore/dom/Element.cpp 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/dom/Element.cpp 2014-11-22 03:29:58 UTC (rev 176496)
@@ -1195,6 +1195,11 @@
return document().completeURL(stripLeadingAndTrailingHTMLSpaces(linkAttribute));
}
+WeakPtr<Element> Element::createWeakPtr()
+{
+ return ensureElementRareData().weakPtrFactory().createWeakPtr();
+}
+
// Returns true is the given attribute is an event handler.
// We consider an event handler any attribute that begins with "on".
// It is a simple solution that has the advantage of not requiring any
Modified: trunk/Source/WebCore/dom/Element.h (176495 => 176496)
--- trunk/Source/WebCore/dom/Element.h 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/dom/Element.h 2014-11-22 03:29:58 UTC (rev 176496)
@@ -551,6 +551,7 @@
void clearHoverAndActiveStatusBeforeDetachingRenderer();
WEBCORE_EXPORT URL absoluteLinkURL() const;
+ WeakPtr<Element> createWeakPtr();
protected:
Element(const QualifiedName&, Document&, ConstructionType);
Modified: trunk/Source/WebCore/dom/ElementRareData.cpp (176495 => 176496)
--- trunk/Source/WebCore/dom/ElementRareData.cpp 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/dom/ElementRareData.cpp 2014-11-22 03:29:58 UTC (rev 176496)
@@ -39,7 +39,7 @@
RegionOversetState regionOversetState;
LayoutSize sizeForResizing;
IntSize scrollOffset;
- void* pointers[7];
+ void* pointers[8];
};
static_assert(sizeof(ElementRareData) == sizeof(SameSizeAsElementRareData), "ElementRareData should stay small");
Modified: trunk/Source/WebCore/dom/ElementRareData.h (176495 => 176496)
--- trunk/Source/WebCore/dom/ElementRareData.h 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/dom/ElementRareData.h 2014-11-22 03:29:58 UTC (rev 176496)
@@ -36,7 +36,7 @@
class ElementRareData : public NodeRareData {
public:
- explicit ElementRareData(RenderElement*);
+ ElementRareData(Element&, RenderElement*);
~ElementRareData();
void setBeforePseudoElement(PassRefPtr<PseudoElement>);
@@ -114,6 +114,8 @@
bool hasPendingResources() const { return m_hasPendingResources; }
void setHasPendingResources(bool has) { m_hasPendingResources = has; }
+ WeakPtrFactory<Element>& weakPtrFactory() { return m_weakPtrFactory; }
+
private:
short m_tabIndex;
unsigned short m_childIndex;
@@ -147,6 +149,7 @@
RefPtr<PseudoElement> m_beforePseudoElement;
RefPtr<PseudoElement> m_afterPseudoElement;
+ WeakPtrFactory<Element> m_weakPtrFactory;
void releasePseudoElement(PseudoElement*);
};
@@ -156,7 +159,7 @@
return IntSize(LayoutUnit::max(), LayoutUnit::max());
}
-inline ElementRareData::ElementRareData(RenderElement* renderer)
+inline ElementRareData::ElementRareData(Element& element, RenderElement* renderer)
: NodeRareData(renderer)
, m_tabIndex(0)
, m_childIndex(0)
@@ -175,6 +178,7 @@
, m_childrenAffectedByPropertyBasedBackwardPositionalRules(false)
, m_regionOversetState(RegionUndefined)
, m_minimumSizeForResizing(defaultMinimumSizeForResizing())
+ , m_weakPtrFactory(&element)
{
}
Modified: trunk/Source/WebCore/dom/Node.cpp (176495 => 176496)
--- trunk/Source/WebCore/dom/Node.cpp 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/dom/Node.cpp 2014-11-22 03:29:58 UTC (rev 176496)
@@ -350,7 +350,7 @@
{
NodeRareData* data;
if (is<Element>(*this))
- data = ""
+ data = "" downcast<RenderElement>(m_data.m_renderer)).release();
else
data = ""
ASSERT(data);
Modified: trunk/Source/WebCore/page/DOMTimer.cpp (176495 => 176496)
--- trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-22 03:29:58 UTC (rev 176496)
@@ -99,9 +99,12 @@
return document && document->domTreeVersion() != m_initialDOMTreeVersion;
}
- void elementsChangedOutsideViewport(Vector<RefPtr<StyledElement>>& elements) const
+ void elementsChangedOutsideViewport(Vector<WeakPtr<Element>>& elements) const
{
- copyToVector(m_elementsChangedOutsideViewport, elements);
+ ASSERT(elements.isEmpty());
+ elements.reserveCapacity(m_elementsChangedOutsideViewport.size());
+ for (auto& element : m_elementsChangedOutsideViewport)
+ elements.uncheckedAppend(element->createWeakPtr());
}
static DOMTimerFireState* current;
@@ -471,9 +474,10 @@
{
ASSERT(isIntervalDependentOnViewport());
// Check if the elements that caused this timer to be throttled are still outside the viewport.
- for (auto& element : m_elementsCausingThrottling) {
+ for (auto& weakElementPtr : m_elementsCausingThrottling) {
+ Element* element = weakElementPtr.get();
// Skip elements that were removed from the document.
- if (!element->inDocument())
+ if (!element || !element->inDocument())
continue;
if (element->isInsideViewport(&visibleRect)) {
Modified: trunk/Source/WebCore/page/DOMTimer.h (176495 => 176496)
--- trunk/Source/WebCore/page/DOMTimer.h 2014-11-22 03:27:53 UTC (rev 176495)
+++ trunk/Source/WebCore/page/DOMTimer.h 2014-11-22 03:29:58 UTC (rev 176496)
@@ -31,11 +31,13 @@
#include <memory>
#include <wtf/HashSet.h>
#include <wtf/RefCounted.h>
+#include <wtf/WeakPtr.h>
namespace WebCore {
class DOMTimerFireState;
class Document;
+ class Element;
class HTMLPlugInElement;
class IntRect;
class ScheduledAction;
@@ -91,9 +93,10 @@
TimerThrottleState m_throttleState;
double m_currentTimerInterval;
bool m_shouldForwardUserGesture;
- // Hold a reference to the elements in case they get removed from the
- // Document after the timer is throttled.
- Vector<RefPtr<StyledElement>> m_elementsCausingThrottling;
+ // Use WeakPtrs because we don't want to keep the elements alive but we
+ // still need to handle cases where the elements get destroyed after
+ // the timer has fired.
+ Vector<WeakPtr<Element>> m_elementsCausingThrottling;
};
} // namespace WebCore
