Diff
Modified: trunk/LayoutTests/ChangeLog (185022 => 185023)
--- trunk/LayoutTests/ChangeLog 2015-05-30 00:19:01 UTC (rev 185022)
+++ trunk/LayoutTests/ChangeLog 2015-05-30 00:26:43 UTC (rev 185023)
@@ -1,3 +1,14 @@
+2015-05-29 Keith Miller <[email protected]>
+
+ 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.
+
2015-05-29 Zalan Bujtas <[email protected]>
Text disappears shortly after page load on Nexus 7 site.
Added: trunk/LayoutTests/js/dom/script-tests/weakmap-gc-unobservable-dom-nodes.js (0 => 185023)
--- 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)
@@ -0,0 +1,15 @@
+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));
+ });
+
Added: trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes-expected.txt (0 => 185023)
--- 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)
@@ -0,0 +1,9 @@
+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
+
Added: trunk/LayoutTests/js/dom/weakmap-gc-unobservable-dom-nodes.html (0 => 185023)
--- 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)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (185022 => 185023)
--- 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)
@@ -1,3 +1,25 @@
+2015-05-29 Keith Miller <[email protected]>
+
+ 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):
+
2015-05-29 Anders Carlsson <[email protected]>
Get rid of WAKViewPrivate.h
Modified: trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp (185022 => 185023)
--- 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)
@@ -76,24 +76,8 @@
using namespace HTMLNames;
-static inline bool isObservable(JSNode* jsNode, Node* node)
+static inline bool isReachableFromDOM(Node* node, SlotVisitor& visitor)
{
- // 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)
-{
if (!node->inDocument()) {
if (is<Element>(*node)) {
auto& element = downcast<Element>(*node);
@@ -121,13 +105,13 @@
return true;
}
- return isObservable(jsNode, node) && visitor.containsOpaqueRoot(root(node));
+ return visitor.containsOpaqueRoot(root(node));
}
bool JSNodeOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)
{
JSNode* jsNode = jsCast<JSNode*>(handle.slot()->asCell());
- return isReachableFromDOM(jsNode, &jsNode->impl(), visitor);
+ return isReachableFromDOM(&jsNode->impl(), visitor);
}
JSValue JSNode::insertBefore(ExecState* exec)
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (185022 => 185023)
--- 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)
@@ -2927,18 +2927,6 @@
}
if ((!$hasParent && !GetCustomIsReachable($interface)) || GetGenerateIsReachable($interface) || $codeGenerator->InheritsExtendedAttribute($interface, "ActiveDOMObject")) {
- 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");
- }
push(@implContent, "bool JS${interfaceName}Owner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)\n");
push(@implContent, "{\n");
@@ -2948,7 +2936,7 @@
# their pending activities complete. To wallpaper over this bug, _javascript_
# wrappers unconditionally keep ActiveDOMObjects with pending activity alive.
# FIXME: Fix this lifetime issue in the DOM, and move this hasPendingActivity
- # check below the isObservable check.
+ # check just above the (GetGenerateIsReachable($interface) eq "Impl") check below.
my $emittedJSCast = 0;
if ($codeGenerator->InheritsExtendedAttribute($interface, "ActiveDOMObject")) {
push(@implContent, " auto* js${interfaceName} = jsCast<JS${interfaceName}*>(handle.slot()->asCell());\n");
@@ -2977,8 +2965,6 @@
push(@implContent, " auto* js${interfaceName} = jsCast<JS${interfaceName}*>(handle.slot()->asCell());\n");
$emittedJSCast = 1;
}
- push(@implContent, " if (!isObservable(js${interfaceName}))\n");
- push(@implContent, " return false;\n");
my $rootString;
if (GetGenerateIsReachable($interface) eq "Impl") {
