Oilpan: Make the Node hierarchy RefCountedGarbageCollected instead of TreeShared.

Source/core/svg/SVGElementInstance.cpp

 /*
  * Copyright (C) 2007, 2008 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  * Copyright (C) 2011 Torch Mobile (Beijing) Co. Ltd. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 68 matching lines @@
 DEFINE_FORWARDING_ATTRIBUTE_EVENT_LISTENER(SVGElementInstance, correspondingElement(), selectstart);
 DEFINE_FORWARDING_ATTRIBUTE_EVENT_LISTENER(SVGElementInstance, correspondingElement(), submit);
 DEFINE_FORWARDING_ATTRIBUTE_EVENT_LISTENER(SVGElementInstance, correspondingElement(), unload);
 
 PassRefPtr<SVGElementInstance> SVGElementInstance::create(SVGUseElement* correspondingUseElement, SVGUseElement* directUseElement, PassRefPtr<SVGElement> originalElement)
 {
     return adoptRef(new SVGElementInstance(correspondingUseElement, directUseElement, originalElement));
 }
 
 SVGElementInstance::SVGElementInstance(SVGUseElement* correspondingUseElement, SVGUseElement* directUseElement, PassRefPtr<SVGElement> originalElement)
-    : m_parentInstance(0)
+    : m_parentInstance(nullptr)
     , m_correspondingUseElement(correspondingUseElement)
     , m_directUseElement(directUseElement)
     , m_element(originalElement)
-    , m_previousSibling(0)
-    , m_nextSibling(0)
-    , m_firstChild(0)
-    , m_lastChild(0)
+    , m_previousSibling(nullptr)
+    , m_nextSibling(nullptr)
+    , m_firstChild(nullptr)
+    , m_lastChild(nullptr)
 {
     ASSERT(m_correspondingUseElement);
     ASSERT(m_element);
     ScriptWrappable::init(this);
 
 #ifndef NDEBUG
     instanceCounter.increment();
 #endif
 }
 
 SVGElementInstance::~SVGElementInstance()
 {
-    // Call detach because we may be deleted directly if we are a child of a detached instance.
-    detach();
-
 #ifndef NDEBUG
     instanceCounter.decrement();
 #endif
 
+#if !ENABLE(OILPAN)
+    // Call detach because we may be deleted directly if we are a child of a detached instance.
+    detach();
     m_element = nullptr;
+#endif
 }
 
 // It's important not to inline removedLastRef, because we don't want to inline the code to
 // delete an SVGElementInstance at each deref call site.
 void SVGElementInstance::removedLastRef()
 {
 #if !ENABLE(OILPAN)
 #if SECURITY_ASSERT_ENABLED
     m_deletionHasBegun = true;
 #endif
     delete this;
 #endif
 }
 
 void SVGElementInstance::detach()
 {
     // Clear all pointers. When the node is detached from the shadow DOM it should be removed but,
     // due to ref counting, it may not be. So clear everything to avoid dangling pointers.
 
     for (SVGElementInstance* node = firstChild(); node; node = node->nextSibling())
         node->detach();
 
     // Deregister as instance for passed element, if we haven't already.
     if (m_element->instancesForElement().contains(shadowTreeElement()))
         m_element->removeInstanceMapping(this);

[haraken, 2014/05/06 15:59:42]

In order to remove this, I think you need to:

- Make SVGElementRareData::m_elementInstances a hash set of WeakMembers.

- Add !ENABLE(OILPAN) to SVGElement::removeInstanceMapping.

What do you think?

[Mads Ager (chromium), 2014/05/07 12:13:16]

I changed it to use weak processing. That will keep the current behavior and
there is already a lot of weak processing in SVGElementRareData. I'm  not
entirely sure that we have to have the weak processing. It seems to me that the
element and the instances should live and die together instead?

I don't have to remove this code here. It is not called via the destructor
anymore and when it is called directly it should probably remove the instance
mapping explicitly.

     // DO NOT clear ref to m_element because JavaScriptCore uses it for garbage collection

[haraken, 2014/05/06 15:59:42]

This comment is obsolete :)

[Mads Ager (chromium), 2014/05/07 12:13:16]

Yes it is, let me remove it. :)

 
     m_shadowTreeElement = nullptr;
 
     m_directUseElement = 0;
     m_correspondingUseElement = 0;
 
+#if !ENABLE(OILPAN)

[haraken, 2014/05/06 15:59:42]

Can detach() be called in oilpan bulids? If no, you can remove the
!ENABLE(OILPAN) and instead add !ENABLE(OILPAN) to detach().

[Mads Ager (chromium), 2014/05/07 12:13:16]

Yes, it can be called via the SVGUseElement.

     removeDetachedChildrenInContainer<SVGElementInstance, SVGElementInstance>(*this);
+#endif
 }
 
 void SVGElementInstance::setShadowTreeElement(SVGElement* element)
 {
     ASSERT(element);
     m_shadowTreeElement = element;
     // Register as instance for passed element.
     m_element->mapInstanceToElement(this);
 
 }
@@ skipping 54 matching lines @@
 }
 
 EventTargetData& SVGElementInstance::ensureEventTargetData()
 {
     // EventTarget would use these methods if we were actually using its add/removeEventListener logic.
     // As we're forwarding those calls to the correspondingElement(), no one should ever call this function.
     ASSERT_NOT_REACHED();
     return *eventTargetData();
 }
 
+void SVGElementInstance::trace(Visitor* visitor)
+{
+    visitor->trace(m_parentInstance);
+    visitor->trace(m_element);
+    visitor->trace(m_shadowTreeElement);
+    visitor->trace(m_previousSibling);
+    visitor->trace(m_nextSibling);
+    visitor->trace(m_firstChild);
+    visitor->trace(m_lastChild);
 }
+
+}