Diff
Modified: trunk/LayoutTests/ChangeLog (105371 => 105372)
--- trunk/LayoutTests/ChangeLog 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/LayoutTests/ChangeLog 2012-01-19 02:55:03 UTC (rev 105372)
@@ -1,3 +1,18 @@
+2012-01-18 Andreas Kling <awesomekl...@apple.com>
+
+ Cache and reuse the NodeList returned by Node::childNodes().
+ <http://webkit.org/b/76591>
+
+ Reviewed by Ryosuke Niwa.
+
+ Updated gc-9.html to document the new lifetime characteristics of a .childNodes with
+ custom properties. Also added a test to verify that .childNodes === .childNodes.
+
+ * fast/dom/gc-9-expected.txt:
+ * fast/dom/gc-9.html:
+ * fast/dom/node-childNodes-idempotence-expected.txt: Added.
+ * fast/dom/node-childNodes-idempotence.html: Added.
+
2012-01-18 Alexey Proskuryakov <a...@apple.com>
Need infrastructure to test Content-Disposition filename encoding support
Modified: trunk/LayoutTests/fast/dom/gc-9-expected.txt (105371 => 105372)
--- trunk/LayoutTests/fast/dom/gc-9-expected.txt 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/LayoutTests/fast/dom/gc-9-expected.txt 2012-01-19 02:55:03 UTC (rev 105372)
@@ -14,7 +14,7 @@
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 1 and is.
-PASS: document.body.childNodes.myCustomProperty should be undefined and is.
+PASS: document.body.childNodes.myCustomProperty should be 1 and is.
PASS: document.all.myCustomProperty should be 1 and is.
PASS: document.images.myCustomProperty should be 1 and is.
PASS: document.embeds.myCustomProperty should be 1 and is.
@@ -50,7 +50,7 @@
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 1 and is.
-PASS: document.body.childNodes.myCustomProperty should be undefined and is.
+PASS: document.body.childNodes.myCustomProperty should be 1 and is.
PASS: document.all.myCustomProperty should be 1 and is.
PASS: document.images.myCustomProperty should be 1 and is.
PASS: document.embeds.myCustomProperty should be 1 and is.
Modified: trunk/LayoutTests/fast/dom/gc-9.html (105371 => 105372)
--- trunk/LayoutTests/fast/dom/gc-9.html 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/LayoutTests/fast/dom/gc-9.html 2012-01-19 02:55:03 UTC (rev 105372)
@@ -123,7 +123,7 @@
[ "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", "allow custom" ],
- [ "document.body.childNodes" ],
+ [ "document.body.childNodes", "allow custom" ],
[ "document.all", "allow custom" ],
[ "document.images", "allow custom" ],
Added: trunk/LayoutTests/fast/dom/node-childNodes-idempotence-expected.txt (0 => 105372)
--- trunk/LayoutTests/fast/dom/node-childNodes-idempotence-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/node-childNodes-idempotence-expected.txt 2012-01-19 02:55:03 UTC (rev 105372)
@@ -0,0 +1,11 @@
+This test verifies that Node.childNodes returns the same NodeList when called repeatedly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.documentElement.childNodes is document.documentElement.childNodes
+PASS document.documentElement.childNodes === document.documentElement.childNodes is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/node-childNodes-idempotence.html (0 => 105372)
--- trunk/LayoutTests/fast/dom/node-childNodes-idempotence.html (rev 0)
+++ trunk/LayoutTests/fast/dom/node-childNodes-idempotence.html 2012-01-19 02:55:03 UTC (rev 105372)
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+</head>
+<body>
+<script>
+
+description("This test verifies that Node.childNodes returns the same NodeList when called repeatedly.");
+
+shouldBe("document.documentElement.childNodes", "document.documentElement.childNodes");
+shouldBeTrue("document.documentElement.childNodes === document.documentElement.childNodes");
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (105371 => 105372)
--- trunk/Source/WebCore/ChangeLog 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/ChangeLog 2012-01-19 02:55:03 UTC (rev 105372)
@@ -1,3 +1,67 @@
+2012-01-18 Andreas Kling <awesomekl...@apple.com>
+
+ Cache and reuse the NodeList returned by Node::childNodes().
+ <http://webkit.org/b/76591>
+
+ Reviewed by Ryosuke Niwa.
+
+ Instead of only caching the DynamicNodeList::Caches for .childNodes on NodeRareData,
+ cache the full ChildNodeList object. Lifetime management is left to wrappers who
+ invalidate the cached (raw) pointer via Node::removeCachedChildNodeList(), called
+ from ~ChildNodeList().
+
+ This is a slight behavior change, in that Node.childNodes === Node.childNodes will
+ now be true. This matches the behavior of both Firefox and Opera.
+
+ This reduces memory consumption by 192 kB (on 32-bit) when viewing the full
+ HTML5 spec at <http://whatwg.org/c>
+
+ Test: fast/dom/gc-9.html
+ fast/dom/node-childNodes-idempotence.html
+
+ * dom/Node.cpp:
+ (WebCore::Node::childNodes):
+ * dom/NodeRareData.h:
+ (WebCore::NodeRareData::NodeRareData):
+ (WebCore::NodeRareData::childNodeList):
+ (WebCore::NodeRareData::setChildNodeList):
+
+ Only construct one ChildNodeList per Node and store it on NodeRareData for
+ retrieval across childNodes() calls.
+
+ * dom/ChildNodeList.h:
+ (WebCore::ChildNodeList::create):
+ * dom/ChildNodeList.cpp:
+ (WebCore::ChildNodeList::ChildNodeList):
+
+ Construct the Caches at creation instead of passing it to the constructor.
+
+ (WebCore::ChildNodeList::reset):
+
+ Added, resets the internal cache.
+
+ (WebCore::ChildNodeList::~ChildNodeList):
+
+ Call Node::removeCachedChildNodeList().
+
+ * dom/DynamicNodeList.cpp:
+ * dom/DynamicNodeList.h:
+
+ Have DynamicNodeList (and subclasses) respond "true" to isDynamicNodeList().
+ Previously only DynamicSubtreeNodeList (and subclasses) were doing this.
+ Without it, JSC may GC our ChildNodeLists prematurely (due to NodeList's
+ isReachableFromOpaqueRoots() implementation checking isDynamicNodeList().)
+
+ * dom/Node.h:
+ * dom/Node.cpp:
+ (WebCore::Node::removeCachedChildNodeList):
+
+ Added for ~ChildNodeList() to remove the pointer to itself from the Node.
+
+ (WebCore::NodeRareData::clearChildNodeListCache):
+
+ Call ChildNodeList::reset().
+
2012-01-18 James Robinson <jam...@chromium.org>
Unreviewed, rolling out r105366.
Modified: trunk/Source/WebCore/dom/ChildNodeList.cpp (105371 => 105372)
--- trunk/Source/WebCore/dom/ChildNodeList.cpp 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/dom/ChildNodeList.cpp 2012-01-19 02:55:03 UTC (rev 105372)
@@ -27,12 +27,17 @@
namespace WebCore {
-ChildNodeList::ChildNodeList(PassRefPtr<Node> node, DynamicNodeList::Caches* caches)
+ChildNodeList::ChildNodeList(PassRefPtr<Node> node)
: DynamicNodeList(node)
- , m_caches(caches)
+ , m_caches(Caches::create())
{
}
+ChildNodeList::~ChildNodeList()
+{
+ node()->removeCachedChildNodeList();
+}
+
unsigned ChildNodeList::length() const
{
if (m_caches->isLengthCacheValid)
Modified: trunk/Source/WebCore/dom/ChildNodeList.h (105371 => 105372)
--- trunk/Source/WebCore/dom/ChildNodeList.h 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/dom/ChildNodeList.h 2012-01-19 02:55:03 UTC (rev 105372)
@@ -31,16 +31,20 @@
class ChildNodeList : public DynamicNodeList {
public:
- static PassRefPtr<ChildNodeList> create(PassRefPtr<Node> rootNode, Caches* caches)
+ static PassRefPtr<ChildNodeList> create(PassRefPtr<Node> rootNode)
{
- return adoptRef(new ChildNodeList(rootNode, caches));
+ return adoptRef(new ChildNodeList(rootNode));
}
+ virtual ~ChildNodeList();
+
virtual unsigned length() const;
virtual Node* item(unsigned index) const;
+ void reset() { m_caches->reset(); }
+
protected:
- ChildNodeList(PassRefPtr<Node> rootNode, Caches*);
+ ChildNodeList(PassRefPtr<Node> rootNode);
virtual bool nodeMatches(Element*) const;
Modified: trunk/Source/WebCore/dom/DynamicNodeList.cpp (105371 => 105372)
--- trunk/Source/WebCore/dom/DynamicNodeList.cpp 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/dom/DynamicNodeList.cpp 2012-01-19 02:55:03 UTC (rev 105372)
@@ -132,11 +132,6 @@
return 0;
}
-bool DynamicSubtreeNodeList::isDynamicNodeList() const
-{
- return true;
-}
-
void DynamicSubtreeNodeList::invalidateCache()
{
m_caches->reset();
Modified: trunk/Source/WebCore/dom/DynamicNodeList.h (105371 => 105372)
--- trunk/Source/WebCore/dom/DynamicNodeList.h 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/dom/DynamicNodeList.h 2012-01-19 02:55:03 UTC (rev 105372)
@@ -64,6 +64,9 @@
protected:
virtual bool nodeMatches(Element*) const = 0;
RefPtr<Node> m_node;
+
+private:
+ virtual bool isDynamicNodeList() const OVERRIDE { return true; }
};
class DynamicSubtreeNodeList : public DynamicNodeList {
@@ -79,7 +82,6 @@
mutable RefPtr<Caches> m_caches;
private:
- virtual bool isDynamicNodeList() const;
Node* itemForwardsFromCurrent(Node* start, unsigned offset, int remainingOffset) const;
Node* itemBackwardsFromCurrent(Node* start, unsigned offset, int remainingOffset) const;
};
Modified: trunk/Source/WebCore/dom/Node.cpp (105371 => 105372)
--- trunk/Source/WebCore/dom/Node.cpp 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/dom/Node.cpp 2012-01-19 02:55:03 UTC (rev 105372)
@@ -552,7 +552,13 @@
PassRefPtr<NodeList> Node::childNodes()
{
- return ChildNodeList::create(this, ensureRareData()->ensureChildNodeListCache());
+ NodeRareData* data = ""
+ if (data->childNodeList())
+ return PassRefPtr<NodeList>(data->childNodeList());
+
+ RefPtr<ChildNodeList> list = ChildNodeList::create(this);
+ data->setChildNodeList(list.get());
+ return list.release();
}
Node *Node::lastDescendant() const
@@ -1074,6 +1080,12 @@
data->m_labelsNodeListCache = 0;
}
+void Node::removeCachedChildNodeList()
+{
+ ASSERT(rareData());
+ rareData()->setChildNodeList(0);
+}
+
Node* Node::traverseNextNode(const Node* stayWithin) const
{
if (firstChild())
@@ -2935,13 +2947,8 @@
void NodeRareData::clearChildNodeListCache()
{
- if (!m_childNodeListCache)
- return;
-
- if (m_childNodeListCache->hasOneRef())
- m_childNodeListCache.clear();
- else
- m_childNodeListCache->reset();
+ if (m_childNodeList)
+ m_childNodeList->reset();
}
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/Node.h (105371 => 105372)
--- trunk/Source/WebCore/dom/Node.h 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/dom/Node.h 2012-01-19 02:55:03 UTC (rev 105372)
@@ -553,6 +553,8 @@
void removeCachedTagNodeList(TagNodeList*, const QualifiedName&);
void removeCachedLabelsNodeList(DynamicSubtreeNodeList*);
+ void removeCachedChildNodeList();
+
PassRefPtr<NodeList> getElementsByTagName(const AtomicString&);
PassRefPtr<NodeList> getElementsByTagNameNS(const AtomicString& namespaceURI, const AtomicString& localName);
PassRefPtr<NodeList> getElementsByName(const String& elementName);
Modified: trunk/Source/WebCore/dom/NodeRareData.h (105371 => 105372)
--- trunk/Source/WebCore/dom/NodeRareData.h 2012-01-19 02:39:04 UTC (rev 105371)
+++ trunk/Source/WebCore/dom/NodeRareData.h 2012-01-19 02:55:03 UTC (rev 105372)
@@ -95,6 +95,7 @@
public:
NodeRareData()
: m_treeScope(0)
+ , m_childNodeList(0)
, m_tabIndex(0)
, m_tabIndexWasSetExplicitly(false)
, m_isFocused(false)
@@ -132,13 +133,10 @@
return m_nodeLists.get();
}
void clearChildNodeListCache();
- DynamicNodeList::Caches* ensureChildNodeListCache()
- {
- if (!m_childNodeListCache)
- m_childNodeListCache = DynamicNodeList::Caches::create();
- return m_childNodeListCache.get();
- }
+ ChildNodeList* childNodeList() const { return m_childNodeList; }
+ void setChildNodeList(ChildNodeList* list) { m_childNodeList = list; }
+
short tabIndex() const { return m_tabIndex; }
void setTabIndexExplicitly(short index) { m_tabIndex = index; m_tabIndexWasSetExplicitly = true; }
bool tabIndexSetExplicitly() const { return m_tabIndexWasSetExplicitly; }
@@ -241,7 +239,7 @@
TreeScope* m_treeScope;
OwnPtr<NodeListsNodeData> m_nodeLists;
- RefPtr<DynamicNodeList::Caches> m_childNodeListCache;
+ ChildNodeList* m_childNodeList;
OwnPtr<EventTargetData> m_eventTargetData;
short m_tabIndex;
bool m_tabIndexWasSetExplicitly : 1;
