Cache and reuse the HTMLFormElement.elements collection.
authorkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Dec 2011 20:15:16 +0000 (20:15 +0000)
committerkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Dec 2011 20:15:16 +0000 (20:15 +0000)
<http://webkit.org/b/75375>

Reviewed by Anders Carlsson.

Source/WebCore:

Let HTMLFormElement::elements() cache the returned collection and tie it to the
lifetime of the form. This reduces memory consumption by ~70 kB (on 64-bit) when
viewing your average popular post on reddit.com.

Test: fast/dom/form-elements-collection-idempotence.html
      fast/dom/gc-9.html

* html/HTMLFormElement.h:
* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::elements):

    Cache the HTMLFormCollection returned by elements() on the HTMLFormElement.
    Remove the per-form CollectionCache and let the collection manage that.

* html/HTMLCollection.h:
* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::HTMLCollection):
(WebCore::HTMLCollection::create):

    Have the HTMLCollection constructor take a bool argument that decides whether
    we retain the base node pointer or not. This mechanism is a temporary measure
    until all collection types are owned by their respective base nodes.

* html/HTMLFormCollection.h:
* html/HTMLFormCollection.cpp:
(WebCore::HTMLFormCollection::HTMLFormCollection):
(WebCore::HTMLFormCollection::create):

    Tell the base class constructor to not retain the back-pointer to the form.

LayoutTests:

- Update gc-9.html to document the new lifetime characteristics of HTMLFormElement.elements.
- Add a test to verify that HTMLFormElement.elements returns the same object when called repeatedly.

* fast/dom/form-elements-collection-idempotence-expected.txt: Added.
* fast/dom/form-elements-collection-idempotence.html: Added.
* fast/dom/gc-9-expected.txt:
* fast/dom/gc-9.html:

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/form-elements-collection-idempotence-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/form-elements-collection-idempotence.html [new file with mode: 0644]
LayoutTests/fast/dom/gc-9-expected.txt
LayoutTests/fast/dom/gc-9.html
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLCollection.cpp
Source/WebCore/html/HTMLCollection.h
Source/WebCore/html/HTMLFormCollection.cpp
Source/WebCore/html/HTMLFormCollection.h
Source/WebCore/html/HTMLFormElement.cpp
Source/WebCore/html/HTMLFormElement.h

index e9859a5..70132eb 100644 (file)
@@ -1,5 +1,20 @@
 2011-12-30  Andreas Kling  <awesomekling@apple.com>
 
+        Cache and reuse the HTMLFormElement.elements collection.
+        <http://webkit.org/b/75375>
+
+        Reviewed by Anders Carlsson.
+
+        - Update gc-9.html to document the new lifetime characteristics of HTMLFormElement.elements.
+        - Add a test to verify that HTMLFormElement.elements returns the same object when called repeatedly.
+
+        * fast/dom/form-elements-collection-idempotence-expected.txt: Added.
+        * fast/dom/form-elements-collection-idempotence.html: Added.
+        * fast/dom/gc-9-expected.txt:
+        * fast/dom/gc-9.html:
+
+2011-12-30  Andreas Kling  <awesomekling@apple.com>
+
         WebKitCSSKeyframeRule.style.parentRule should point to the keyframe rule.
         <http://webkit.org/b/75336>
 
diff --git a/LayoutTests/fast/dom/form-elements-collection-idempotence-expected.txt b/LayoutTests/fast/dom/form-elements-collection-idempotence-expected.txt
new file mode 100644 (file)
index 0000000..ddec5e7
--- /dev/null
@@ -0,0 +1,11 @@
+This test verifies that HTMLFormElement.elements returns the same collection when called repeatedly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS formElement.elements is formElement.elements
+PASS formElement.elements === formElement.elements is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/form-elements-collection-idempotence.html b/LayoutTests/fast/dom/form-elements-collection-idempotence.html
new file mode 100644 (file)
index 0000000..ebbe54c
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+
+description("This test verifies that HTMLFormElement.elements returns the same collection when called repeatedly.");
+
+var formElement = document.createElement("form");
+
+shouldBe("formElement.elements", "formElement.elements");
+shouldBeTrue("formElement.elements === formElement.elements");
+
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 6b58cec..25d0484 100644 (file)
@@ -23,7 +23,7 @@ PASS: document.links.myCustomProperty should be 1 and is.
 PASS: document.forms.myCustomProperty should be 1 and is.
 PASS: document.anchors.myCustomProperty should be 1 and is.
 PASS: document.scripts.myCustomProperty should be 1 and is.
-PASS: document.getElementsByTagName('form')[0].elements.myCustomProperty should be undefined and is.
+PASS: document.getElementsByTagName('form')[0].elements.myCustomProperty should be 1 and is.
 PASS: document.getElementsByTagName('table')[0].rows.myCustomProperty should be undefined and is.
 PASS: document.getElementsByTagName('table')[0].rows[0].cells.myCustomProperty should be undefined and is.
 PASS: document.getElementsByTagName('table')[0].tBodies.myCustomProperty should be undefined and is.
@@ -59,7 +59,7 @@ PASS: document.links.myCustomProperty should be 1 and is.
 PASS: document.forms.myCustomProperty should be 1 and is.
 PASS: document.anchors.myCustomProperty should be 1 and is.
 PASS: document.scripts.myCustomProperty should be 1 and is.
-PASS: document.getElementsByTagName('form')[0].elements.myCustomProperty should be undefined and is.
+PASS: document.getElementsByTagName('form')[0].elements.myCustomProperty should be 1 and is.
 PASS: document.getElementsByTagName('table')[0].rows.myCustomProperty should be undefined and is.
 PASS: document.getElementsByTagName('table')[0].rows[0].cells.myCustomProperty should be undefined and is.
 PASS: document.getElementsByTagName('table')[0].tBodies.myCustomProperty should be undefined and is.
index 55b5ba1..d68268b 100644 (file)
@@ -134,7 +134,7 @@ var objectsToTest = [
     [ "document.anchors", "allow custom" ],
     [ "document.scripts", "allow custom" ],
 
-    [ "document.getElementsByTagName('form')[0].elements" ],
+    [ "document.getElementsByTagName('form')[0].elements", "allow custom" ],
     [ "document.getElementsByTagName('table')[0].rows" ],
     [ "document.getElementsByTagName('table')[0].rows[0].cells" ],
     [ "document.getElementsByTagName('table')[0].tBodies" ],
index 9d0d686..3ae0622 100644 (file)
@@ -1,5 +1,42 @@
 2011-12-30  Andreas Kling  <awesomekling@apple.com>
 
+        Cache and reuse the HTMLFormElement.elements collection.
+        <http://webkit.org/b/75375>
+
+        Reviewed by Anders Carlsson.
+
+        Let HTMLFormElement::elements() cache the returned collection and tie it to the
+        lifetime of the form. This reduces memory consumption by ~70 kB (on 64-bit) when
+        viewing your average popular post on reddit.com.
+
+        Test: fast/dom/form-elements-collection-idempotence.html
+              fast/dom/gc-9.html
+
+        * html/HTMLFormElement.h:
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::elements):
+
+            Cache the HTMLFormCollection returned by elements() on the HTMLFormElement.
+            Remove the per-form CollectionCache and let the collection manage that.
+
+        * html/HTMLCollection.h:
+        * html/HTMLCollection.cpp:
+        (WebCore::HTMLCollection::HTMLCollection):
+        (WebCore::HTMLCollection::create):
+
+            Have the HTMLCollection constructor take a bool argument that decides whether
+            we retain the base node pointer or not. This mechanism is a temporary measure
+            until all collection types are owned by their respective base nodes.
+
+        * html/HTMLFormCollection.h:
+        * html/HTMLFormCollection.cpp:
+        (WebCore::HTMLFormCollection::HTMLFormCollection):
+        (WebCore::HTMLFormCollection::create):
+
+            Tell the base class constructor to not retain the back-pointer to the form.
+
+2011-12-30  Andreas Kling  <awesomekling@apple.com>
+
         Unreviewed buildfix after r103841.
 
         * inspector/InspectorMemoryAgent.cpp:
index 6cd8dec..250e39d 100644 (file)
@@ -46,15 +46,16 @@ HTMLCollection::HTMLCollection(Document* document, CollectionType type)
 {
 }
 
-HTMLCollection::HTMLCollection(PassRefPtr<Node> base, CollectionType type, CollectionCache* info)
-    : m_baseIsRetained(true)
+HTMLCollection::HTMLCollection(Node* base, CollectionType type, CollectionCache* info, bool retainBaseNode)
+    : m_baseIsRetained(retainBaseNode)
     , m_includeChildren(shouldIncludeChildren(type))
     , m_ownsInfo(false)
     , m_type(type)
-    , m_base(base.get())
+    , m_base(base)
     , m_info(info)
 {
-    m_base->ref();
+    if (m_baseIsRetained)
+        m_base->ref();
 }
 
 bool HTMLCollection::shouldIncludeChildren(CollectionType type)
@@ -96,7 +97,7 @@ PassRefPtr<HTMLCollection> HTMLCollection::createForCachingOnDocument(Document*
 
 PassRefPtr<HTMLCollection> HTMLCollection::create(PassRefPtr<Node> base, CollectionType type)
 {
-    return adoptRef(new HTMLCollection(base, type));
+    return adoptRef(new HTMLCollection(base.get(), type));
 }
 
 HTMLCollection::~HTMLCollection()
index a1dd1ff..a1cf96c 100644 (file)
@@ -62,7 +62,7 @@ public:
     CollectionType type() const { return static_cast<CollectionType>(m_type); }
 
 protected:
-    HTMLCollection(PassRefPtr<Node> base, CollectionType, CollectionCache* = 0);
+    HTMLCollection(Node* base, CollectionType, CollectionCache* = 0, bool retainBaseNode = true);
     HTMLCollection(Document*, CollectionType);
 
     CollectionCache* info() const { return m_info; }
index f24bf66..7d453f7 100644 (file)
@@ -36,19 +36,12 @@ using namespace HTMLNames;
 // Since the collections are to be "live", we have to do the
 // calculation every time if anything has changed.
 
-inline CollectionCache* HTMLFormCollection::formCollectionInfo(HTMLFormElement* form)
+HTMLFormCollection::HTMLFormCollection(HTMLFormElement* form)
+    : HTMLCollection(form, OtherCollection, 0, /* retainBaseNode */ false)
 {
-    if (!form->m_collectionCache)
-        form->m_collectionCache = adoptPtr(new CollectionCache);
-    return form->m_collectionCache.get();
 }
 
-HTMLFormCollection::HTMLFormCollection(PassRefPtr<HTMLFormElement> form)
-    : HTMLCollection(form.get(), OtherCollection, formCollectionInfo(form.get()))
-{
-}
-
-PassRefPtr<HTMLFormCollection> HTMLFormCollection::create(PassRefPtr<HTMLFormElement> form)
+PassRefPtr<HTMLFormCollection> HTMLFormCollection::create(HTMLFormElement* form)
 {
     return adoptRef(new HTMLFormCollection(form));
 }
index e2fee91..d9afccf 100644 (file)
@@ -35,7 +35,7 @@ class QualifiedName;
 
 class HTMLFormCollection : public HTMLCollection {
 public:
-    static PassRefPtr<HTMLFormCollection> create(PassRefPtr<HTMLFormElement>);
+    static PassRefPtr<HTMLFormCollection> create(HTMLFormElement*);
 
     virtual ~HTMLFormCollection();
 
@@ -45,7 +45,7 @@ public:
     virtual Node* namedItem(const AtomicString& name) const;
 
 private:
-    HTMLFormCollection(PassRefPtr<HTMLFormElement>);
+    HTMLFormCollection(HTMLFormElement*);
 
     virtual void updateNameCache() const;
     virtual unsigned calcLength() const;
index 797a082..cf3e6ce 100644 (file)
@@ -500,7 +500,9 @@ void HTMLFormElement::removeImgElement(HTMLImageElement* e)
 
 PassRefPtr<HTMLCollection> HTMLFormElement::elements()
 {
-    return HTMLFormCollection::create(this);
+    if (!m_elementsCollection)
+        m_elementsCollection = HTMLFormCollection::create(this);
+    return m_elementsCollection;
 }
 
 String HTMLFormElement::name() const
index 638cec3..2dd8af1 100644 (file)
@@ -41,8 +41,6 @@ class HTMLInputElement;
 class HTMLFormCollection;
 class TextEncoding;
 
-struct CollectionCache;
-
 class HTMLFormElement : public HTMLElement {
 public:
     static PassRefPtr<HTMLFormElement> create(Document*);
@@ -151,7 +149,7 @@ private:
 
     FormSubmission::Attributes m_attributes;
     OwnPtr<AliasMap> m_elementAliases;
-    OwnPtr<CollectionCache> m_collectionCache;
+    RefPtr<HTMLFormCollection> m_elementsCollection;
 
     CheckedRadioButtons m_checkedRadioButtons;