<!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>[211238] trunk/Source/WebCore</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/211238">211238</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2017-01-26 16:11:54 -0800 (Thu, 26 Jan 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>EventTarget should visit the JSEventListeners using visitAdditionalChildren
https://bugs.webkit.org/show_bug.cgi?id=167462
Reviewed by Michael Saboff.
No new tests because this is already caught by existing testing. This would show up as ASSERTs
in debug, and we suspect it might be at fault for null deref crashes.
Previously, EventTarget would have its event listeners visited by its subclasses' visitChildren
methods. Every subclass of EventTarget would call EventTarget's visitJSEventListeners. For
example, this means that if JSFoo has seven classes between it and JSEventTarget in the JSCell
class hierarchy, then JSFoo::visitChildren would end up calling visitJSEventListeners seven extra
times.
Also, the weird way that visitJSEventListeners was called meant that it was not part of the GC's
output constraint processing. This meant that it would not be called when the GC tried to
terminate. So, if something about the event listener changes during a GC cycle, the GC would
potentially fail to mark one of the references.
Both problems can be solved by simply moving the call to visitJSEventListeners into
visitAdditionalChildren.
* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::visitAdditionalChildren):
* bindings/js/JSEventTargetCustom.cpp:
(WebCore::JSEventTarget::visitAdditionalChildren):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation):
* dom/EventTarget.idl:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSDOMWindowCustomcpp">trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSEventTargetCustomcpp">trunk/Source/WebCore/bindings/js/JSEventTargetCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSWorkerGlobalScopeCustomcpp">trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsscriptsCodeGeneratorJSpm">trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm</a></li>
<li><a href="#trunkSourceWebCoredomEventTargetidl">trunk/Source/WebCore/dom/EventTarget.idl</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (211237 => 211238)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-01-26 23:50:58 UTC (rev 211237)
+++ trunk/Source/WebCore/ChangeLog 2017-01-27 00:11:54 UTC (rev 211238)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2017-01-26 Filip Pizlo <fpizlo@apple.com>
+
+ EventTarget should visit the JSEventListeners using visitAdditionalChildren
+ https://bugs.webkit.org/show_bug.cgi?id=167462
+
+ Reviewed by Michael Saboff.
+
+ No new tests because this is already caught by existing testing. This would show up as ASSERTs
+ in debug, and we suspect it might be at fault for null deref crashes.
+
+ Previously, EventTarget would have its event listeners visited by its subclasses' visitChildren
+ methods. Every subclass of EventTarget would call EventTarget's visitJSEventListeners. For
+ example, this means that if JSFoo has seven classes between it and JSEventTarget in the JSCell
+ class hierarchy, then JSFoo::visitChildren would end up calling visitJSEventListeners seven extra
+ times.
+
+ Also, the weird way that visitJSEventListeners was called meant that it was not part of the GC's
+ output constraint processing. This meant that it would not be called when the GC tried to
+ terminate. So, if something about the event listener changes during a GC cycle, the GC would
+ potentially fail to mark one of the references.
+
+ Both problems can be solved by simply moving the call to visitJSEventListeners into
+ visitAdditionalChildren.
+
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::visitAdditionalChildren):
+ * bindings/js/JSEventTargetCustom.cpp:
+ (WebCore::JSEventTarget::visitAdditionalChildren):
+ * bindings/scripts/CodeGeneratorJS.pm:
+ (GenerateImplementation):
+ * dom/EventTarget.idl:
+
</ins><span class="cx"> 2017-01-26 Andy Estes <aestes@apple.com>
</span><span class="cx">
</span><span class="cx"> [QuickLook] Create temporary files with NSFileProtectionCompleteUnlessOpen
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSDOMWindowCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (211237 => 211238)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2017-01-26 23:50:58 UTC (rev 211237)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2017-01-27 00:11:54 UTC (rev 211238)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2007-2010, 2016 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2007-2017 Apple Inc. All rights reserved.
</ins><span class="cx"> * Copyright (C) 2011 Google Inc. All rights reserved.
</span><span class="cx"> *
</span><span class="cx"> * This library is free software; you can redistribute it and/or
</span><span class="lines">@@ -52,6 +52,11 @@
</span><span class="cx"> {
</span><span class="cx"> if (Frame* frame = wrapped().frame())
</span><span class="cx"> visitor.addOpaqueRoot(frame);
</span><ins>+
+ // Normally JSEventTargetCustom.cpp's JSEventTarget::visitAdditionalChildren() would call this. But
+ // even though DOMWindow is an EventTarget, JSDOMWindow does not subclass JSEventTarget, so we need
+ // to do this here.
+ wrapped().visitJSEventListeners(visitor);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if ENABLE(USER_MESSAGE_HANDLERS)
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSEventTargetCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSEventTargetCustom.cpp (211237 => 211238)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSEventTargetCustom.cpp 2017-01-26 23:50:58 UTC (rev 211237)
+++ trunk/Source/WebCore/bindings/js/JSEventTargetCustom.cpp 2017-01-27 00:11:54 UTC (rev 211238)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2008, 2016 Apple Inc. All Rights Reserved.
</del><ins>+ * Copyright (C) 2008-2017 Apple Inc. All Rights Reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -82,4 +82,9 @@
</span><span class="cx"> return nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void JSEventTarget::visitAdditionalChildren(SlotVisitor& visitor)
+{
+ wrapped().visitJSEventListeners(visitor);
+}
+
</ins><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSWorkerGlobalScopeCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp (211237 => 211238)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp 2017-01-26 23:50:58 UTC (rev 211237)
+++ trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp 2017-01-27 00:11:54 UTC (rev 211238)
</span><span class="lines">@@ -42,6 +42,11 @@
</span><span class="cx"> visitor.addOpaqueRoot(navigator);
</span><span class="cx"> ScriptExecutionContext& context = wrapped();
</span><span class="cx"> visitor.addOpaqueRoot(&context);
</span><ins>+
+ // Normally JSEventTargetCustom.cpp's JSEventTarget::visitAdditionalChildren() would call this. But
+ // even though WorkerGlobalScope is an EventTarget, JSWorkerGlobalScope does not subclass
+ // JSEventTarget, so we need to do this here.
+ wrapped().visitJSEventListeners(visitor);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> JSValue JSWorkerGlobalScope::setTimeout(ExecState& state)
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsscriptsCodeGeneratorJSpm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (211237 => 211238)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2017-01-26 23:50:58 UTC (rev 211237)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2017-01-27 00:11:54 UTC (rev 211238)
</span><span class="lines">@@ -4158,9 +4158,6 @@
</span><span class="cx"> push(@implContent, " auto* thisObject = jsCast<${className}*>(cell);\n");
</span><span class="cx"> push(@implContent, " ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n");
</span><span class="cx"> push(@implContent, " Base::visitChildren(thisObject, visitor);\n");
</span><del>- if ($codeGenerator->InheritsInterface($interface, "EventTarget")) {
- push(@implContent, " thisObject->wrapped().visitJSEventListeners(visitor);\n");
- }
</del><span class="cx"> push(@implContent, " thisObject->visitAdditionalChildren(visitor);\n") if $interface->extendedAttributes->{JSCustomMarkFunction};
</span><span class="cx"> if ($interface->extendedAttributes->{ReportExtraMemoryCost}) {
</span><span class="cx"> push(@implContent, " visitor.reportExtraMemoryVisited(thisObject->wrapped().memoryCost());\n");
</span></span></pre></div>
<a id="trunkSourceWebCoredomEventTargetidl"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/EventTarget.idl (211237 => 211238)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/EventTarget.idl 2017-01-26 23:50:58 UTC (rev 211237)
+++ trunk/Source/WebCore/dom/EventTarget.idl 2017-01-27 00:11:54 UTC (rev 211238)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
</ins><span class="cx"> * Copyright (C) 2006 Samuel Weinig <sam.weinig@gmail.com>
</span><span class="cx"> *
</span><span class="cx"> * This library is free software; you can redistribute it and/or
</span><span class="lines">@@ -23,6 +23,7 @@
</span><span class="cx"> Exposed=(Window,Worker),
</span><span class="cx"> IsImmutablePrototypeExoticObjectOnPrototype,
</span><span class="cx"> JSCustomHeader,
</span><ins>+ JSCustomMarkFunction,
</ins><span class="cx"> JSCustomToNativeObject,
</span><span class="cx"> ] interface EventTarget {
</span><span class="cx"> [ImplementedAs=addEventListenerForBindings] void addEventListener([AtomicString] DOMString type, EventListener? callback, optional (AddEventListenerOptions or boolean) options = false);
</span></span></pre>
</div>
</div>
</body>
</html>