<!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>[172396] 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/172396">172396</a></dd>
<dt>Author</dt> <dd>burg@cs.washington.edu</dd>
<dt>Date</dt> <dd>2014-08-11 11:10:25 -0700 (Mon, 11 Aug 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>Web Inspector: Add a helper to avoid leaking single-fire event listeners in Promise chains
https://bugs.webkit.org/show_bug.cgi?id=135772

Reviewed by Timothy Hatcher.

Source/WebInspectorUI:

A common pattern when working with promise chains is to convert an event
handler into a promise by using a single-fire event listener with the
resolve continuation as the callback. This is fine if the event fires;
if it doesn't fire, then the event emitter permanently keeps a reference to the
this-object and the callback.

This patch adds EventListener, a proxy object for events that can be manipulated
from multiple promise callback functions. If a promise is rejected, the catch
block can disconnect any event listeners set up earlier in the promise chain.

This patch also reimplements EventListenerSet to use multiple EventListeners,
since they share the same logic to uniformly handle Inspector and DOM events.

Test: inspector/event-listener.html
Test: inspector/event-listener-set.html

* UserInterface/Base/EventListener.js: Added.
(WebInspector.EventListener):
(WebInspector.EventListener.prototype.this._callback):
(WebInspector.EventListener.prototype.connect):
(WebInspector.EventListener.prototype.disconnect):
* UserInterface/Base/EventListenerSet.js: Update license block.
(WebInspector.EventListenerSet.prototype.register):
(WebInspector.EventListenerSet.prototype.install):
(WebInspector.EventListenerSet.prototype.uninstall):
* UserInterface/Main.html: Include EventListener.
* UserInterface/Test.html: Include EventListener and EventListenerSet.

LayoutTests:

* inspector/event-listener-expected.txt: Added.
* inspector/event-listener-set-expected.txt: Added.
* inspector/event-listener-set.html: Added.
* inspector/event-listener.html: Added.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUIChangeLog">trunk/Source/WebInspectorUI/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceBaseEventListenerSetjs">trunk/Source/WebInspectorUI/UserInterface/Base/EventListenerSet.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceMainhtml">trunk/Source/WebInspectorUI/UserInterface/Main.html</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceTesthtml">trunk/Source/WebInspectorUI/UserInterface/Test.html</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsinspectoreventlistenerexpectedtxt">trunk/LayoutTests/inspector/event-listener-expected.txt</a></li>
<li><a href="#trunkLayoutTestsinspectoreventlistenersetexpectedtxt">trunk/LayoutTests/inspector/event-listener-set-expected.txt</a></li>
<li><a href="#trunkLayoutTestsinspectoreventlistenersethtml">trunk/LayoutTests/inspector/event-listener-set.html</a></li>
<li><a href="#trunkLayoutTestsinspectoreventlistenerhtml">trunk/LayoutTests/inspector/event-listener.html</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceBaseEventListenerjs">trunk/Source/WebInspectorUI/UserInterface/Base/EventListener.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (172395 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2014-08-11 17:52:15 UTC (rev 172395)
+++ trunk/LayoutTests/ChangeLog        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2014-08-11  Brian J. Burg  &lt;burg@cs.washington.edu&gt;
+
+        Web Inspector: Add a helper to avoid leaking single-fire event listeners in Promise chains
+        https://bugs.webkit.org/show_bug.cgi?id=135772
+
+        Reviewed by Timothy Hatcher.
+
+        * inspector/event-listener-expected.txt: Added.
+        * inspector/event-listener-set-expected.txt: Added.
+        * inspector/event-listener-set.html: Added.
+        * inspector/event-listener.html: Added.
+
</ins><span class="cx"> 2014-08-10  Oliver Hunt  &lt;oliver@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Destructuring assignment in a var declaration list incorrectly consumes subsequent variable initialisers
</span></span></pre></div>
<a id="trunkLayoutTestsinspectoreventlistenerexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/event-listener-expected.txt (0 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/event-listener-expected.txt                                (rev 0)
+++ trunk/LayoutTests/inspector/event-listener-expected.txt        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -0,0 +1,16 @@
</span><ins>+Testing basic functionality of WebInspector.EventListener.
+
+Connecting the listener.
+Invoked callback for kaboom event.
+Invoked callback for kaboom event.
+Disconnecting the listener.
+Connecting the listener.
+Invoked callback for kaboom event.
+Disconnecting the listener.
+Connecting the listener.
+Disconnecting the listener.
+Connecting the single-fire listener.
+Invoked callback for kaboom event.
+Disconnecting the single-fire listener.
+Invoked callback for kaboom event.
+
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectoreventlistenersetexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/event-listener-set-expected.txt (0 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/event-listener-set-expected.txt                                (rev 0)
+++ trunk/LayoutTests/inspector/event-listener-set-expected.txt        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -0,0 +1,25 @@
</span><ins>+Testing basic functionality of WebInspector.EventListenerSet.
+
+Registering listeners.
+Installing listeners.
+Dispatching events.
+Invoked callback for foo event.
+Invoked callback for bar event.
+Invoked callback for baz event.
+Uninstalling and disconnecting listeners.
+Registering listeners.
+Dispatching events.
+Invoked callback for foo event.
+Invoked callback for bar event.
+Invoked callback for baz event.
+Uninstalling listeners.
+Installing listeners.
+Dispatching events.
+Invoked callback for foo event.
+Invoked callback for bar event.
+Invoked callback for baz event.
+Unregistering everything.
+Dispatching events.
+Unintalling and disconnecting listeners.
+Dispatching events.
+
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectoreventlistenersethtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/event-listener-set.html (0 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/event-listener-set.html                                (rev 0)
+++ trunk/LayoutTests/inspector/event-listener-set.html        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -0,0 +1,117 @@
</span><ins>+&lt;!doctype html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script type=&quot;text/javascript&quot; src=&quot;../http/tests/inspector/inspector-test.js&quot;&gt;&lt;/script&gt;
+&lt;script&gt;
+function test()
+{
+    const FooEvent = &quot;foo&quot;;
+    const BarEvent = &quot;bar&quot;;
+    const BazEvent = &quot;baz&quot;;
+    var emitter1 = new WebInspector.Object();
+    var emitter2 = new WebInspector.Object();
+    var context1 = new WebInspector.Object();
+    var context2 = new WebInspector.Object();
+    var data1 = [1, 2, 3];
+    var data2 = [4, 6, 8];
+
+    function fooCallback(event)
+    {
+        InspectorTest.assert(this === context1, &quot;Callback invoked with wrong |this| binding.&quot;);
+        InspectorTest.assert(event.target === emitter1, &quot;Callback invoked with wrong event emitter.&quot;);
+        InspectorTest.assert(event.data === data1, &quot;Callback invoked with wrong event data.&quot;);
+
+        InspectorTest.log(&quot;Invoked callback for foo event.&quot;);
+    }
+
+    function barCallback(event)
+    {
+        InspectorTest.assert(this === context1, &quot;Callback invoked with wrong |this| binding.&quot;);
+        InspectorTest.assert(event.target === emitter1, &quot;Callback invoked with wrong event emitter.&quot;);
+        InspectorTest.assert(event.data === data2, &quot;Callback invoked with wrong event data.&quot;);
+
+        InspectorTest.log(&quot;Invoked callback for bar event.&quot;);
+    }
+
+    function bazCallback(event)
+    {
+        InspectorTest.assert(this === context2, &quot;Callback invoked with wrong |this| binding.&quot;);
+        InspectorTest.assert(event.target === emitter2, &quot;Callback invoked with wrong event emitter.&quot;);
+        InspectorTest.assert(event.data === data2, &quot;Callback invoked with wrong event data.&quot;);
+
+        InspectorTest.log(&quot;Invoked callback for baz event.&quot;);
+    }
+
+    // Test for multiple firings of listeners in the set.
+
+    var listenerSet = new WebInspector.EventListenerSet(context1);
+    InspectorTest.assert(!emitter1.hasEventListeners(FooEvent), &quot;Emitter should not have any listeners.&quot;);
+    emitter1.dispatchEventToListeners(FooEvent, data1); // Should not fire anything.
+
+    InspectorTest.log(&quot;Registering listeners.&quot;);
+    listenerSet.register(emitter1, FooEvent, fooCallback);
+    listenerSet.register(emitter1, BarEvent, barCallback);
+    listenerSet.register(emitter2, BazEvent, bazCallback, context2);
+    InspectorTest.assert(!emitter1.hasEventListeners(FooEvent), &quot;Emitter should not have a listener.&quot;);
+    InspectorTest.assert(!emitter1.hasEventListeners(BarEvent), &quot;Emitter should not have a listener.&quot;);
+    InspectorTest.assert(!emitter2.hasEventListeners(BazEvent), &quot;Emitter should not have a listener.&quot;);
+
+    InspectorTest.log(&quot;Installing listeners.&quot;);
+    listenerSet.install();
+    InspectorTest.assert(emitter1.hasEventListeners(FooEvent), &quot;Emitter should have a listener.&quot;);
+    InspectorTest.assert(emitter1.hasEventListeners(BarEvent), &quot;Emitter should have a listener.&quot;);
+    InspectorTest.assert(emitter2.hasEventListeners(BazEvent), &quot;Emitter should have a listener.&quot;);
+
+    InspectorTest.log(&quot;Dispatching events.&quot;);
+    emitter1.dispatchEventToListeners(FooEvent, data1); // Should fire.
+    emitter1.dispatchEventToListeners(BarEvent, data2); // Should fire.
+    emitter2.dispatchEventToListeners(BazEvent, data2); // Should fire.
+    InspectorTest.log(&quot;Uninstalling and disconnecting listeners.&quot;);
+    listenerSet.uninstall(true);
+
+    InspectorTest.log(&quot;Registering listeners.&quot;);
+    listenerSet.register(emitter1, FooEvent, fooCallback);
+    listenerSet.register(emitter1, BarEvent, barCallback);
+    listenerSet.register(emitter2, BazEvent, bazCallback, context2);
+
+    listenerSet.install();
+    InspectorTest.log(&quot;Dispatching events.&quot;);
+    emitter1.dispatchEventToListeners(FooEvent, data1); // Should fire.
+    emitter1.dispatchEventToListeners(BarEvent, data2); // Should fire.
+    emitter2.dispatchEventToListeners(BazEvent, data2); // Should fire.
+    InspectorTest.log(&quot;Uninstalling listeners.&quot;);
+    listenerSet.uninstall();
+
+    InspectorTest.log(&quot;Installing listeners.&quot;);
+    listenerSet.install();
+    InspectorTest.log(&quot;Dispatching events.&quot;);
+    emitter1.dispatchEventToListeners(FooEvent, data1); // Should fire.
+    emitter1.dispatchEventToListeners(BarEvent, data2); // Should fire.
+    emitter2.dispatchEventToListeners(BazEvent, data2); // Should fire.
+
+    InspectorTest.log(&quot;Unregistering everything.&quot;);
+    listenerSet.unregister();
+    InspectorTest.log(&quot;Dispatching events.&quot;);
+    emitter1.dispatchEventToListeners(FooEvent, data1); // Should not fire.
+    emitter1.dispatchEventToListeners(BarEvent, data2); // Should not fire.
+    emitter2.dispatchEventToListeners(BazEvent, data2); // Should not fire.
+
+    InspectorTest.log(&quot;Unintalling and disconnecting listeners.&quot;);
+    listenerSet.uninstall(true);
+    InspectorTest.log(&quot;Dispatching events.&quot;);
+    emitter1.dispatchEventToListeners(FooEvent, data1); // Should not fire.
+    emitter1.dispatchEventToListeners(BarEvent, data2); // Should not fire.
+    emitter2.dispatchEventToListeners(BazEvent, data2); // Should not fire.
+
+    InspectorTest.assert(!emitter1.hasEventListeners(FooEvent), &quot;Emitter should not have a listener.&quot;);
+    InspectorTest.assert(!emitter1.hasEventListeners(BarEvent), &quot;Emitter should not have a listener.&quot;);
+    InspectorTest.assert(!emitter2.hasEventListeners(BazEvent), &quot;Emitter should not have a listener.&quot;);
+
+    InspectorTest.completeTest();
+}
+&lt;/script&gt;
+&lt;/head&gt;
+&lt;body onload=&quot;runTest()&quot;&gt;
+    &lt;p&gt;Testing basic functionality of WebInspector.EventListenerSet.&lt;/p&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectoreventlistenerhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/event-listener.html (0 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/event-listener.html                                (rev 0)
+++ trunk/LayoutTests/inspector/event-listener.html        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -0,0 +1,93 @@
</span><ins>+&lt;!doctype html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script type=&quot;text/javascript&quot; src=&quot;../http/tests/inspector/inspector-test.js&quot;&gt;&lt;/script&gt;
+&lt;script&gt;
+function test()
+{
+    const KaboomEvent = &quot;kaboom&quot;;
+    var emitter = new WebInspector.Object();
+    var context = new WebInspector.Object();
+    var data = [1, 2, 3];
+
+    function kaboomCallback(event) {
+        InspectorTest.assert(this === context, &quot;Callback invoked with wrong |this| binding.&quot;);
+        InspectorTest.assert(event.target === emitter, &quot;Callback invoked with wrong event emitter.&quot;);
+        InspectorTest.assert(event.data === data, &quot;Callback invoked with wrong event data.&quot;);
+
+        InspectorTest.log(&quot;Invoked callback for kaboom event.&quot;);
+    }
+
+    // Test for multiple firings of the listener.
+
+    var listener = new WebInspector.EventListener(context);
+    InspectorTest.assert(!emitter.hasEventListeners(KaboomEvent), &quot;Emitter should not have any listeners.&quot;);
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should not fire anything.
+
+    InspectorTest.log(&quot;Connecting the listener.&quot;);
+    listener.connect(emitter, KaboomEvent, kaboomCallback);
+    InspectorTest.assert(emitter.hasEventListeners(KaboomEvent), &quot;Emitter should have a listener.&quot;);
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should fire.
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should fire.
+    InspectorTest.log(&quot;Disconnecting the listener.&quot;);
+    listener.disconnect();
+
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should not fire anything.
+    InspectorTest.assert(!emitter.hasEventListeners(KaboomEvent), &quot;Emitter should not have any listeners.&quot;);
+
+    // Test reconnection.
+
+    InspectorTest.log(&quot;Connecting the listener.&quot;);
+    listener.connect(emitter, KaboomEvent, kaboomCallback);
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should fire.
+    InspectorTest.log(&quot;Disconnecting the listener.&quot;);
+    listener.disconnect();
+
+    // Test unused listener.
+
+    InspectorTest.log(&quot;Connecting the listener.&quot;);
+    listener.connect(emitter, KaboomEvent, kaboomCallback);
+    InspectorTest.log(&quot;Disconnecting the listener.&quot;);
+    listener.disconnect();
+
+    // Test for single firing of the listener.
+
+    var singleListener = new WebInspector.EventListener(context, true);
+    InspectorTest.assert(!emitter.hasEventListeners(KaboomEvent), &quot;Emitter should not have any listeners.&quot;);
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should not fire anything.
+
+    InspectorTest.log(&quot;Connecting the single-fire listener.&quot;);
+    singleListener.connect(emitter, KaboomEvent, kaboomCallback);
+    InspectorTest.assert(emitter.hasEventListeners(KaboomEvent), &quot;Emitter should have a listener.&quot;);
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should fire.
+    InspectorTest.assert(!emitter.hasEventListeners(KaboomEvent), &quot;Emitter should not have any listeners.&quot;);
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should not fire.
+    InspectorTest.log(&quot;Disconnecting the single-fire listener.&quot;);
+    singleListener.disconnect(); // Should cause an error.
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should not fire.
+
+    // Test for various error cases and abuse.
+
+    var badListener = new WebInspector.EventListener(context);
+    badListener.connect(data, data, data); // Should complain about non-callable callback.
+    badListener.connect(null, KaboomEvent, kaboomCallback); // Should complain about non-callable callback.
+    badListener.connect(emitter, KaboomEvent, null); // Should complain about non-callable callback.
+    badListener.connect(emitter, null, kaboomCallback); // Should complain about null event.
+
+    var badListener2 = new WebInspector.EventListener(context);
+    badListener2.disconnect(); // Should complain about already disconnected.
+    badListener2.connect(emitter, KaboomEvent, kaboomCallback);
+    badListener2.connect(emitter, KaboomEvent, kaboomCallback); // Should complain about already connected.
+    emitter.dispatchEventToListeners(KaboomEvent, data); // Should fire.
+    badListener2.connect(emitter, KaboomEvent, kaboomCallback); // Should complain about already connected.
+    badListener2.disconnect();
+    badListener2.disconnect(); // Should complain about already disconnected.
+
+    InspectorTest.completeTest();
+}
+&lt;/script&gt;
+&lt;/head&gt;
+&lt;body onload=&quot;runTest()&quot;&gt;
+    &lt;p&gt;Testing basic functionality of WebInspector.EventListener.&lt;/p&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkSourceWebInspectorUIChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/ChangeLog (172395 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/ChangeLog        2014-08-11 17:52:15 UTC (rev 172395)
+++ trunk/Source/WebInspectorUI/ChangeLog        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2014-08-11  Brian J. Burg  &lt;burg@cs.washington.edu&gt;
+
+        Web Inspector: Add a helper to avoid leaking single-fire event listeners in Promise chains
+        https://bugs.webkit.org/show_bug.cgi?id=135772
+
+        Reviewed by Timothy Hatcher.
+
+        A common pattern when working with promise chains is to convert an event
+        handler into a promise by using a single-fire event listener with the
+        resolve continuation as the callback. This is fine if the event fires;
+        if it doesn't fire, then the event emitter permanently keeps a reference to the
+        this-object and the callback.
+
+        This patch adds EventListener, a proxy object for events that can be manipulated
+        from multiple promise callback functions. If a promise is rejected, the catch
+        block can disconnect any event listeners set up earlier in the promise chain.
+
+        This patch also reimplements EventListenerSet to use multiple EventListeners,
+        since they share the same logic to uniformly handle Inspector and DOM events.
+
+        Test: inspector/event-listener.html
+        Test: inspector/event-listener-set.html
+
+        * UserInterface/Base/EventListener.js: Added.
+        (WebInspector.EventListener):
+        (WebInspector.EventListener.prototype.this._callback):
+        (WebInspector.EventListener.prototype.connect):
+        (WebInspector.EventListener.prototype.disconnect):
+        * UserInterface/Base/EventListenerSet.js: Update license block.
+        (WebInspector.EventListenerSet.prototype.register):
+        (WebInspector.EventListenerSet.prototype.install):
+        (WebInspector.EventListenerSet.prototype.uninstall):
+        * UserInterface/Main.html: Include EventListener.
+        * UserInterface/Test.html: Include EventListener and EventListenerSet.
+
</ins><span class="cx"> 2014-08-10  Timothy Hatcher  &lt;timothy@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: new glyphs are visible on OS X 10.9 builds
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceBaseEventListenerjs"></a>
<div class="addfile"><h4>Added: trunk/Source/WebInspectorUI/UserInterface/Base/EventListener.js (0 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Base/EventListener.js                                (rev 0)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/EventListener.js        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -0,0 +1,87 @@
</span><ins>+/*
+ * Copyright (C) 2013, 2014 University of Washington. All rights reserved.
+ * Copyright (C) 2014 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+WebInspector.EventListener = function(thisObject, fireOnce)
+{
+    this._thisObject = thisObject;
+    this._emitter = null;
+    this._callback = null;
+    this._fireOnce = fireOnce;
+}
+
+WebInspector.EventListener.prototype = {
+    connect: function(emitter, type, callback, usesCapture)
+    {
+        console.assert(!this._emitter &amp;&amp; !this._callback, &quot;EventListener already bound to a callback.&quot;, this);
+        console.assert(callback, &quot;Missing callback for event: &quot; + type);
+        console.assert(emitter, &quot;Missing event emitter for event: &quot; + type);
+        var emitterIsValid = emitter &amp;&amp; (emitter instanceof WebInspector.Object || emitter instanceof Node || (typeof emitter.addEventListener === &quot;function&quot;));
+        console.assert(emitterIsValid,  &quot;Event emitter &quot;, emitter, &quot; (type:&quot; + type + &quot;) is null or does not implement Node or WebInspector.Object!&quot;);
+
+        if (!emitterIsValid || !type || !callback)
+            return;
+
+        this._emitter = emitter;
+        this._type = type;
+        this._usesCapture = !!usesCapture;
+
+        if (emitter instanceof Node)
+            callback = callback.bind(this._thisObject);
+
+        if (this._fireOnce) {
+            var listener = this;
+            this._callback = function() {
+                listener.disconnect();
+                callback.apply(this, arguments);
+            }
+        } else
+            this._callback = callback;
+
+        if (this._emitter instanceof Node)
+            this._emitter.addEventListener(this._type, this._callback, this._usesCapture);
+        else
+            this._emitter.addEventListener(this._type, this._callback, this._thisObject);
+    },
+
+    disconnect: function()
+    {
+        console.assert(this._emitter &amp;&amp; this._callback, &quot;EventListener is not bound to a callback.&quot;, this);
+
+        if (!this._emitter || !this._callback)
+            return;
+
+        if (this._emitter instanceof Node)
+            this._emitter.removeEventListener(this._type, this._callback, this._usesCapture);
+        else
+            this._emitter.removeEventListener(this._type, this._callback, this._thisObject);
+
+        if (this._fireOnce)
+            delete this._thisObject;
+        delete this._emitter;
+        delete this._type;
+        delete this._callback;
+    }
+};
</ins></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceBaseEventListenerSetjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Base/EventListenerSet.js (172395 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Base/EventListenerSet.js        2014-08-11 17:52:15 UTC (rev 172395)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/EventListenerSet.js        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2013 University of Washington. All rights reserved.
</del><ins>+ * Copyright (C) 2013, 2014 University of Washington. All rights reserved.
</ins><span class="cx">  * Copyright (C) 2014 Apple Inc. All rights reserved.
</span><span class="cx">  *
</span><span class="cx">  * Redistribution and use in source and binary forms, with or without
</span><span class="lines">@@ -11,15 +11,15 @@
</span><span class="cx">  *    notice, this list of conditions and the following disclaimer in the
</span><span class="cx">  *    documentation and/or other materials provided with the distribution.
</span><span class="cx">  *
</span><del>- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS &quot;AS
- * IS&quot; AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
- * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
- * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
</del><ins>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
</ins><span class="cx">  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
</span><span class="cx">  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
</span><span class="cx">  */
</span><span class="lines">@@ -38,16 +38,17 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> WebInspector.EventListenerSet.prototype = {
</span><del>-    register: function(emitter, type, listener, thisObject, useCapture)
</del><ins>+    register: function(emitter, type, callback, thisObject, usesCapture)
</ins><span class="cx">     {
</span><del>-        console.assert(listener, &quot;Missing listener for event: &quot; + type);
-        console.assert(emitter, &quot;Missing event emitter for event: &quot; + type);
-        console.assert(emitter instanceof WebInspector.Object || emitter instanceof Node || (typeof emitter.addEventListener === &quot;function&quot;), &quot;Event emitter&quot;, emitter, &quot; (type:&quot; + type + &quot;) does not implement Node or WebInspector.Object!&quot;);
</del><ins>+        console.assert(callback, &quot;Missing callback for event: &quot; + type);
+        console.assert(type, &quot;Tried to register listener for unknown event: &quot; + type);
+        var emitterIsValid = emitter &amp;&amp; (emitter instanceof WebInspector.Object || emitter instanceof Node || (typeof emitter.addEventListener === &quot;function&quot;));
+        console.assert(emitterIsValid,  &quot;Event emitter &quot;, emitter, &quot; (type:&quot; + type + &quot;) is null or does not implement Node or WebInspector.Object!&quot;);
</ins><span class="cx"> 
</span><del>-        if (emitter instanceof Node)
-            listener = listener.bind(thisObject || this._defaultThisObject);
</del><ins>+        if (!emitterIsValid || !type || !callback)
+            return;
</ins><span class="cx"> 
</span><del>-        this._listeners.push({emitter: emitter, type: type, listener: listener, thisObject: thisObject, useCapture: useCapture});
</del><ins>+        this._listeners.push({listener: new WebInspector.EventListener(thisObject || this._defaultThisObject), emitter: emitter, type: type, callback: callback, usesCapture: usesCapture});
</ins><span class="cx">     },
</span><span class="cx"> 
</span><span class="cx">     unregister: function()
</span><span class="lines">@@ -60,33 +61,27 @@
</span><span class="cx">     install: function()
</span><span class="cx">     {
</span><span class="cx">         console.assert(!this._installed, &quot;Already installed listener group: &quot; + this.name);
</span><ins>+        if (this._installed)
+            return;
</ins><span class="cx"> 
</span><span class="cx">         this._installed = true;
</span><span class="cx"> 
</span><del>-        for (var listenerData of this._listeners) {
-            if (listenerData.emitter instanceof Node)
-                listenerData.emitter.addEventListener(listenerData.type, listenerData.listener, listenerData.useCapture);
-            else
-                listenerData.emitter.addEventListener(listenerData.type, listenerData.listener, listenerData.thisObject || this._defaultThisObject);
-        }
</del><ins>+        for (var data of this._listeners)
+            data.listener.connect(data.emitter, data.type, data.callback, data.usesCapture);
</ins><span class="cx">     },
</span><span class="cx"> 
</span><span class="cx">     uninstall: function(unregisterListeners)
</span><span class="cx">     {
</span><span class="cx">         console.assert(this._installed, &quot;Trying to uninstall listener group &quot; + this.name + &quot;, but it isn't installed.&quot;);
</span><ins>+        if (!this._installed)
+            return;
</ins><span class="cx"> 
</span><span class="cx">         this._installed = false;
</span><span class="cx"> 
</span><del>-        for (var listenerData of this._listeners) {
-            if (listenerData.emitter instanceof Node)
-                listenerData.emitter.removeEventListener(listenerData.type, listenerData.listener, listenerData.useCapture);
-            else
-                listenerData.emitter.removeEventListener(listenerData.type, listenerData.listener, listenerData.thisObject || this._defaultThisObject);
-        }
</del><ins>+        for (var data of this._listeners)
+            data.listener.disconnect();
</ins><span class="cx"> 
</span><del>-        if (unregisterListeners) {
</del><ins>+        if (unregisterListeners)
</ins><span class="cx">             this._listeners = [];
</span><del>-            delete this._defaultThisObject;
-        }
</del><span class="cx">     },
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceMainhtml"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Main.html (172395 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Main.html        2014-08-11 17:52:15 UTC (rev 172395)
+++ trunk/Source/WebInspectorUI/UserInterface/Main.html        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -165,6 +165,7 @@
</span><span class="cx">     &lt;script src=&quot;Base/Object.js&quot;&gt;&lt;/script&gt;
</span><span class="cx"> 
</span><span class="cx">     &lt;script src=&quot;Base/DOMUtilities.js&quot;&gt;&lt;/script&gt;
</span><ins>+    &lt;script src=&quot;Base/EventListener.js&quot;&gt;&lt;/script&gt;
</ins><span class="cx">     &lt;script src=&quot;Base/EventListenerSet.js&quot;&gt;&lt;/script&gt;
</span><span class="cx">     &lt;script src=&quot;Base/ImageUtilities.js&quot;&gt;&lt;/script&gt;
</span><span class="cx">     &lt;script src=&quot;Base/LoadLocalizedStrings.js&quot;&gt;&lt;/script&gt;
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceTesthtml"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Test.html (172395 => 172396)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Test.html        2014-08-11 17:52:15 UTC (rev 172395)
+++ trunk/Source/WebInspectorUI/UserInterface/Test.html        2014-08-11 18:10:25 UTC (rev 172396)
</span><span class="lines">@@ -35,6 +35,8 @@
</span><span class="cx">     &lt;script src=&quot;Base/Test.js&quot;&gt;&lt;/script&gt;
</span><span class="cx"> 
</span><span class="cx">     &lt;script src=&quot;Base/DOMUtilities.js&quot;&gt;&lt;/script&gt;
</span><ins>+    &lt;script src=&quot;Base/EventListener.js&quot;&gt;&lt;/script&gt;
+    &lt;script src=&quot;Base/EventListenerSet.js&quot;&gt;&lt;/script&gt;
</ins><span class="cx">     &lt;script src=&quot;Base/URLUtilities.js&quot;&gt;&lt;/script&gt;
</span><span class="cx">     &lt;script src=&quot;Base/Utilities.js&quot;&gt;&lt;/script&gt;
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>