Oilpan: Temporary fix for leaks through mapping from element to attribute node list.

I think we need to bite the bullet and change the semantics of our
weak collections. These leaks are really hard to notice when you are
programming. See crbug.com/381629 which is the bug report I have
filed for tracking that.

R=erik.corry@gmail.com, haraken@chromium.org, oilpan-reviews@chromium.org, sigbjornf@opera.com
BUG=381629

Review URL: https://codereview.chromium.org/317203005

git-svn-id: svn://svn.chromium.org/blink/trunk@175670 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/LayoutTests/fast/dom/Attr/script-tests/access-after-element-destruction.js

 description("Tests that accessing Attr after its Element has been destroyed works without crashing.");
 
-function gc()
-{
-    if (window.GCController)
-        return GCController.collect();
-
-    // Trigger garbage collection indirectly.
-    for (var i = 0; i < 100000; i++)
-        new String(i);
-}
+jsTestIsAsync = true;
 
 var element = document.createElement("p");
 element.setAttribute("a", "b");
 var attributes = element.attributes;
 element = null;
+var attr = null;
 
-gc();
+asyncGC(function() {
+    shouldBe("attributes.length", "1");
+    shouldBe("attributes[0]", "attributes.item(0)");
+    shouldBe("attributes.getNamedItem('a')", "attributes.item(0)");
 
-shouldBe("attributes.length", "1");
-shouldBe("attributes[0]", "attributes.item(0)");
-shouldBe("attributes.getNamedItem('a')", "attributes.item(0)");
+    shouldBe("attributes.item(0).name", "'a'");
+    shouldBe("attributes.item(0).value", "'b'");
 
-shouldBe("attributes.item(0).name", "'a'");
-shouldBe("attributes.item(0).value", "'b'");
+    attributes.item(0).value = 'c';
 
-attributes.item(0).value = 'c';
+    shouldBe("attributes.item(0).value", "'c'");
 
-shouldBe("attributes.item(0).value", "'c'");
+    attributes.removeNamedItem('a');
 
-attributes.removeNamedItem('a');
+    shouldBe("attributes.length", "0");
 
-shouldBe("attributes.length", "0");
+    element = document.createElement("p");
+    element.setAttribute("a", "b");
+    attr = element.attributes.item(0);
+    element = null;
 
-element = document.createElement("p");
-element.setAttribute("a", "b");
-var attr = element.attributes.item(0);
-element = null;
+    asyncGC(function() {
 
-gc();
+        shouldBe("attr.name", "'a'");
+        shouldBe("attr.value", "'b'");
 
-shouldBe("attr.name", "'a'");
-shouldBe("attr.value", "'b'");
+        attr.value = 'c';
 
-attr.value = 'c';
+        shouldBe("attr.value", "'c'");
 
-shouldBe("attr.value", "'c'");
+        finishJSTest();
+    });
+});

third_party/WebKit/Source/core/dom/Attr.cpp

@@ -219,9 +219,15 @@ void Attr::attachToElement(Element* element, const AtomicString& attachedLocalNa
     m_standaloneValueOrAttachedLocalName = attachedLocalName;
 }
 
+void Attr::clearWeakMembers(Visitor* visitor)
+{
+    if (m_element && !visitor->isAlive(m_element))
+        detachFromElementWithValue(value());
+}
+
 void Attr::trace(Visitor* visitor)
 {
-    visitor->trace(m_element);
+    visitor->registerWeakMembers<Attr, &Attr::clearWeakMembers>(this);
     ContainerNode::trace(visitor);
 }
 

third_party/WebKit/Source/core/dom/Attr.h

@@ -64,6 +64,7 @@ public:
     const AtomicString& prefix() const { return m_name.prefix(); }
 
     virtual void trace(Visitor*) OVERRIDE;
+    void clearWeakMembers(Visitor*);
 
 private:
     Attr(Element&, const QualifiedName&);
@@ -89,7 +90,13 @@ private:
 
     // Attr wraps either an element/name, or a name/value pair (when it's a standalone Node.)
     // Note that m_name is always set, but m_element/m_standaloneValue may be null.
-    RawPtrWillBeMember<Element> m_element;
+    //
+    // FIXME: Oilpan: m_element should be a Member. However, because of the
+    // current semantics of weak maps, we have to make it a WeakMember in order
+    // to not leak through the attrNodeListMap in Element.cpp. Once the semantics
+    // of weak maps has changed we should make this a Member and remove the custom
+    // weak processing.
+    RawPtrWillBeWeakMember<Element> m_element;
     QualifiedName m_name;
     // Holds the value if it is a standalone Node, or the local name of the
     // attribute it is attached to on an Element. The latter may (letter case)