<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[176402] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/176402">176402</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2014-11-20 10:44:12 -0800 (Thu, 20 Nov 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageDOMTimercpp">trunk/Source/WebCore/page/DOMTimer.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastdomresourcesframewiththrottledtimerhtml">trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html</a></li>
<li><a href="#trunkLayoutTestsfastdomthrottledtimerrunningondocumentdestructionexpectedtxt">trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastdomthrottledtimerrunningondocumentdestructionhtml">trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (176401 => 176402)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-11-20 18:11:07 UTC (rev 176401)
+++ trunk/LayoutTests/ChangeLog 2014-11-20 18:44:12 UTC (rev 176402)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2014-11-20 Chris Dumez <cdumez@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2014-11-20 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Simple line layout: Introduce text fragment continuation.
</span></span></pre></div>
<a id="trunkLayoutTestsfastdomresourcesframewiththrottledtimerhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/resources/frame-with-throttled-timer.html (0 => 176402)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,14 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastdomthrottledtimerrunningondocumentdestructionexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction-expected.txt (0 => 176402)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+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
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastdomthrottledtimerrunningondocumentdestructionhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/throttled-timer-running-on-document-destruction.html (0 => 176402)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,22 @@
</span><ins>+<!DOCTYPE html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<iframe id="testFrame" src="resources/frame-with-throttled-timer.html"></iframe>
+
+<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="../../resources/js-test-post.js"></script>
+</body>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (176401 => 176402)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2014-11-20 Chris Dumez <cdumez@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2014-11-20 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Simple line layout: Introduce text fragment continuation.
</span></span></pre></div>
<a id="trunkSourceWebCorepageDOMTimercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/DOMTimer.cpp (176401 => 176402)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -99,11 +99,6 @@
</span><span class="cx"> return document && document->domTreeVersion() != m_initialDOMTreeVersion;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- void setChangedStyleOfElementOutsideViewport(StyledElement& element)
- {
- m_elementsChangedOutsideViewport.add(&element);
- }
-
</del><span class="cx"> void elementsChangedOutsideViewport(Vector<RefPtr<StyledElement>>& elements) const
</span><span class="cx"> {
</span><span class="cx"> copyToVector(m_elementsChangedOutsideViewport, elements);
</span><span class="lines">@@ -209,7 +204,9 @@
</span><span class="cx">
</span><span class="cx"> DOMTimer::~DOMTimer()
</span><span class="cx"> {
</span><del>- if (isIntervalDependentOnViewport())
</del><ins>+ // If the ScriptExecutionContext has already been destroyed, there is
+ // no need to stop listening for viewport changes.
+ if (scriptExecutionContext() && isIntervalDependentOnViewport())
</ins><span class="cx"> unregisterForViewportChanges();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -443,6 +440,7 @@
</span><span class="cx">
</span><span class="cx"> void DOMTimer::unregisterForViewportChanges()
</span><span class="cx"> {
</span><ins>+ ASSERT(scriptExecutionContext());
</ins><span class="cx"> if (auto* frameView = downcast<Document>(*scriptExecutionContext()).view())
</span><span class="cx"> frameView->unregisterThrottledDOMTimer(this);
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>