<!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>[185023] 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/185023">185023</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2015-05-29 17:26:43 -0700 (Fri, 29 May 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>WeakMap reference w/ DOM element as key does not survive long enough.
https://bugs.webkit.org/show_bug.cgi?id=137651
Patch by Keith Miller <keith_miller@apple.com> on 2015-05-29
Reviewed by Geoffrey Garen.
Source/WebCore:
Remove isObservable functions as an "unobservable wrappers"
optimization is invalid with WeakMaps. Performance testing
will be done after the code is commited. If major
performance issues occur the patch will be rolled out.
Test: js/dom/weakmap-gc-unobservable-dom-nodes.html
* bindings/js/JSNodeCustom.cpp:
(WebCore::isReachableFromDOM):
(WebCore::JSNodeOwner::isReachableFromOpaqueRoots):
(WebCore::JSNode::insertBefore):
(WebCore::isObservable): Deleted.
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation):
LayoutTests:
* js/dom/script-tests/weakmap-gc-unobservable-dom-nodes.js: Added.
(.set gc):
* js/dom/weakmap-gc-unobservable-dom-nodes.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="#trunkSourceWebCorebindingsjsJSNodeCustomcpp">trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsscriptsCodeGeneratorJSpm">trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsjsdomscripttestsweakmapgcunobservabledomnodesjs">trunk/LayoutTests/js/dom/script-tests/weakmap-gc-unobservable-dom-nodes.js</a></li>
<li><a href="#trunkLayoutTestsjsdomweakmapgcunobservabledomnodesexpectedtxt">trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes-expected.txt</a></li>
<li><a href="#trunkLayoutTestsjsdomweakmapgcunobservabledomnodeshtml">trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (185022 => 185023)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-05-30 00:19:01 UTC (rev 185022)
+++ trunk/LayoutTests/ChangeLog 2015-05-30 00:26:43 UTC (rev 185023)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+2015-05-29 Keith Miller <keith_miller@apple.com>
+
+ WeakMap reference w/ DOM element as key does not survive long enough.
+ https://bugs.webkit.org/show_bug.cgi?id=137651
+
+ Reviewed by Geoffrey Garen.
+
+ * js/dom/script-tests/weakmap-gc-unobservable-dom-nodes.js: Added.
+ (.set gc):
+ * js/dom/weakmap-gc-unobservable-dom-nodes.html: Added.
+
</ins><span class="cx"> 2015-05-29 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Text disappears shortly after page load on Nexus 7 site.
</span></span></pre></div>
<a id="trunkLayoutTestsjsdomscripttestsweakmapgcunobservabledomnodesjs"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/dom/script-tests/weakmap-gc-unobservable-dom-nodes.js (0 => 185023)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/dom/script-tests/weakmap-gc-unobservable-dom-nodes.js (rev 0)
+++ trunk/LayoutTests/js/dom/script-tests/weakmap-gc-unobservable-dom-nodes.js 2015-05-30 00:26:43 UTC (rev 185023)
</span><span class="lines">@@ -0,0 +1,15 @@
</span><ins>+var wmap = new WeakMap();
+
+// Need to add several elements so the bug manifests.
+for (var i = 0; i < 5; i++) {
+ document.body.appendChild(document.createElement('p'));
+ wmap.set(document.body.lastElementChild, 4);
+}
+
+gc();
+
+Array.prototype.forEach.call(
+ document.querySelectorAll('p'), function(el) {
+ shouldBeDefined(wmap.get(el));
+ });
+
</ins></span></pre></div>
<a id="trunkLayoutTestsjsdomweakmapgcunobservabledomnodesexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes-expected.txt (0 => 185023)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes-expected.txt (rev 0)
+++ trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes-expected.txt 2015-05-30 00:26:43 UTC (rev 185023)
</span><span class="lines">@@ -0,0 +1,9 @@
</span><ins>+PASS 4 is defined.
+PASS 4 is defined.
+PASS 4 is defined.
+PASS 4 is defined.
+PASS 4 is defined.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsjsdomweakmapgcunobservabledomnodeshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes.html (0 => 185023)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes.html (rev 0)
+++ trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes.html 2015-05-30 00:26:43 UTC (rev 185023)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/weakmap-gc-unobservable-dom-nodes.js"></script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (185022 => 185023)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-05-30 00:19:01 UTC (rev 185022)
+++ trunk/Source/WebCore/ChangeLog 2015-05-30 00:26:43 UTC (rev 185023)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2015-05-29 Keith Miller <keith_miller@apple.com>
+
+ WeakMap reference w/ DOM element as key does not survive long enough.
+ https://bugs.webkit.org/show_bug.cgi?id=137651
+
+ Reviewed by Geoffrey Garen.
+
+ Remove isObservable functions as an "unobservable wrappers"
+ optimization is invalid with WeakMaps. Performance testing
+ will be done after the code is commited. If major
+ performance issues occur the patch will be rolled out.
+
+ Test: js/dom/weakmap-gc-unobservable-dom-nodes.html
+
+ * bindings/js/JSNodeCustom.cpp:
+ (WebCore::isReachableFromDOM):
+ (WebCore::JSNodeOwner::isReachableFromOpaqueRoots):
+ (WebCore::JSNode::insertBefore):
+ (WebCore::isObservable): Deleted.
+ * bindings/scripts/CodeGeneratorJS.pm:
+ (GenerateImplementation):
+
</ins><span class="cx"> 2015-05-29 Anders Carlsson <andersca@apple.com>
</span><span class="cx">
</span><span class="cx"> Get rid of WAKViewPrivate.h
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSNodeCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp (185022 => 185023)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp 2015-05-30 00:19:01 UTC (rev 185022)
+++ trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp 2015-05-30 00:26:43 UTC (rev 185023)
</span><span class="lines">@@ -76,24 +76,8 @@
</span><span class="cx">
</span><span class="cx"> using namespace HTMLNames;
</span><span class="cx">
</span><del>-static inline bool isObservable(JSNode* jsNode, Node* node)
</del><ins>+static inline bool isReachableFromDOM(Node* node, SlotVisitor& visitor)
</ins><span class="cx"> {
</span><del>- // The root node keeps the tree intact.
- if (!node->parentNode())
- return true;
-
- if (jsNode->hasCustomProperties())
- return true;
-
- // A node's JS wrapper is responsible for marking its JS event listeners.
- if (node->hasEventListeners())
- return true;
-
- return false;
-}
-
-static inline bool isReachableFromDOM(JSNode* jsNode, Node* node, SlotVisitor& visitor)
-{
</del><span class="cx"> if (!node->inDocument()) {
</span><span class="cx"> if (is<Element>(*node)) {
</span><span class="cx"> auto& element = downcast<Element>(*node);
</span><span class="lines">@@ -121,13 +105,13 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- return isObservable(jsNode, node) && visitor.containsOpaqueRoot(root(node));
</del><ins>+ return visitor.containsOpaqueRoot(root(node));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool JSNodeOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)
</span><span class="cx"> {
</span><span class="cx"> JSNode* jsNode = jsCast<JSNode*>(handle.slot()->asCell());
</span><del>- return isReachableFromDOM(jsNode, &jsNode->impl(), visitor);
</del><ins>+ return isReachableFromDOM(&jsNode->impl(), visitor);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> JSValue JSNode::insertBefore(ExecState* exec)
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsscriptsCodeGeneratorJSpm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (185022 => 185023)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2015-05-30 00:19:01 UTC (rev 185022)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2015-05-30 00:26:43 UTC (rev 185023)
</span><span class="lines">@@ -2927,18 +2927,6 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if ((!$hasParent && !GetCustomIsReachable($interface)) || GetGenerateIsReachable($interface) || $codeGenerator->InheritsExtendedAttribute($interface, "ActiveDOMObject")) {
</span><del>- if (GetGenerateIsReachable($interface)) {
- push(@implContent, "static inline bool isObservable(JS${interfaceName}* js${interfaceName})\n");
- push(@implContent, "{\n");
- push(@implContent, " if (js${interfaceName}->hasCustomProperties())\n");
- push(@implContent, " return true;\n");
- if ($eventTarget) {
- push(@implContent, " if (js${interfaceName}->impl().hasEventListeners())\n");
- push(@implContent, " return true;\n");
- }
- push(@implContent, " return false;\n");
- push(@implContent, "}\n\n");
- }
</del><span class="cx">
</span><span class="cx"> push(@implContent, "bool JS${interfaceName}Owner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)\n");
</span><span class="cx"> push(@implContent, "{\n");
</span><span class="lines">@@ -2948,7 +2936,7 @@
</span><span class="cx"> # their pending activities complete. To wallpaper over this bug, JavaScript
</span><span class="cx"> # wrappers unconditionally keep ActiveDOMObjects with pending activity alive.
</span><span class="cx"> # FIXME: Fix this lifetime issue in the DOM, and move this hasPendingActivity
</span><del>- # check below the isObservable check.
</del><ins>+ # check just above the (GetGenerateIsReachable($interface) eq "Impl") check below.
</ins><span class="cx"> my $emittedJSCast = 0;
</span><span class="cx"> if ($codeGenerator->InheritsExtendedAttribute($interface, "ActiveDOMObject")) {
</span><span class="cx"> push(@implContent, " auto* js${interfaceName} = jsCast<JS${interfaceName}*>(handle.slot()->asCell());\n");
</span><span class="lines">@@ -2977,8 +2965,6 @@
</span><span class="cx"> push(@implContent, " auto* js${interfaceName} = jsCast<JS${interfaceName}*>(handle.slot()->asCell());\n");
</span><span class="cx"> $emittedJSCast = 1;
</span><span class="cx"> }
</span><del>- push(@implContent, " if (!isObservable(js${interfaceName}))\n");
- push(@implContent, " return false;\n");
</del><span class="cx">
</span><span class="cx"> my $rootString;
</span><span class="cx"> if (GetGenerateIsReachable($interface) eq "Impl") {
</span></span></pre>
</div>
</div>
</body>
</html>