Changeset 103855 in webkit

Timestamp:

Dec 30, 2011, 6:43:33 PM (13 years ago)

Author:

[email protected]

Message:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/gc-9-expected.txt (modified) (2 diffs)
• LayoutTests/fast/dom/gc-9.html (modified) (1 diff)
• LayoutTests/fast/dom/select-options-collection-idempotence-expected.txt (added)
• LayoutTests/fast/dom/select-options-collection-idempotence.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLOptionsCollection.cpp (modified) (2 diffs)
• Source/WebCore/html/HTMLOptionsCollection.h (modified) (2 diffs)
• Source/WebCore/html/HTMLSelectElement.cpp (modified) (2 diffs)
• Source/WebCore/html/HTMLSelectElement.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-12-30  Andreas Kling  <[email protected]>
+
+        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  <[email protected]>
 

trunk/LayoutTests/fast/dom/gc-9-expected.txt

@@ -14 +14 @@
 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.
@@ -50 +50 @@
 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.

trunk/LayoutTests/fast/dom/gc-9.html

@@ -123 +123 @@
     [ "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" ],
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-12-30  Andreas Kling  <[email protected]>
+
+        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  <[email protected]>
 

trunk/Source/WebCore/html/HTMLOptionsCollection.cpp

@@ -28 +28 @@
 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));
@@ -88 +88 @@
 }
 
+void HTMLOptionsCollection::invalidateCache()
+{
+    if (info())
+        info()->reset();
+}
+
 } //namespace

trunk/Source/WebCore/html/HTMLOptionsCollection.h

@@ -36 +36 @@
 class HTMLOptionsCollection : public HTMLCollection {
 public:
-    static PassRefPtr<HTMLOptionsCollection> create(PassRefPtr<HTMLSelectElement>);
+    static PassRefPtr<HTMLOptionsCollection> create(HTMLSelectElement*);
 
     void add(PassRefPtr<HTMLOptionElement>, ExceptionCode&);
@@ -47 +47 @@
     void setLength(unsigned, ExceptionCode&);
 
+    void invalidateCache();
+
 private:
-    HTMLOptionsCollection(PassRefPtr<HTMLSelectElement>);
+    HTMLOptionsCollection(HTMLSelectElement*);
 };
 

trunk/Source/WebCore/html/HTMLSelectElement.cpp

@@ -328 +328 @@
 PassRefPtr<HTMLOptionsCollection> HTMLSelectElement::options()
 {
-    return HTMLOptionsCollection::create(this);
+    if (!m_optionsCollection)
+        m_optionsCollection = HTMLOptionsCollection::create(this);
+    return m_optionsCollection;
 }
 
@@ -681 +683 @@
     setOptionsChangedOnRenderer();
     setNeedsStyleRecalc();
-    if (!inDocument())
-        m_collectionInfo.reset();
+    if (!inDocument() && m_optionsCollection)
+        m_optionsCollection->invalidateCache();
 }
 

trunk/Source/WebCore/html/HTMLSelectElement.h

@@ -27 +27 @@
 #define HTMLSelectElement_h
 
-#include "CollectionCache.h"
 #include "Event.h"
 #include "HTMLFormControlElement.h"
+#include "HTMLOptionsCollection.h"
 #include <wtf/Vector.h>
 
@@ -84 +84 @@
     Node* namedItem(const AtomicString& name);
     Node* item(unsigned index);
-
-    CollectionCache* collectionInfo() { m_collectionInfo.checkConsistency(); return &m_collectionInfo; }
 
     void scrollToSelection();
@@ -177 +175 @@
     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;