<!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>[183600] 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/183600">183600</a></dd>
<dt>Author</dt> <dd>simon.fraser@apple.com</dd>
<dt>Date</dt> <dd>2015-04-29 18:38:20 -0700 (Wed, 29 Apr 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>Crash at WebCore::Document::absoluteRegionForEventTargets
https://bugs.webkit.org/show_bug.cgi?id=144426
rdar://problem/20502166

Reviewed by Tim Horton.

Source/WebCore:

When a frame had wheel event handlers, we would register the document itself
as a handler in its parent document. This is problematic, because there's not
code path that removes it when the frame is destroyed.

It turns out we don't need to do this at all; the non-fast scrollable region
already takes handlers in subframes into account.

Tests: fast/events/wheelevent-in-frame.html
       fast/events/wheelevent-in-reattached-frame.html

* dom/Document.cpp:
(WebCore::Document::didAddWheelEventHandler):
(WebCore::Document::didRemoveWheelEventHandler):

LayoutTests:

Test that disconnects a frame with a wheel event handler then GCs, and one that
disconnects are reconnects. In both case, the parent document should have zero
wheel event handlers registered on it.

* fast/events/wheelevent-in-frame-expected.txt: Added.
* fast/events/wheelevent-in-frame.html: Added.
* fast/events/wheelevent-in-reattached-frame-expected.txt: Added.
* fast/events/wheelevent-in-reattached-frame.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="#trunkSourceWebCoredomDocumentcpp">trunk/Source/WebCore/dom/Document.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasteventswheeleventinframeexpectedtxt">trunk/LayoutTests/fast/events/wheelevent-in-frame-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfasteventswheeleventinframehtml">trunk/LayoutTests/fast/events/wheelevent-in-frame.html</a></li>
<li><a href="#trunkLayoutTestsfasteventswheeleventinreattachedframeexpectedtxt">trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfasteventswheeleventinreattachedframehtml">trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (183599 => 183600)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2015-04-30 01:21:08 UTC (rev 183599)
+++ trunk/LayoutTests/ChangeLog        2015-04-30 01:38:20 UTC (rev 183600)
</span><span class="lines">@@ -1,5 +1,22 @@
</span><span class="cx"> 2015-04-29  Simon Fraser  &lt;simon.fraser@apple.com&gt;
</span><span class="cx"> 
</span><ins>+        Crash at WebCore::Document::absoluteRegionForEventTargets 
+        https://bugs.webkit.org/show_bug.cgi?id=144426
+        rdar://problem/20502166
+
+        Reviewed by Tim Horton.
+
+        Test that disconnects a frame with a wheel event handler then GCs, and one that
+        disconnects are reconnects. In both case, the parent document should have zero
+        wheel event handlers registered on it.
+
+        * fast/events/wheelevent-in-frame-expected.txt: Added.
+        * fast/events/wheelevent-in-frame.html: Added.
+        * fast/events/wheelevent-in-reattached-frame-expected.txt: Added.
+        * fast/events/wheelevent-in-reattached-frame.html: Added.
+
+2015-04-29  Simon Fraser  &lt;simon.fraser@apple.com&gt;
+
</ins><span class="cx">         Compute the non-fast-scrollable region in main-document coordinates
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=144420
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkLayoutTestsfasteventswheeleventinframeexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/events/wheelevent-in-frame-expected.txt (0 => 183600)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/events/wheelevent-in-frame-expected.txt                                (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-frame-expected.txt        2015-04-30 01:38:20 UTC (rev 183600)
</span><span class="lines">@@ -0,0 +1,29 @@
</span><ins>+Tests that detaching a frame with a wheel event handlers doesn't crash.
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfasteventswheeleventinframehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/events/wheelevent-in-frame.html (0 => 183600)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/events/wheelevent-in-frame.html                                (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-frame.html        2015-04-30 01:38:20 UTC (rev 183600)
</span><span class="lines">@@ -0,0 +1,63 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+
+&lt;html&gt;
+&lt;head&gt;
+    &lt;style&gt;
+    &lt;/style&gt;
+    &lt;script src=&quot;../../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+    &lt;script&gt;
+    &lt;/script&gt;
+&lt;/head&gt;
+&lt;body&gt;
+
+    &lt;script&gt;
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    description(&quot;Tests that detaching a frame with a wheel event handlers doesn't crash.&quot;);
+    
+    const maxLoads = 10;
+    var loadCount = 0;
+
+    function makeFrame()
+    {
+        var frame = document.createElement('iframe');
+        frame.addEventListener('load', function() {
+            if (window.internals)
+                shouldBe(&quot;internals.wheelEventHandlerCount()&quot;, &quot;0&quot;);
+
+            frame.remove();
+            window.setTimeout(checkFrameRemoved, 0);
+        });
+
+        frame.src = 'resources/wheel-event-handler-on-document.html';
+        addFrameToDocument(frame);
+    }
+    
+    function checkFrameRemoved()
+    {
+        gc();
+
+        if (window.internals)
+            shouldBe(&quot;internals.wheelEventHandlerCount()&quot;, &quot;0&quot;);
+
+        if (++loadCount == maxLoads) {
+            isSuccessfullyParsed();
+            if (window.testRunner)
+                testRunner.notifyDone();
+
+            return;
+        }
+
+        window.setTimeout(makeFrame, 0);
+    }
+
+    function addFrameToDocument(frame)
+    {
+        document.body.appendChild(frame);
+    }
+    
+    makeFrame();
+    &lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsfasteventswheeleventinreattachedframeexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt (0 => 183600)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt                                (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame-expected.txt        2015-04-30 01:38:20 UTC (rev 183600)
</span><span class="lines">@@ -0,0 +1,29 @@
</span><ins>+Tests that detaching and reattaching a frame with a wheel event handlers doesn't crash.
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS internals.wheelEventHandlerCount() is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfasteventswheeleventinreattachedframehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame.html (0 => 183600)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame.html                                (rev 0)
+++ trunk/LayoutTests/fast/events/wheelevent-in-reattached-frame.html        2015-04-30 01:38:20 UTC (rev 183600)
</span><span class="lines">@@ -0,0 +1,66 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+
+&lt;html&gt;
+&lt;head&gt;
+    &lt;style&gt;
+    &lt;/style&gt;
+    &lt;script src=&quot;../../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+    &lt;script&gt;
+    &lt;/script&gt;
+&lt;/head&gt;
+&lt;body&gt;
+
+    &lt;script&gt;
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    description(&quot;Tests that detaching and reattaching a frame with a wheel event handlers doesn't crash.&quot;);
+    
+    const maxLoads = 10;
+    var loadCount = 0;
+
+    var frame;
+    function makeFrame()
+    {
+        frame = document.createElement('iframe');
+        frame.addEventListener('load', function() {
+            if (window.internals)
+                shouldBe(&quot;internals.wheelEventHandlerCount()&quot;, &quot;0&quot;);
+
+            frame.remove();
+            window.setTimeout(checkFrameRemoved, 0);
+        });
+
+        frame.src = 'resources/wheel-event-handlers-dynamic.html';
+        addFrameToDocument(frame);
+    }
+    
+    function checkFrameRemoved()
+    {
+        gc();
+
+        if (window.internals)
+            shouldBe(&quot;internals.wheelEventHandlerCount()&quot;, &quot;0&quot;);
+
+        if (++loadCount == maxLoads) {
+            isSuccessfullyParsed();
+            if (window.testRunner)
+                testRunner.notifyDone();
+
+            return;
+        }
+
+        window.setTimeout(function() {
+            addFrameToDocument(frame);
+        }, 0);
+    }
+
+    function addFrameToDocument(frame)
+    {
+        document.body.appendChild(frame);
+    }
+    
+    makeFrame();
+    &lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (183599 => 183600)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-04-30 01:21:08 UTC (rev 183599)
+++ trunk/Source/WebCore/ChangeLog        2015-04-30 01:38:20 UTC (rev 183600)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2015-04-29  Simon Fraser  &lt;simon.fraser@apple.com&gt;
+
+        Crash at WebCore::Document::absoluteRegionForEventTargets 
+        https://bugs.webkit.org/show_bug.cgi?id=144426
+        rdar://problem/20502166
+
+        Reviewed by Tim Horton.
+
+        When a frame had wheel event handlers, we would register the document itself
+        as a handler in its parent document. This is problematic, because there's not
+        code path that removes it when the frame is destroyed.
+        
+        It turns out we don't need to do this at all; the non-fast scrollable region
+        already takes handlers in subframes into account.
+
+        Tests: fast/events/wheelevent-in-frame.html
+               fast/events/wheelevent-in-reattached-frame.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::didAddWheelEventHandler):
+        (WebCore::Document::didRemoveWheelEventHandler):
+
</ins><span class="cx"> 2015-04-29  David Kilzer  &lt;ddkilzer@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Attempt #2: Switch QuickLook soft-linking to use QuickLookSoftLink.{h,mm}
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumentcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Document.cpp (183599 => 183600)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Document.cpp        2015-04-30 01:21:08 UTC (rev 183599)
+++ trunk/Source/WebCore/dom/Document.cpp        2015-04-30 01:38:20 UTC (rev 183600)
</span><span class="lines">@@ -5949,11 +5949,6 @@
</span><span class="cx"> 
</span><span class="cx">     m_wheelEventTargets-&gt;add(&amp;node);
</span><span class="cx"> 
</span><del>-    if (Document* parent = parentDocument()) {
-        parent-&gt;didAddWheelEventHandler(*this);
-        return;
-    }
-
</del><span class="cx">     wheelEventHandlersChanged();
</span><span class="cx"> 
</span><span class="cx">     if (Frame* frame = this-&gt;frame())
</span><span class="lines">@@ -5979,11 +5974,6 @@
</span><span class="cx">     if (!removeHandlerFromSet(*m_wheelEventTargets, node, removal))
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (Document* parent = parentDocument()) {
-        parent-&gt;didRemoveWheelEventHandler(*this);
-        return;
-    }
-
</del><span class="cx">     wheelEventHandlersChanged();
</span><span class="cx"> 
</span><span class="cx">     if (Frame* frame = this-&gt;frame())
</span></span></pre>
</div>
</div>

</body>
</html>