Cache and reuse the HTMLSelectElement.options collection.

<http://webkit.org/b/75399>

Reviewed by Anders Carlsson.

Source/WebCore:

Let HTMLSelectElement::options() cache the returned collection and tie it to the
lifetime of the form. This shrinks HTMLSelectElement by sizeof(CollectionCache)
minus one pointer.

Test: fast/dom/select-options-collection-idempotence.html
      fast/gc-9.html

* html/HTMLSelectElement.h:
* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::options):

    Cache the HTMLOptionsCollection returned by options() on the HTMLSelectElement.
    Remove the per-select CollectionCache and let the collection manage that.

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

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

* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::setRecalcListItems):
* html/HTMLOptionsCollection.cpp:
(WebCore::HTMLOptionsCollection::invalidateCache):

    Added so HTMLSelectElement can invalidate the collection without triggering
    unnecessary instantiation of a CollectionCache.

LayoutTests:

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

* fast/dom/gc-9-expected.txt:
* fast/dom/gc-9.html:
* fast/dom/select-options-collection-idempotence-expected.txt: Added.
* fast/dom/select-options-collection-idempotence.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 54cf047..d8c9505 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2011-12-30  Andreas Kling  <awesomekling@apple.com>
+
+        Cache and reuse the HTMLSelectElement.options collection.
+        <http://webkit.org/b/75399>
+
+        Reviewed by Anders Carlsson.
+
+        - Update gc-9.html to document the new lifetime characteristics of HTMLSelectElement.options.
+        - Add a test to verify that HTMLSelectElement.options returns the same object when called repeatedly.
+
+        * fast/dom/gc-9-expected.txt:
+        * fast/dom/gc-9.html:
+        * fast/dom/select-options-collection-idempotence-expected.txt: Added.
+        * fast/dom/select-options-collection-idempotence.html: Added.
+
 2011-12-30  Robert Hogan  <robert@webkit.org>
 
         [Qt] fast/css/absolute-inline-alignment.html fails

diff --git a/LayoutTests/fast/dom/gc-9-expected.txt b/LayoutTests/fast/dom/gc-9-expected.txt
index 25d0484..3594c29 100644 (file)
--- a/LayoutTests/fast/dom/gc-9-expected.txt
+++ b/LayoutTests/fast/dom/gc-9-expected.txt
@@ -13,7 +13,7 @@ PASS: document.getElementsByTagName('body').myCustomProperty should be 1 and is.
 PASS: document.getElementsByTagName('canvas')[0].getContext('2d').myCustomProperty should be 1 and is.
 PASS: document.getElementsByTagName('canvas')[0].getContext('2d').createLinearGradient(0, 0, 0, 0).myCustomProperty should be undefined and is.
 PASS: document.getElementsByTagName('canvas')[0].getContext('2d').createPattern(new Image(), 'no-repeat').myCustomProperty should be undefined and is.
-PASS: document.getElementsByTagName('select')[0].options.myCustomProperty should be undefined and is.
+PASS: document.getElementsByTagName('select')[0].options.myCustomProperty should be 1 and is.
 PASS: document.body.childNodes.myCustomProperty should be undefined and is.
 PASS: document.all.myCustomProperty should be 1 and is.
 PASS: document.images.myCustomProperty should be 1 and is.
@@ -49,7 +49,7 @@ PASS: document.getElementsByTagName('body').myCustomProperty should be 1 and is.
 PASS: document.getElementsByTagName('canvas')[0].getContext('2d').myCustomProperty should be 1 and is.
 PASS: document.getElementsByTagName('canvas')[0].getContext('2d').createLinearGradient(0, 0, 0, 0).myCustomProperty should be undefined and is.
 PASS: document.getElementsByTagName('canvas')[0].getContext('2d').createPattern(new Image(), 'no-repeat').myCustomProperty should be undefined and is.
-PASS: document.getElementsByTagName('select')[0].options.myCustomProperty should be undefined and is.
+PASS: document.getElementsByTagName('select')[0].options.myCustomProperty should be 1 and is.
 PASS: document.body.childNodes.myCustomProperty should be undefined and is.
 PASS: document.all.myCustomProperty should be 1 and is.
 PASS: document.images.myCustomProperty should be 1 and is.

diff --git a/LayoutTests/fast/dom/gc-9.html b/LayoutTests/fast/dom/gc-9.html
index d68268b..c71ae53 100644 (file)
--- a/LayoutTests/fast/dom/gc-9.html
+++ b/LayoutTests/fast/dom/gc-9.html
@@ -122,7 +122,7 @@ var objectsToTest = [
     [ "document.getElementsByTagName('canvas')[0].getContext('2d')", "allow custom" ], // CanvasRenderingContext2D
     [ "document.getElementsByTagName('canvas')[0].getContext('2d').createLinearGradient(0, 0, 0, 0)" ], // CanvasGradient
     [ "document.getElementsByTagName('canvas')[0].getContext('2d').createPattern(new Image(), 'no-repeat')" ], // CanvasPattern
-    [ "document.getElementsByTagName('select')[0].options" ],
+    [ "document.getElementsByTagName('select')[0].options", "allow custom" ],
     [ "document.body.childNodes" ],
 
     [ "document.all", "allow custom" ],

diff --git a/LayoutTests/fast/dom/select-options-collection-idempotence-expected.txt b/LayoutTests/fast/dom/select-options-collection-idempotence-expected.txt
new file mode 100644 (file)
index 0000000..faf503f
--- /dev/null
+++ b/LayoutTests/fast/dom/select-options-collection-idempotence-expected.txt
@@ -0,0 +1,11 @@
+This test verifies that HTMLSelectElement.options returns the same collection when called repeatedly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS selectElement.options is selectElement.options
+PASS selectElement.options === selectElement.options is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

diff --git a/LayoutTests/fast/dom/select-options-collection-idempotence.html b/LayoutTests/fast/dom/select-options-collection-idempotence.html
new file mode 100644 (file)
index 0000000..582a548
--- /dev/null
+++ b/LayoutTests/fast/dom/select-options-collection-idempotence.html
@@ -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 HTMLSelectElement.options returns the same collection when called repeatedly.");
+
+var selectElement = document.createElement("select");
+
+shouldBe("selectElement.options", "selectElement.options");
+shouldBeTrue("selectElement.options === selectElement.options");
+
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e76c824..5b1f0d9 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2011-12-30  Andreas Kling  <awesomekling@apple.com>
+
+        Cache and reuse the HTMLSelectElement.options collection.
+        <http://webkit.org/b/75399>
+
+        Reviewed by Anders Carlsson.
+
+        Let HTMLSelectElement::options() cache the returned collection and tie it to the
+        lifetime of the form. This shrinks HTMLSelectElement by sizeof(CollectionCache)
+        minus one pointer.
+
+        Test: fast/dom/select-options-collection-idempotence.html
+              fast/gc-9.html
+
+        * html/HTMLSelectElement.h:
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::options):
+
+            Cache the HTMLOptionsCollection returned by options() on the HTMLSelectElement.
+            Remove the per-select CollectionCache and let the collection manage that.
+
+        * html/HTMLOptionsCollection.h:
+        * html/HTMLOptionsCollection.cpp:
+        (WebCore::HTMLOptionsCollection::create):
+        (WebCore::HTMLOptionsCollection::HTMLOptionsCollection):
+
+            Tell the base class constructor to not retain the back-pointer to the element.
+
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::setRecalcListItems):
+        * html/HTMLOptionsCollection.cpp:
+        (WebCore::HTMLOptionsCollection::invalidateCache):
+
+            Added so HTMLSelectElement can invalidate the collection without triggering
+            unnecessary instantiation of a CollectionCache.
+
 2011-12-30  Kentaro Hara  <haraken@chromium.org>
 
         Enable the [Supplemental] IDL on CMake

diff --git a/Source/WebCore/html/HTMLOptionsCollection.cpp b/Source/WebCore/html/HTMLOptionsCollection.cpp
index 83f3b09..5798441 100644 (file)
--- a/Source/WebCore/html/HTMLOptionsCollection.cpp
+++ b/Source/WebCore/html/HTMLOptionsCollection.cpp
@@ -27,12 +27,12 @@
 
 namespace WebCore {
 
-HTMLOptionsCollection::HTMLOptionsCollection(PassRefPtr<HTMLSelectElement> select)
-    : HTMLCollection(select.get(), SelectOptions, select->collectionInfo())
+HTMLOptionsCollection::HTMLOptionsCollection(HTMLSelectElement* select)
+    : HTMLCollection(select, SelectOptions, 0, /* retainBaseNode */ false)
 {
 }
 
-PassRefPtr<HTMLOptionsCollection> HTMLOptionsCollection::create(PassRefPtr<HTMLSelectElement> select)
+PassRefPtr<HTMLOptionsCollection> HTMLOptionsCollection::create(HTMLSelectElement* select)
 {
     return adoptRef(new HTMLOptionsCollection(select));
 }
@@ -87,4 +87,10 @@ void HTMLOptionsCollection::setLength(unsigned length, ExceptionCode& ec)
     toHTMLSelectElement(base())->setLength(length, ec);
 }
 
+void HTMLOptionsCollection::invalidateCache()
+{
+    if (info())
+        info()->reset();
+}
+
 } //namespace

diff --git a/Source/WebCore/html/HTMLOptionsCollection.h b/Source/WebCore/html/HTMLOptionsCollection.h
index a82749b..e6a1413 100644 (file)
--- a/Source/WebCore/html/HTMLOptionsCollection.h
+++ b/Source/WebCore/html/HTMLOptionsCollection.h
@@ -35,7 +35,7 @@ typedef int ExceptionCode;
 
 class HTMLOptionsCollection : public HTMLCollection {
 public:
-    static PassRefPtr<HTMLOptionsCollection> create(PassRefPtr<HTMLSelectElement>);
+    static PassRefPtr<HTMLOptionsCollection> create(HTMLSelectElement*);
 
     void add(PassRefPtr<HTMLOptionElement>, ExceptionCode&);
     void add(PassRefPtr<HTMLOptionElement>, int index, ExceptionCode&);
@@ -46,8 +46,10 @@ public:
 
     void setLength(unsigned, ExceptionCode&);
 
+    void invalidateCache();
+
 private:
-    HTMLOptionsCollection(PassRefPtr<HTMLSelectElement>);
+    HTMLOptionsCollection(HTMLSelectElement*);
 };
 
 } //namespace

diff --git a/Source/WebCore/html/HTMLSelectElement.cpp b/Source/WebCore/html/HTMLSelectElement.cpp
index 1cbae1e..4965119 100644 (file)
--- a/Source/WebCore/html/HTMLSelectElement.cpp
+++ b/Source/WebCore/html/HTMLSelectElement.cpp
@@ -327,7 +327,9 @@ RenderObject* HTMLSelectElement::createRenderer(RenderArena* arena, RenderStyle*
 
 PassRefPtr<HTMLOptionsCollection> HTMLSelectElement::options()
 {
-    return HTMLOptionsCollection::create(this);
+    if (!m_optionsCollection)
+        m_optionsCollection = HTMLOptionsCollection::create(this);
+    return m_optionsCollection;
 }
 
 void HTMLSelectElement::updateListItemSelectedStates()
@@ -680,8 +682,8 @@ void HTMLSelectElement::setRecalcListItems()
     m_activeSelectionAnchorIndex = -1;
     setOptionsChangedOnRenderer();
     setNeedsStyleRecalc();
-    if (!inDocument())
-        m_collectionInfo.reset();
+    if (!inDocument() && m_optionsCollection)
+        m_optionsCollection->invalidateCache();
 }
 
 void HTMLSelectElement::recalcListItems(bool updateSelectedStates) const

diff --git a/Source/WebCore/html/HTMLSelectElement.h b/Source/WebCore/html/HTMLSelectElement.h
index e9fc686..0d9ecaa 100644 (file)
--- a/Source/WebCore/html/HTMLSelectElement.h
+++ b/Source/WebCore/html/HTMLSelectElement.h
@@ -26,9 +26,9 @@
 #ifndef HTMLSelectElement_h
 #define HTMLSelectElement_h
 
-#include "CollectionCache.h"
 #include "Event.h"
 #include "HTMLFormControlElement.h"
+#include "HTMLOptionsCollection.h"
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -84,8 +84,6 @@ public:
     Node* namedItem(const AtomicString& name);
     Node* item(unsigned index);
 
-    CollectionCache* collectionInfo() { m_collectionInfo.checkConsistency(); return &m_collectionInfo; }
-
     void scrollToSelection();
 
     void listBoxSelectItem(int listIndex, bool allowMultiplySelections, bool shift, bool fireOnChangeNow = true);
@@ -176,7 +174,8 @@ private:
 
     virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
 
-    CollectionCache m_collectionInfo;
+    RefPtr<HTMLOptionsCollection> m_optionsCollection;
+
     // m_listItems contains HTMLOptionElement, HTMLOptGroupElement, and HTMLHRElement objects.
     mutable Vector<HTMLElement*> m_listItems;
     Vector<bool> m_lastOnChangeSelection;