Title: [176402] trunk
- Revision
- 176402
- Author
- [email protected]
- Date
- 2014-11-20 10:44:12 -0800 (Thu, 20 Nov 2014)
Log Message
Crash when destroying a Document that has a throttled timer still running
https://bugs.webkit.org/show_bug.cgi?id=138914
Reviewed by Benjamin Poulain.
Source/WebCore:
Upon destruction, a throttled DOMTimer whose interval depends on
viewport changes will try to unregister itself from the view. It gets
the view pointer from its Document. However, scriptExecutionContext()
can return null if the Document is being destroyed (i.e. ~DOMTimer()
is called from ~ScriptExecutionContext(), as the ScriptExecutionContext
owns the DOMTimer).
This patch adds a null check for scriptExecutionContext() in the
DOMTimer destructor to avoid this issue.
Test: fast/dom/throttled-timer-running-on-document-destruction.html
* page/DOMTimer.cpp:
(WebCore::DOMTimer::~DOMTimer):
(WebCore::DOMTimer::unregisterForViewportChanges):
Add assertion to make sure scriptExecutionContext() does not return
null.
(WebCore::DOMTimerFireState::setChangedStyleOfElementOutsideViewport): Deleted.
Killed this function as this was dead code.
LayoutTests:
Add a layout test to test the case where a Document gets destroyed while
throttled timer is still running.
* fast/dom/resources/frame-with-throttled-timer.html: Added.
* fast/dom/throttled-timer-running-on-document-destruction-expected.txt: Added.
* fast/dom/throttled-timer-running-on-document-destruction.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (176401 => 176402)
--- trunk/LayoutTests/ChangeLog 2014-11-20 18:11:07 UTC (rev 176401)
+++ trunk/LayoutTests/ChangeLog 2014-11-20 18:44:12 UTC (rev 176402)
@@ -1,3 +1,17 @@
+2014-11-20 Chris Dumez <[email protected]>
+
+ Crash when destroying a Document that has a throttled timer still running
+ https://bugs.webkit.org/show_bug.cgi?id=138914
+
+ Reviewed by Benjamin Poulain.
+
+ Add a layout test to test the case where a Document gets destroyed while
+ throttled timer is still running.
+
+ * fast/dom/resources/frame-with-throttled-timer.html: Added.
+ * fast/dom/throttled-timer-running-on-document-destruction-expected.txt: Added.
+ * fast/dom/throttled-timer-running-on-document-destruction.html: Added.
+
2014-11-20 Zalan Bujtas <[email protected]>
Simple line layout: Introduce text fragment continuation.
Added: trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html (0 => 176402)
--- trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html (rev 0)
+++ trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html 2014-11-20 18:44:12 UTC (rev 176402)
@@ -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";
+}, 5);
+</script>
+</body>
Added: trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction-expected.txt (0 => 176402)
--- trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction-expected.txt 2014-11-20 18:44:12 UTC (rev 176402)
@@ -0,0 +1,10 @@
+Test that we don't crash if a throttled timer is still running when the document is destroyed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Did not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html (0 => 176402)
--- trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html (rev 0)
+++ trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html 2014-11-20 18:44:12 UTC (rev 176402)
@@ -0,0 +1,22 @@
+<!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()
+{
+ document.body.removeChild(document.getElementById("testFrame"));
+ gc();
+ testPassed("Did not crash.");
+ finishJSTest();
+}
+
+setTimeout(removeFrame, 300);
+
+</script>
+<script src=""
+</body>
Modified: trunk/Source/WebCore/ChangeLog (176401 => 176402)
--- trunk/Source/WebCore/ChangeLog 2014-11-20 18:11:07 UTC (rev 176401)
+++ trunk/Source/WebCore/ChangeLog 2014-11-20 18:44:12 UTC (rev 176402)
@@ -1,3 +1,32 @@
+2014-11-20 Chris Dumez <[email protected]>
+
+ Crash when destroying a Document that has a throttled timer still running
+ https://bugs.webkit.org/show_bug.cgi?id=138914
+
+ Reviewed by Benjamin Poulain.
+
+ Upon destruction, a throttled DOMTimer whose interval depends on
+ viewport changes will try to unregister itself from the view. It gets
+ the view pointer from its Document. However, scriptExecutionContext()
+ can return null if the Document is being destroyed (i.e. ~DOMTimer()
+ is called from ~ScriptExecutionContext(), as the ScriptExecutionContext
+ owns the DOMTimer).
+
+ This patch adds a null check for scriptExecutionContext() in the
+ DOMTimer destructor to avoid this issue.
+
+ Test: fast/dom/throttled-timer-running-on-document-destruction.html
+
+ * page/DOMTimer.cpp:
+ (WebCore::DOMTimer::~DOMTimer):
+
+ (WebCore::DOMTimer::unregisterForViewportChanges):
+ Add assertion to make sure scriptExecutionContext() does not return
+ null.
+
+ (WebCore::DOMTimerFireState::setChangedStyleOfElementOutsideViewport): Deleted.
+ Killed this function as this was dead code.
+
2014-11-20 Zalan Bujtas <[email protected]>
Simple line layout: Introduce text fragment continuation.
Modified: trunk/Source/WebCore/page/DOMTimer.cpp (176401 => 176402)
--- trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-20 18:11:07 UTC (rev 176401)
+++ trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-20 18:44:12 UTC (rev 176402)
@@ -99,11 +99,6 @@
return document && document->domTreeVersion() != m_initialDOMTreeVersion;
}
- void setChangedStyleOfElementOutsideViewport(StyledElement& element)
- {
- m_elementsChangedOutsideViewport.add(&element);
- }
-
void elementsChangedOutsideViewport(Vector<RefPtr<StyledElement>>& elements) const
{
copyToVector(m_elementsChangedOutsideViewport, elements);
@@ -209,7 +204,9 @@
DOMTimer::~DOMTimer()
{
- if (isIntervalDependentOnViewport())
+ // If the ScriptExecutionContext has already been destroyed, there is
+ // no need to stop listening for viewport changes.
+ if (scriptExecutionContext() && isIntervalDependentOnViewport())
unregisterForViewportChanges();
}
@@ -443,6 +440,7 @@
void DOMTimer::unregisterForViewportChanges()
{
+ ASSERT(scriptExecutionContext());
if (auto* frameView = downcast<Document>(*scriptExecutionContext()).view())
frameView->unregisterThrottledDOMTimer(this);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
