Diff
Modified: trunk/LayoutTests/ChangeLog (103872 => 103873)
--- trunk/LayoutTests/ChangeLog 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/LayoutTests/ChangeLog 2012-01-01 05:54:09 UTC (rev 103873)
@@ -1,3 +1,13 @@
+2012-01-01 Andreas Kling <[email protected]>
+
+ Make HTMLCollections play nice after their base node is gone.
+ <http://webkit.org/b/75410>
+
+ Reviewed by Anders Carlsson.
+
+ * fast/dom/htmlcollection-zombies-expected.txt: Added.
+ * fast/dom/htmlcollection-zombies.html: Added.
+
2011-12-31 Dan Bernstein <[email protected]>
REGRESSION (WebKit2): Cursor, hover states not updated when page scrolls under stationary mouse pointer
Added: trunk/LayoutTests/fast/dom/htmlcollection-zombies-expected.txt (0 => 103873)
--- trunk/LayoutTests/fast/dom/htmlcollection-zombies-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/htmlcollection-zombies-expected.txt 2012-01-01 05:54:09 UTC (rev 103873)
@@ -0,0 +1,30 @@
+This test tests the behavior of collections after their base node has been destroyed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Testing selectElement's options collection.
+PASS collection.length is 2
+PASS collection[0].getAttribute('name') is 'foo'
+PASS collection[1].getAttribute('name') is 'bar'
+Destroying selectElement
+PASS collection.length is 0
+PASS collection[0] is undefined
+Testing formElement's elements collection.
+PASS collection.length is 2
+PASS collection[0].getAttribute('name') is 'foo'
+PASS collection[1].getAttribute('name') is 'bar'
+Destroying formElement
+PASS collection.length is 0
+PASS collection[0] is undefined
+Testing tableElement's rows collection.
+PASS collection.length is 2
+PASS collection[0].getAttribute('name') is 'foo'
+PASS collection[1].getAttribute('name') is 'bar'
+Destroying tableElement
+PASS collection.length is 0
+PASS collection[0] is undefined
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/htmlcollection-zombies.html (0 => 103873)
--- trunk/LayoutTests/fast/dom/htmlcollection-zombies.html (rev 0)
+++ trunk/LayoutTests/fast/dom/htmlcollection-zombies.html 2012-01-01 05:54:09 UTC (rev 103873)
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+</head>
+<body>
+<select id="selectElement">
+<option name="foo"/>
+<option name="bar"/>
+</select>
+<form id="formElement">
+<input name="foo"/>
+<input name="bar"/>
+</form>
+<table id="tableElement">
+<tr name="foo"></tr>
+<tr name="bar"></tr>
+</table>
+<script>
+
+description("This test tests the behavior of collections after their base node has been destroyed.");
+
+var element;
+var collection;
+
+function testZombieBehavior(id, collectionGetterName)
+{
+ debug("Testing " + id + "'s " + collectionGetterName + " collection.");
+
+ element = document.getElementById(id);
+ collection = eval("element." + collectionGetterName);
+
+ shouldBe("collection.length", "2");
+ shouldBe("collection[0].getAttribute('name')", "'foo'");
+ shouldBe("collection[1].getAttribute('name')", "'bar'");
+
+ debug("Destroying " + id);
+ document.body.removeChild(element);
+ element = null;
+ gc();
+
+ shouldBe("collection.length", "0");
+ shouldBe("collection[0]", "undefined");
+}
+
+testZombieBehavior("selectElement", "options");
+testZombieBehavior("formElement", "elements");
+testZombieBehavior("tableElement", "rows");
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (103872 => 103873)
--- trunk/Source/WebCore/ChangeLog 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/ChangeLog 2012-01-01 05:54:09 UTC (rev 103873)
@@ -1,5 +1,71 @@
2012-01-01 Andreas Kling <[email protected]>
+ Make HTMLCollections play nice after their base node is gone.
+ <http://webkit.org/b/75410>
+
+ Reviewed by Anders Carlsson.
+
+ Added HTMLCollection::detachFromNode() and call that from destructors of nodes
+ with cached collections.
+
+ Sprinkled checks/assertions where applicable to make sure HTMLCollections are
+ empty after their associated node has been destroyed.
+
+ This is a slight change in behavior, as collections would previously keep
+ their nodes alive indefinitely. Added a test to document this.
+
+ Test: fast/dom/htmlcollection-zombies.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::~Document):
+ * html/HTMLAllCollection.cpp:
+ (WebCore::HTMLAllCollection::namedItemWithIndex):
+ * html/HTMLCollection.cpp:
+ (WebCore::HTMLCollection::detachFromNode):
+ (WebCore::HTMLCollection::resetCollectionInfo):
+ (WebCore::HTMLCollection::itemAfter):
+ (WebCore::HTMLCollection::calcLength):
+ (WebCore::HTMLCollection::length):
+ (WebCore::HTMLCollection::item):
+ (WebCore::HTMLCollection::nextItem):
+ (WebCore::HTMLCollection::namedItem):
+ (WebCore::HTMLCollection::updateNameCache):
+ (WebCore::HTMLCollection::hasNamedItem):
+ (WebCore::HTMLCollection::namedItems):
+ (WebCore::HTMLCollection::tags):
+ * html/HTMLCollection.h:
+ * html/HTMLFormCollection.cpp:
+ (WebCore::HTMLFormCollection::calcLength):
+ (WebCore::HTMLFormCollection::item):
+ (WebCore::HTMLFormCollection::getNamedItem):
+ (WebCore::HTMLFormCollection::getNamedFormItem):
+ (WebCore::HTMLFormCollection::namedItem):
+ (WebCore::HTMLFormCollection::updateNameCache):
+ * html/HTMLFormElement.cpp:
+ (WebCore::HTMLFormElement::~HTMLFormElement):
+ * html/HTMLNameCollection.cpp:
+ (WebCore::HTMLNameCollection::itemAfter):
+ * html/HTMLOptionsCollection.cpp:
+ (WebCore::HTMLOptionsCollection::add):
+ (WebCore::HTMLOptionsCollection::remove):
+ (WebCore::HTMLOptionsCollection::selectedIndex):
+ (WebCore::HTMLOptionsCollection::setSelectedIndex):
+ (WebCore::HTMLOptionsCollection::setLength):
+ * html/HTMLPropertiesCollection.cpp:
+ (WebCore::HTMLPropertiesCollection::length):
+ (WebCore::HTMLPropertiesCollection::item):
+ (WebCore::HTMLPropertiesCollection::names):
+ * html/HTMLSelectElement.cpp:
+ (WebCore::HTMLSelectElement::~HTMLSelectElement):
+ * html/HTMLSelectElement.h:
+ * html/HTMLTableElement.cpp:
+ (WebCore::HTMLTableElement::~HTMLTableElement):
+ * html/HTMLTableElement.h:
+ * html/HTMLTableRowsCollection.cpp:
+ (WebCore::HTMLTableRowsCollection::itemAfter):
+
+2012-01-01 Andreas Kling <[email protected]>
+
HTMLCollection: Remove the constructor's custom CollectionCache* argument.
<http://webkit.org/b/75414>
Modified: trunk/Source/WebCore/dom/Document.cpp (103872 => 103873)
--- trunk/Source/WebCore/dom/Document.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/dom/Document.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -566,6 +566,14 @@
if (m_mediaQueryMatcher)
m_mediaQueryMatcher->documentDestroyed();
+
+ for (unsigned i = 0; i < NumUnnamedDocumentCachedTypes; ++i) {
+ if (m_collections[i])
+ m_collections[i]->detachFromNode();
+ }
+
+ if (m_allCollection)
+ m_allCollection->detachFromNode();
}
void Document::removedLastRef()
Modified: trunk/Source/WebCore/html/HTMLAllCollection.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLAllCollection.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLAllCollection.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -47,6 +47,9 @@
Node* HTMLAllCollection::namedItemWithIndex(const AtomicString& name, unsigned index) const
{
+ if (!base())
+ return 0;
+
resetCollectionInfo();
updateNameCache();
info()->checkConsistency();
Modified: trunk/Source/WebCore/html/HTMLCollection.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLCollection.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLCollection.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -109,8 +109,16 @@
m_base->deref();
}
+void HTMLCollection::detachFromNode()
+{
+ m_base = 0;
+ m_baseIsRetained = false;
+}
+
void HTMLCollection::resetCollectionInfo() const
{
+ ASSERT(m_base);
+
uint64_t docversion = static_cast<HTMLDocument*>(m_base->document())->domTreeVersion();
if (!m_info) {
@@ -184,6 +192,8 @@
Element* HTMLCollection::itemAfter(Element* previous) const
{
+ ASSERT(m_base);
+
Node* current;
if (!previous)
current = m_base->firstChild();
@@ -203,6 +213,8 @@
unsigned HTMLCollection::calcLength() const
{
+ ASSERT(m_base);
+
unsigned len = 0;
for (Element* current = itemAfter(0); current; current = itemAfter(current))
++len;
@@ -213,6 +225,9 @@
// calculation every time if anything has changed
unsigned HTMLCollection::length() const
{
+ if (!m_base)
+ return 0;
+
resetCollectionInfo();
if (!m_info->hasLength) {
m_info->length = calcLength();
@@ -223,6 +238,9 @@
Node* HTMLCollection::item(unsigned index) const
{
+ if (!m_base)
+ return 0;
+
resetCollectionInfo();
if (m_info->current && m_info->position == index)
return m_info->current;
@@ -249,8 +267,9 @@
Node* HTMLCollection::nextItem() const
{
+ ASSERT(m_base);
resetCollectionInfo();
-
+
// Look for the 'second' item. The first one is currentItem, already given back.
Element* retval = itemAfter(m_info->current);
m_info->current = retval;
@@ -288,6 +307,9 @@
Node* HTMLCollection::namedItem(const AtomicString& name) const
{
+ if (!m_base)
+ return 0;
+
// http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/nameditem.asp
// This method first searches for an object with a matching id
// attribute. If a match is not found, the method then searches for an
@@ -315,9 +337,11 @@
void HTMLCollection::updateNameCache() const
{
+ ASSERT(m_base);
+
if (m_info->hasNameCache)
return;
-
+
for (Element* element = itemAfter(0); element; element = itemAfter(element)) {
if (!element->isHTMLElement())
continue;
@@ -335,6 +359,9 @@
bool HTMLCollection::hasNamedItem(const AtomicString& name) const
{
+ if (!m_base)
+ return false;
+
if (name.isEmpty())
return false;
@@ -357,8 +384,10 @@
void HTMLCollection::namedItems(const AtomicString& name, Vector<RefPtr<Node> >& result) const
{
+ if (!m_base)
+ return;
+
ASSERT(result.isEmpty());
-
if (name.isEmpty())
return;
@@ -378,6 +407,9 @@
PassRefPtr<NodeList> HTMLCollection::tags(const String& name)
{
+ if (!m_base)
+ return 0;
+
return m_base->getElementsByTagName(name);
}
Modified: trunk/Source/WebCore/html/HTMLCollection.h (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLCollection.h 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLCollection.h 2012-01-01 05:54:09 UTC (rev 103873)
@@ -61,6 +61,8 @@
Node* base() const { return m_base; }
CollectionType type() const { return static_cast<CollectionType>(m_type); }
+ void detachFromNode();
+
protected:
HTMLCollection(Node* base, CollectionType, bool retainBaseNode = true);
HTMLCollection(Document*, CollectionType);
Modified: trunk/Source/WebCore/html/HTMLFormCollection.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLFormCollection.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLFormCollection.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -52,11 +52,15 @@
unsigned HTMLFormCollection::calcLength() const
{
+ ASSERT(base());
return static_cast<HTMLFormElement*>(base())->length();
}
Node* HTMLFormCollection::item(unsigned index) const
{
+ if (!base())
+ return 0;
+
resetCollectionInfo();
if (info()->current && info()->position == index)
@@ -93,6 +97,9 @@
Element* HTMLFormCollection::getNamedItem(const QualifiedName& attrName, const AtomicString& name) const
{
+ if (!base())
+ return 0;
+
info()->position = 0;
return getNamedFormItem(attrName, name, 0);
}
@@ -101,6 +108,9 @@
{
HTMLFormElement* form = static_cast<HTMLFormElement*>(base());
+ if (!form)
+ return 0;
+
bool foundInputElements = false;
for (unsigned i = 0; i < form->m_associatedElements.size(); ++i) {
FormAssociatedElement* associatedElement = form->m_associatedElements[i];
@@ -134,6 +144,9 @@
Node* HTMLFormCollection::namedItem(const AtomicString& name) const
{
+ if (!base())
+ return 0;
+
// http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/nameditem.asp
// This method first searches for an object with a matching id
// attribute. If a match is not found, the method then searches for an
@@ -149,6 +162,9 @@
void HTMLFormCollection::updateNameCache() const
{
+ if (!base())
+ return;
+
if (info()->hasNameCache)
return;
Modified: trunk/Source/WebCore/html/HTMLFormElement.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLFormElement.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLFormElement.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -91,6 +91,9 @@
HTMLFormElement::~HTMLFormElement()
{
+ if (m_elementsCollection)
+ m_elementsCollection->detachFromNode();
+
if (!shouldAutocomplete())
document()->unregisterForPageCacheSuspensionCallbacks(this);
Modified: trunk/Source/WebCore/html/HTMLNameCollection.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLNameCollection.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLNameCollection.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -40,6 +40,9 @@
Element* HTMLNameCollection::itemAfter(Element* previous) const
{
+ if (!base())
+ return 0;
+
ASSERT(previous != base());
Node* current;
Modified: trunk/Source/WebCore/html/HTMLOptionsCollection.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLOptionsCollection.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLOptionsCollection.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -60,6 +60,9 @@
ec = 0;
HTMLSelectElement* select = toHTMLSelectElement(base());
+ if (!select)
+ return;
+
if (index == -1 || unsigned(index) >= length())
select->add(newOption, 0, ec);
else
@@ -70,21 +73,29 @@
void HTMLOptionsCollection::remove(int index)
{
+ if (!base())
+ return;
toHTMLSelectElement(base())->remove(index);
}
int HTMLOptionsCollection::selectedIndex() const
{
+ if (!base())
+ return -1;
return toHTMLSelectElement(base())->selectedIndex();
}
void HTMLOptionsCollection::setSelectedIndex(int index)
{
+ if (!base())
+ return;
toHTMLSelectElement(base())->setSelectedIndex(index);
}
void HTMLOptionsCollection::setLength(unsigned length, ExceptionCode& ec)
{
+ if (!base())
+ return;
toHTMLSelectElement(base())->setLength(length, ec);
}
Modified: trunk/Source/WebCore/html/HTMLPropertiesCollection.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLPropertiesCollection.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLPropertiesCollection.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -123,6 +123,9 @@
unsigned HTMLPropertiesCollection::length() const
{
+ if (!base())
+ return 0;
+
if (!base()->isHTMLElement() || !toHTMLElement(base())->fastHasAttribute(itemscopeAttr))
return 0;
@@ -133,6 +136,9 @@
Node* HTMLPropertiesCollection::item(unsigned index) const
{
+ if (!base())
+ return 0;
+
if (!base()->isHTMLElement() || !toHTMLElement(base())->fastHasAttribute(itemscopeAttr))
return 0;
@@ -151,6 +157,9 @@
m_properties.clear();
m_propertyNames->clear();
+ if (!base())
+ return 0;
+
if (!base()->isHTMLElement() || !toHTMLElement(base())->fastHasAttribute(itemscopeAttr))
return m_propertyNames;
Modified: trunk/Source/WebCore/html/HTMLSelectElement.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLSelectElement.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLSelectElement.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -92,6 +92,12 @@
ASSERT(hasTagName(selectTag));
}
+HTMLSelectElement::~HTMLSelectElement()
+{
+ if (m_optionsCollection)
+ m_optionsCollection->detachFromNode();
+}
+
PassRefPtr<HTMLSelectElement> HTMLSelectElement::create(const QualifiedName& tagName, Document* document, HTMLFormElement* form)
{
ASSERT(tagName.matches(selectTag));
Modified: trunk/Source/WebCore/html/HTMLSelectElement.h (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLSelectElement.h 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLSelectElement.h 2012-01-01 05:54:09 UTC (rev 103873)
@@ -28,7 +28,6 @@
#include "Event.h"
#include "HTMLFormControlElement.h"
-#include "HTMLOptionsCollection.h"
#include <wtf/Vector.h>
namespace WebCore {
@@ -40,6 +39,8 @@
public:
static PassRefPtr<HTMLSelectElement> create(const QualifiedName&, Document*, HTMLFormElement*);
+ virtual ~HTMLSelectElement();
+
int selectedIndex() const;
void setSelectedIndex(int);
Modified: trunk/Source/WebCore/html/HTMLTableElement.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLTableElement.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLTableElement.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -54,6 +54,12 @@
ASSERT(hasTagName(tableTag));
}
+HTMLTableElement::~HTMLTableElement()
+{
+ if (m_rowsCollection)
+ m_rowsCollection->detachFromNode();
+}
+
PassRefPtr<HTMLTableElement> HTMLTableElement::create(Document* document)
{
return adoptRef(new HTMLTableElement(tableTag, document));
Modified: trunk/Source/WebCore/html/HTMLTableElement.h (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLTableElement.h 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLTableElement.h 2012-01-01 05:54:09 UTC (rev 103873)
@@ -40,6 +40,8 @@
static PassRefPtr<HTMLTableElement> create(Document*);
static PassRefPtr<HTMLTableElement> create(const QualifiedName&, Document*);
+ virtual ~HTMLTableElement();
+
HTMLTableCaptionElement* caption() const;
void setCaption(PassRefPtr<HTMLTableCaptionElement>, ExceptionCode&);
Modified: trunk/Source/WebCore/html/HTMLTableRowsCollection.cpp (103872 => 103873)
--- trunk/Source/WebCore/html/HTMLTableRowsCollection.cpp 2012-01-01 04:34:01 UTC (rev 103872)
+++ trunk/Source/WebCore/html/HTMLTableRowsCollection.cpp 2012-01-01 05:54:09 UTC (rev 103873)
@@ -163,6 +163,7 @@
Element* HTMLTableRowsCollection::itemAfter(Element* previous) const
{
+ ASSERT(base());
ASSERT(!previous || previous->hasLocalName(trTag));
return rowAfter(static_cast<HTMLTableElement*>(base()), static_cast<HTMLTableRowElement*>(previous));
}
