[V8] HTMLCollection wrappers are not retained
authorarv@chromium.org <arv@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Jun 2012 03:32:05 +0000 (03:32 +0000)
committerarv@chromium.org <arv@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Jun 2012 03:32:05 +0000 (03:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90208

Reviewed by Adam Barth.

Source/WebCore:

Generate visitDOMWrapper for HTMLCollection and HTMLAllCollection so that we add an implicit reference from the owner
to the collection.

Tests: fast/dom/htmlallcollection-reachable.html
       fast/dom/htmlcollection-reachable.html

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation): Instead of hard coding to use base() for HTMLAllCollection and HTMLCollection we now
annotate the IDL file to use GenerateIsReachable=ImplBaseRoot.
* bindings/scripts/CodeGeneratorV8.pm:
(GenerateVisitDOMWrapper): Generate visitDOMWrapper if GenerateIsReachable is ImplBaseRoot.
* bindings/scripts/IDLAttributes.txt: Added ImplBaseRoot.
* html/HTMLAllCollection.idl: Added annotations.
* html/HTMLCollection.idl: Ditto.

LayoutTests:

* fast/dom/htmlallcollection-reachable-expected.txt: Added.
* fast/dom/htmlallcollection-reachable.html: Added.
* fast/dom/htmlcollection-reachable-expected.txt: Added.
* fast/dom/htmlcollection-reachable.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121615 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/htmlallcollection-reachable-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/htmlallcollection-reachable.html [new file with mode: 0644]
LayoutTests/fast/dom/htmlcollection-reachable-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/htmlcollection-reachable.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
Source/WebCore/bindings/scripts/IDLAttributes.txt
Source/WebCore/html/HTMLAllCollection.idl
Source/WebCore/html/HTMLCollection.idl

index f0832c7..2afc323 100644 (file)
@@ -1,3 +1,15 @@
+2012-06-29  Erik Arvidsson  <arv@chromium.org>
+
+        [V8] HTMLCollection wrappers are not retained
+        https://bugs.webkit.org/show_bug.cgi?id=90208
+
+        Reviewed by Adam Barth.
+
+        * fast/dom/htmlallcollection-reachable-expected.txt: Added.
+        * fast/dom/htmlallcollection-reachable.html: Added.
+        * fast/dom/htmlcollection-reachable-expected.txt: Added.
+        * fast/dom/htmlcollection-reachable.html: Added.
+
 2012-06-29  Tony Chang  <tony@chromium.org>
 
         All child elements of a flex container should be turned into a flex item
diff --git a/LayoutTests/fast/dom/htmlallcollection-reachable-expected.txt b/LayoutTests/fast/dom/htmlallcollection-reachable-expected.txt
new file mode 100644 (file)
index 0000000..89297e2
--- /dev/null
@@ -0,0 +1,5 @@
+PASS document.all.customProperty is 42
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/htmlallcollection-reachable.html b/LayoutTests/fast/dom/htmlallcollection-reachable.html
new file mode 100644 (file)
index 0000000..1e54fd2
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+
+document.all.customProperty = 42;
+gc();
+shouldBe('document.all.customProperty', '42');
+
+</script>
+<script src="../js/resources/js-test-post.js"></script>
diff --git a/LayoutTests/fast/dom/htmlcollection-reachable-expected.txt b/LayoutTests/fast/dom/htmlcollection-reachable-expected.txt
new file mode 100644 (file)
index 0000000..113bf67
--- /dev/null
@@ -0,0 +1,21 @@
+PASS document.anchors.customProperty is 0
+PASS document.applets.customProperty is 1
+PASS document.embeds.customProperty is 2
+PASS document.forms.customProperty is 3
+PASS document.images.customProperty is 4
+PASS document.links.customProperty is 5
+PASS document.plugins.customProperty is 6
+PASS document.scripts.customProperty is 7
+PASS element.children.customProperty is 8
+PASS fieldset.elements.customProperty is 9
+PASS form.elements.customProperty is 10
+PASS map.areas.customProperty is 11
+PASS section.rows.customProperty is 12
+PASS select.selectedOptions.customProperty is 13
+PASS table.rows.customProperty is 14
+PASS table.tBodies.customProperty is 15
+PASS tableRow.cells.customProperty is 16
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/htmlcollection-reachable.html b/LayoutTests/fast/dom/htmlcollection-reachable.html
new file mode 100644 (file)
index 0000000..dfd2e30
--- /dev/null
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<script src="../js/resources/js-test-pre.js"></script>
+<form></form>
+<table><tbody><tr></tr></tbody></table>
+<fieldset></fieldset>
+<select></select>
+<datalist></datalist>
+<map></map>
+<script>
+
+var datalist = document.querySelector('datalist');
+var element = document.createElement('span');
+var fieldset = document.querySelector('fieldset');
+var form = document.querySelector('form');
+var map = document.querySelector('map');
+var section = document.querySelector('tbody');
+var select = document.querySelector('select');
+var table = document.querySelector('table');
+var tableRow = document.querySelector('tr');
+
+var collections = [
+    // datalist is not enabled by default.
+    // 'datalist.options',
+    'document.anchors',
+    'document.applets',
+    'document.embeds',
+    'document.forms',
+    'document.images',
+    'document.links',
+    'document.plugins',
+    'document.scripts',
+    'element.children',
+    'fieldset.elements',
+    'form.elements',
+    'map.areas',
+    'section.rows',
+    'select.selectedOptions',
+    'table.rows',
+    'table.tBodies',
+    'tableRow.cells',
+];
+
+for (var i = 0; i < collections.length; i++) {
+    var code = collections[i];
+    eval(code).customProperty = i;
+    gc();
+    shouldBe(code + '.customProperty', '' + i);
+}
+
+</script>
+<script src="../js/resources/js-test-post.js"></script>
index b07c002..7e52779 100644 (file)
@@ -1,3 +1,25 @@
+2012-06-29  Erik Arvidsson  <arv@chromium.org>
+
+        [V8] HTMLCollection wrappers are not retained
+        https://bugs.webkit.org/show_bug.cgi?id=90208
+
+        Reviewed by Adam Barth.
+
+        Generate visitDOMWrapper for HTMLCollection and HTMLAllCollection so that we add an implicit reference from the owner
+        to the collection.
+
+        Tests: fast/dom/htmlallcollection-reachable.html
+               fast/dom/htmlcollection-reachable.html
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateImplementation): Instead of hard coding to use base() for HTMLAllCollection and HTMLCollection we now
+        annotate the IDL file to use GenerateIsReachable=ImplBaseRoot.
+        * bindings/scripts/CodeGeneratorV8.pm:
+        (GenerateVisitDOMWrapper): Generate visitDOMWrapper if GenerateIsReachable is ImplBaseRoot.
+        * bindings/scripts/IDLAttributes.txt: Added ImplBaseRoot.
+        * html/HTMLAllCollection.idl: Added annotations.
+        * html/HTMLCollection.idl: Ditto.
+
 2012-06-29  Tony Chang  <tony@chromium.org>
 
         All child elements of a flex container should be turned into a flex item
index bcf567e..527e193 100644 (file)
@@ -2346,7 +2346,7 @@ sub GenerateImplementation
                 $rootString .= "    void* root = WebCore::root(element);\n";
             } elsif ($interfaceName eq "CanvasRenderingContext") {
                 $rootString  = "    void* root = WebCore::root(js${implClassName}->impl()->canvas());\n";
-            } elsif ($interfaceName eq "HTMLCollection" or $interfaceName eq "HTMLAllCollection") {
+            } elsif (GetGenerateIsReachable($dataNode) eq "ImplBaseRoot") {
                 $rootString  = "    void* root = WebCore::root(js${implClassName}->impl()->base());\n";
             } else {
                 $rootString  = "    void* root = WebCore::root(js${implClassName}->impl());\n";
index b36da40..90ef218 100644 (file)
@@ -214,12 +214,14 @@ void V8${implClassName}::visitDOMWrapper(DOMDataStore* store, void* object, v8::
 END
     if (GetGenerateIsReachable($dataNode) eq  "ImplElementRoot" ||
         GetGenerateIsReachable($dataNode) eq  "ImplOwnerRoot" ||
-        GetGenerateIsReachable($dataNode) eq  "ImplOwnerNodeRoot") {
+        GetGenerateIsReachable($dataNode) eq  "ImplOwnerNodeRoot" ||
+        GetGenerateIsReachable($dataNode) eq  "ImplBaseRoot") {
 
         my $methodName;
         $methodName = "element" if (GetGenerateIsReachable($dataNode) eq "ImplElementRoot");
         $methodName = "owner" if (GetGenerateIsReachable($dataNode) eq "ImplOwnerRoot");
         $methodName = "ownerNode" if (GetGenerateIsReachable($dataNode) eq "ImplOwnerNodeRoot");
+        $methodName = "base" if (GetGenerateIsReachable($dataNode) eq "ImplBaseRoot");
 
         push(@implContent, <<END);
     if (Node* owner = impl->${methodName}()) {
index 4fe771b..b6d890e 100644 (file)
@@ -52,7 +52,7 @@ DoNotCheckSecurityOnGetter
 DoNotCheckSecurityOnSetter
 EventTarget
 ExtendsDOMGlobalObject
-GenerateIsReachable=|Impl|ImplContext|ImplDocument|ImplElementRoot|ImplFrame
+GenerateIsReachable=ImplElementRoot|ImplBaseRoot
 Immutable
 ImplementedAs=*
 IndexedGetter
@@ -75,7 +75,7 @@ JSCustomPushEventHandlerScope
 JSCustomSetter
 JSCustomToJSObject
 JSCustomToNativeObject
-JSGenerateIsReachable=|Impl|ImplContext|ImplDocument|ImplElementRoot|ImplFrame
+JSGenerateIsReachable=|Impl|ImplContext|ImplDocument|ImplElementRoot|ImplFrame|ImplBaseRoot
 JSGenerateToJSObject
 JSGenerateToNativeObject
 JSInlineGetOwnPropertySlot
@@ -116,6 +116,6 @@ V8DependentLifetime
 V8DoNotCheckSignature
 V8EnabledAtRuntime=*
 V8EnabledPerContext=*
-V8GenerateIsReachable=|ImplElementRoot|ImplOwnerRoot|ImplOwnerNodeRoot
+V8GenerateIsReachable=ImplElementRoot|ImplOwnerRoot|ImplOwnerNodeRoot|ImplBaseRoot
 V8ReadOnly
 V8Unforgeable
index 5ceae01..50e189c 100644 (file)
@@ -30,7 +30,8 @@ module html {
         NamedGetter,
         CustomCall,
         MasqueradesAsUndefined,
-        JSGenerateIsReachable
+        GenerateIsReachable=ImplBaseRoot,
+        V8DependentLifetime
     ] HTMLAllCollection {
         readonly attribute unsigned long length;
         [Custom] Node item(in [Optional=DefaultIsUndefined] unsigned long index);
index 7783776..efd91c7 100644 (file)
@@ -24,7 +24,8 @@ module html {
         IndexedGetter,
         NamedGetter,
         CustomToJSObject,
-        JSGenerateIsReachable,
+        GenerateIsReachable=ImplBaseRoot,
+        V8DependentLifetime,
         ObjCPolymorphic
     ] HTMLCollection {
         readonly attribute unsigned long length;