Remove SVGElementInstance

Source/core/svg/SVGUseElement.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <buis@kde.org>
  * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
  * Copyright (C) 2011 Torch Mobile (Beijing) Co. Ltd. All rights reserved.
  * Copyright (C) 2012 University of Szeged
  * Copyright (C) 2012 Renata Hodovan <reni@webkit.org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 19 matching lines @@
 #include "bindings/v8/ExceptionStatePlaceholder.h"
 #include "core/dom/Document.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/events/Event.h"
 #include "core/dom/shadow/ElementShadow.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/fetch/FetchRequest.h"
 #include "core/fetch/ResourceFetcher.h"
 #include "core/rendering/svg/RenderSVGResource.h"
 #include "core/rendering/svg/RenderSVGTransformableContainer.h"
-#include "core/svg/SVGElementInstance.h"
 #include "core/svg/SVGGElement.h"
 #include "core/svg/SVGLengthContext.h"
 #include "core/svg/SVGSVGElement.h"
 #include "core/xml/parser/XMLDocumentParser.h"
 
 namespace WebCore {
 
 inline SVGUseElement::SVGUseElement(Document& document)
     : SVGGraphicsElement(SVGNames::useTag, document)
     , SVGURIReference(this)
@@ skipping 23 matching lines @@
 }
 
 SVGUseElement::~SVGUseElement()
 {
     setDocumentResource(0);
 #if !ENABLE(OILPAN)
     clearResourceReferences();
 #endif
 }
 
-SVGElementInstance* SVGUseElement::instanceRoot()
-{
-    // If there is no element instance tree, force immediate SVGElementInstance tree
-    // creation by asking the document to invoke our recalcStyle function - as we can't
-    // wait for the lazy creation to happen if e.g. JS wants to access the instanceRoot
-    // object right after creating the element on-the-fly
-    if (!m_targetElementInstance)
-        document().updateRenderTreeIfNeeded();
-
-    return m_targetElementInstance.get();
-}
-
-SVGElementInstance* SVGUseElement::animatedInstanceRoot() const
-{
-    // FIXME: Implement me.
-    return 0;
-}
-
 bool SVGUseElement::isSupportedAttribute(const QualifiedName& attrName)
 {
     DEFINE_STATIC_LOCAL(HashSet<QualifiedName>, supportedAttributes, ());
     if (supportedAttributes.isEmpty()) {
         SVGURIReference::addSupportedAttributes(supportedAttributes);
         supportedAttributes.add(SVGNames::xAttr);
         supportedAttributes.add(SVGNames::yAttr);
         supportedAttributes.add(SVGNames::widthAttr);
         supportedAttributes.add(SVGNames::heightAttr);
     }
@@ skipping 105 matching lines @@
     SVGElement::InvalidationGuard invalidationGuard(this);
 
     RenderObject* renderer = this->renderer();
     if (attrName == SVGNames::xAttr
         || attrName == SVGNames::yAttr
         || attrName == SVGNames::widthAttr
         || attrName == SVGNames::heightAttr) {
         updateRelativeLengthsInformation();
         if (m_targetElementInstance) {
             ASSERT(m_targetElementInstance->correspondingElement());
-            transferUseWidthAndHeightIfNeeded(*this, m_targetElementInstance->shadowTreeElement(), *m_targetElementInstance->correspondingElement());
+            transferUseWidthAndHeightIfNeeded(*this, m_targetElementInstance.get(), *m_targetElementInstance->correspondingElement());
         }
         if (renderer)
             RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer);
         return;
     }
 
     if (SVGURIReference::isKnownAttribute(attrName)) {
         bool isExternalReference = isExternalURIReference(hrefString(), document());
         if (isExternalReference) {
             KURL url = document().completeURL(hrefString());
@@ skipping 73 matching lines @@
 void SVGUseElement::scheduleShadowTreeRecreation()
 {
     if (!referencedScope() || inUseShadowTree())
         return;
     m_needsShadowTreeRecreation = true;
     document().scheduleUseShadowTreeUpdate(*this);
 }
 
 void SVGUseElement::clearResourceReferences()
 {
-    if (m_targetElementInstance) {
-        m_targetElementInstance->detach();
+    if (m_targetElementInstance)
         m_targetElementInstance = nullptr;
-    }
 
     // FIXME: We should try to optimize this, to at least allow partial reclones.
     if (ShadowRoot* shadowTreeRootElement = userAgentShadowRoot())
         shadowTreeRootElement->removeChildren();
 
     m_needsShadowTreeRecreation = false;
     document().unscheduleUseShadowTreeUpdate(*this);
 
     document().accessSVGExtensions().removeAllTargetReferencesForElement(this);
 }
@@ skipping 22 matching lines @@
     }
 
     if (target->isSVGElement()) {
         buildShadowAndInstanceTree(toSVGElement(target));
         invalidateDependentShadowTrees();
     }
 
     ASSERT(!m_needsShadowTreeRecreation);
 }
 
+static PassRefPtr<Node> cloneNodeAndAssociate(Node* toClone)

[haraken, 2014/06/05 05:49:00]

PassRefPtrWillBeRawPtr

+{

[pdr., 2014/06/05 04:40:01]

Can you assert that toClone is a text or svg node?

+    RefPtr<Node> clone = toClone->cloneNode(false);

[haraken, 2014/06/05 05:49:00]

RefPtrWillBeRawPtr

+    if (clone->isSVGElement()) {

[esprehn, 2014/06/05 06:29:16]

Early return.

if (!clone->isSVGElement())
  return clone.release();

+        SVGElement* svgElement = toSVGElement(toClone);
+        ASSERT(!svgElement->correspondingElement());
+        toSVGElement(clone.get())->setCorrespondingElement(svgElement);
+        if (EventTargetData* data = toClone->eventTargetData())
+            data->eventListenerMap.copyEventListenersNotCreatedFromMarkupToTarget(svgElement);

[pdr., 2014/06/05 04:40:01]

What are these events for? I thought that we wouldn't need to do this.

[esprehn, 2014/06/05 06:29:16]

This doesn't look safe, I think you leak access to the user agent shadow root
tree with the event listener here.

+        TrackExceptionState exceptionState;
+        for (Node* n = toClone->firstChild(); n && !exceptionState.hadException(); n = n->nextSibling())

[esprehn, 2014/06/05 06:29:16]

node =, don't abbreviate.

+            clone->appendChild(cloneNodeAndAssociate(n), exceptionState);
+    }
+    return clone.release();
+}
+
 void SVGUseElement::buildShadowAndInstanceTree(SVGElement* target)
 {
     ASSERT(!m_targetElementInstance);
 
     // <use> creates a "user agent" shadow root. Do not build the shadow/instance tree for <use>
     // elements living in a user agent shadow tree because they will get expanded in a second
     // pass -- see expandUseElementsInShadowTree().
     if (inUseShadowTree())
         return;
 
     // Do not allow self-referencing.
     // 'target' may be null, if it's a non SVG namespaced element.
     if (!target || target == this)
         return;
 
-    // Why a seperated instance/shadow tree? SVG demands it:
-    // The instance tree is accesable from JavaScript, and has to
-    // expose a 1:1 copy of the referenced tree, whereas internally we need
-    // to alter the tree for correct "use-on-symbol", "use-on-svg" support.
+    ShadowRoot* shadowTreeRootElement = userAgentShadowRoot();

[esprehn, 2014/06/05 06:29:16]

ShadowRoot& root = ensureUserAgentShadowRoot();

get rid of the ASSERT.

+    ASSERT(shadowTreeRootElement);
 
-    // Build instance tree. Create root SVGElementInstance object for the first sub-tree node.
-    //
-    // Spec: If the 'use' element references a simple graphics element such as a 'rect', then there is only a
-    // single SVGElementInstance object, and the correspondingElement attribute on this SVGElementInstance object
-    // is the SVGRectElement that corresponds to the referenced 'rect' element.
-    m_targetElementInstance = SVGElementInstance::create(this, this, target);
+    // Set up root SVG element in shadow tree.
+    RefPtr<Element> newChild = target->cloneElementWithoutChildren();

[kouhei (in TOK), 2014/06/05 05:41:37]

Oilpan nit: RefPtrWillBeRawPtr

+    m_targetElementInstance = toSVGElement(newChild.get());
+    shadowTreeRootElement->appendChild(newChild.release());
 
-    // Eventually enter recursion to build SVGElementInstance objects for the sub-tree children
+    // Clone the target subtree into the shadow tree, not handling <use> and <symbol> yet.
     bool foundProblem = false;
-    buildInstanceTree(target, m_targetElementInstance.get(), foundProblem, false);
-
-    if (instanceTreeIsLoading(m_targetElementInstance.get()))
-        return;
+    buildShadowTree(target, m_targetElementInstance.get(), foundProblem, false);
 
     // SVG specification does not say a word about <use> & cycles. My view on this is: just ignore it!
     // Non-appearing <use> content is easier to debug, then half-appearing content.
     if (foundProblem) {
         clearResourceReferences();
         return;
     }
 
-    // Assure instance tree building was successfull
+    if (instanceTreeIsLoading(m_targetElementInstance.get()))
+        return;
+
+    // Assure shadow tree building was successfull
     ASSERT(m_targetElementInstance);
-    ASSERT(!m_targetElementInstance->shadowTreeElement());
     ASSERT(m_targetElementInstance->correspondingUseElement() == this);
-    ASSERT(m_targetElementInstance->directUseElement() == this);
     ASSERT(m_targetElementInstance->correspondingElement() == target);
 
-    ShadowRoot* shadowTreeRootElement = userAgentShadowRoot();
-    ASSERT(shadowTreeRootElement);
-
-    // Build shadow tree from instance tree
-    // This also handles the special cases: <use> on <symbol>, <use> on <svg>.
-    buildShadowTree(target, m_targetElementInstance.get(), shadowTreeRootElement);
-
     // Expand all <use> elements in the shadow tree.
     // Expand means: replace the actual <use> element by what it references.
-    expandUseElementsInShadowTree(shadowTreeRootElement);
-
-    // Expand all <symbol> elements in the shadow tree.
-    // Expand means: replace the actual <symbol> element by the <svg> element.
-    expandSymbolElementsInShadowTree(shadowTreeRootElement);
-
-    // If no shadow tree element is present, this means that the reference root
-    // element was removed, as it is disallowed (ie. <use> on <foreignObject>)
-    // Do NOT leave an inconsistent instance tree around, instead destruct it.
-    Node* shadowTreeTargetNode = shadowTreeRootElement->firstChild();
-    if (!shadowTreeTargetNode) {
+    foundProblem = false;
+    expandUseElementsInShadowTree(m_targetElementInstance.get(), foundProblem);

[esprehn, 2014/06/05 06:29:16]

Use a return value? This foundProblem thing is weird.

+    if (foundProblem) {
         clearResourceReferences();
         return;
     }
 
-    // Now that the shadow tree is completly expanded, we can associate
-    // shadow tree elements <-> instances in the instance tree.
-    associateInstancesWithShadowTreeElements(shadowTreeTargetNode, m_targetElementInstance.get());
+    // Expand all <symbol> elements in the shadow tree.
+    // Expand means: replace the actual <symbol> element by the <svg> element.
+    expandSymbolElementsInShadowTree(toSVGElement(shadowTreeRootElement->firstChild()));
 
-    SVGElement* shadowTreeTargetElement = toSVGElement(shadowTreeTargetNode);
-    ASSERT(shadowTreeTargetElement->correspondingElement());
-    transferUseWidthAndHeightIfNeeded(*this, shadowTreeTargetElement, *shadowTreeTargetElement->correspondingElement());
+    m_targetElementInstance = toSVGElement(shadowTreeRootElement->firstChild());
+    transferUseWidthAndHeightIfNeeded(*this, m_targetElementInstance.get(), *m_targetElementInstance->correspondingElement());
 
-    ASSERT(shadowTreeTargetElement->parentNode() == shadowTreeRootElement);
-
-    // Transfer event listeners assigned to the referenced element to our shadow tree elements.
-    transferEventListenersToShadowTree(shadowTreeTargetElement);
+    ASSERT(m_targetElementInstance->parentNode() == shadowTreeRootElement);
 
     // Update relative length information.
     updateRelativeLengthsInformation();
 }
 
 RenderObject* SVGUseElement::createRenderer(RenderStyle*)
 {
     return new RenderSVGTransformableContainer(this);
 }
 
@@ skipping 33 matching lines @@
 RenderObject* SVGUseElement::rendererClipChild() const
 {
     if (Node* n = userAgentShadowRoot()->firstChild()) {
         if (n->isSVGElement() && isDirectReference(*n))
             return toSVGElement(n)->renderer();
     }
 
     return 0;
 }
 
-void SVGUseElement::buildInstanceTree(SVGElement* target, SVGElementInstance* targetInstance, bool& foundProblem, bool foundUse)
+void SVGUseElement::buildShadowTree(SVGElement* target, SVGElement* targetInstance, bool& foundProblem, bool foundUse)

[esprehn, 2014/06/05 06:29:16]

Use a return value, or pass a structure. Don't use the bool& and bool stuff.

 {
     ASSERT(target);
     ASSERT(targetInstance);
 
     // Spec: If the referenced object is itself a 'use', or if there are 'use' subelements within the referenced
     // object, the instance tree will contain recursive expansion of the indirect references to form a complete tree.
-    bool targetIsUseElement = isSVGUseElement(*target);
-    SVGElement* newTarget = 0;
-    if (targetIsUseElement) {
-        foundProblem = hasCycleUseReferencing(toSVGUseElement(target), targetInstance, newTarget);
-        if (foundProblem)
-            return;
-
+    if (isSVGUseElement(*target)) {
         // We only need to track first degree <use> dependencies. Indirect references are handled
         // as the invalidation bubbles up the dependency chain.
         if (!foundUse) {
             document().accessSVGExtensions().addElementReferencingTarget(this, target);
             foundUse = true;
         }
     } else if (isDisallowedElement(target)) {
         foundProblem = true;
         return;
     }
 
-    // A general description from the SVG spec, describing what buildInstanceTree() actually does.
-    //
-    // Spec: If the 'use' element references a 'g' which contains two 'rect' elements, then the instance tree
-    // contains three SVGElementInstance objects, a root SVGElementInstance object whose correspondingElement
-    // is the SVGGElement object for the 'g', and then two child SVGElementInstance objects, each of which has
-    // its correspondingElement that is an SVGRectElement object.
+    targetInstance->setCorrespondingElement(target);
 
-    for (SVGElement* element = Traversal<SVGElement>::firstChild(*target); element; element = Traversal<SVGElement>::nextSibling(*element)) {
+    for (RefPtr<Node> child = target->firstChild(); child; child = child->nextSibling()) {

[kouhei (in TOK), 2014/06/05 05:41:37]

Can this be Node*? I don't think a strong ref is needed here.
If we do need a strong ref, oilpan nit: RefPtrWillBeRawPtr

         // Skip any disallowed element.
-        if (isDisallowedElement(element))
+        if (isDisallowedElement(child.get()))
             continue;
 
-        // Create SVGElementInstance object, for both container/non-container nodes.
-        RefPtrWillBeRawPtr<SVGElementInstance> instance = SVGElementInstance::create(this, 0, element);
-        SVGElementInstance* instancePtr = instance.get();
-        targetInstance->appendChild(instance.release());
-
-        // Enter recursion, appending new instance tree nodes to the "instance" object.
-        buildInstanceTree(element, instancePtr, foundProblem, foundUse);
-        if (foundProblem)
-            return;
+        RefPtrWillBeRawPtr<Node> newChild = child->cloneNode(false);
+        Node* instancePtr = newChild.get();

[esprehn, 2014/06/05 06:29:16]

Get rid of this local var, also don't use Ptr.

+        targetInstance->appendChild(newChild.release());
+        if (instancePtr->isSVGElement()) {

[esprehn, 2014/06/05 06:29:16]

newChild->isSVGElement()

+            // Enter recursion, appending new instance tree nodes to the "instance" object.
+            buildShadowTree(toSVGElement(child), toSVGElement(instancePtr), foundProblem, foundUse);

[esprehn, 2014/06/05 06:29:16]

toSVGElement(newChild.get())

+            if (foundProblem)

[esprehn, 2014/06/05 06:29:16]

I think you want to pass a Context structure that has the foundProblem and
foundUse bits on it instead.

+                return;
+        }
     }
-
-    if (!targetIsUseElement || !newTarget)
-        return;
-
-    RefPtrWillBeRawPtr<SVGElementInstance> newInstance = SVGElementInstance::create(this, toSVGUseElement(target), newTarget);
-    SVGElementInstance* newInstancePtr = newInstance.get();
-    targetInstance->appendChild(newInstance.release());
-    buildInstanceTree(newTarget, newInstancePtr, foundProblem, foundUse);
 }
 
-bool SVGUseElement::hasCycleUseReferencing(SVGUseElement* use, SVGElementInstance* targetInstance, SVGElement*& newTarget)
+bool SVGUseElement::hasCycleUseReferencing(SVGUseElement* use, ContainerNode* targetInstance, SVGElement*& newTarget)
 {
     ASSERT(referencedScope());
     Element* targetElement = SVGURIReference::targetElementFromIRIString(use->hrefString(), *referencedScope());
     newTarget = 0;
     if (targetElement && targetElement->isSVGElement())
         newTarget = toSVGElement(targetElement);
 
     if (!newTarget)
         return false;
 
     // Shortcut for self-references
     if (newTarget == this)
         return true;
 
     AtomicString targetId = newTarget->getIdAttribute();
-    SVGElementInstance* instance = targetInstance->parentNode();
-    while (instance) {
-        SVGElement* element = instance->correspondingElement();
-
+    ContainerNode* instance = targetInstance->parentNode();
+    while (instance && instance->isSVGElement()) {
+        SVGElement* element = toSVGElement(instance);
         if (element->hasID() && element->getIdAttribute() == targetId && element->document() == newTarget->document())
             return true;
 
         instance = instance->parentNode();
     }
     return false;
 }
 
 static inline void removeDisallowedElementsFromSubtree(Element& subtree)
 {
     ASSERT(!subtree.inDocument());
     Element* element = ElementTraversal::firstWithin(subtree);
     while (element) {
         if (isDisallowedElement(element)) {
             Element* next = ElementTraversal::nextSkippingChildren(*element, &subtree);
             // The subtree is not in document so this won't generate events that could mutate the tree.
             element->parentNode()->removeChild(element);
             element = next;
         } else {
             element = ElementTraversal::next(*element, &subtree);
         }
     }
 }
 
-void SVGUseElement::buildShadowTree(SVGElement* target, SVGElementInstance* targetInstance, ShadowRoot* shadowTreeRootElement)
-{
-    // For instance <use> on <foreignObject> (direct case).
-    if (isDisallowedElement(target))
-        return;
-
-    RefPtrWillBeRawPtr<Element> newChild = targetInstance->correspondingElement()->cloneElementWithChildren();
-
-    // We don't walk the target tree element-by-element, and clone each element,
-    // but instead use cloneElementWithChildren(). This is an optimization for the common
-    // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
-    // Though if there are disallowed elements in the subtree, we have to remove them.
-    // For instance: <use> on <g> containing <foreignObject> (indirect case).
-    if (subtreeContainsDisallowedElement(newChild.get()))
-        removeDisallowedElementsFromSubtree(*newChild);
-
-    shadowTreeRootElement->appendChild(newChild.release());
-}
-
-void SVGUseElement::expandUseElementsInShadowTree(Node* element)
+void SVGUseElement::expandUseElementsInShadowTree(SVGElement* element, bool& foundProblem)
 {
     ASSERT(element);
     // Why expand the <use> elements in the shadow tree here, and not just
     // do this directly in buildShadowTree, if we encounter a <use> element?
     //
     // Short answer: Because we may miss to expand some elements. For example, if a <symbol>
     // contains <use> tags, we'd miss them. So once we're done with setting up the
     // actual shadow tree (after the special case modification for svg/symbol) we have
     // to walk it completely and expand all <use> elements.
     if (isSVGUseElement(*element)) {
         SVGUseElement* use = toSVGUseElement(element);
         ASSERT(!use->resourceIsStillLoading());
 
-        ASSERT(referencedScope());
-        Element* targetElement = SVGURIReference::targetElementFromIRIString(use->hrefString(), *referencedScope());
         SVGElement* target = 0;
-        if (targetElement && targetElement->isSVGElement())
-            target = toSVGElement(targetElement);
+        foundProblem = hasCycleUseReferencing(toSVGUseElement(use->correspondingElement()), use, target);
+        if (foundProblem)
+            return;
 
+        if (target && isDisallowedElement(target)) {
+            foundProblem = true;
+            return;
+        }
         // Don't ASSERT(target) here, it may be "pending", too.
         // Setup sub-shadow tree root node
         RefPtrWillBeRawPtr<SVGGElement> cloneParent = SVGGElement::create(referencedScope()->document());
-        use->cloneChildNodes(cloneParent.get());
+        for (Node* child = use->firstChild(); child;) {

[pdr., 2014/06/05 04:40:01]

This doesn't look right. I think you just want:

for (Node* child = use->firstChild(); child; child = child->nextSibling()
    cloneParent->appendChild(child);

+            Node* oldChild = child;
+            child = child->nextSibling();
+            cloneParent->appendChild(oldChild);
+        }
+        cloneParent->setCorrespondingElement(use->correspondingElement());
 
         // Spec: In the generated content, the 'use' will be replaced by 'g', where all attributes from the
         // 'use' element except for x, y, width, height and xlink:href are transferred to the generated 'g' element.
         transferUseAttributesToReplacedElement(use, cloneParent.get());
 
-        if (target && !isDisallowedElement(target)) {
-            RefPtrWillBeRawPtr<Element> newChild = target->cloneElementWithChildren();
+        if (target) {
+            RefPtr<Node> newChild = cloneNodeAndAssociate(target);

[kouhei (in TOK), 2014/06/05 05:41:37]

oilpan: RefPtrWillBeRawPtr

             ASSERT(newChild->isSVGElement());
             transferUseWidthAndHeightIfNeeded(*use, toSVGElement(newChild.get()), *target);
             cloneParent->appendChild(newChild.release());
         }
 
         // We don't walk the target tree element-by-element, and clone each element,
         // but instead use cloneElementWithChildren(). This is an optimization for the common
         // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
         // Though if there are disallowed elements in the subtree, we have to remove them.
         // For instance: <use> on <g> containing <foreignObject> (indirect case).
         if (subtreeContainsDisallowedElement(cloneParent.get()))
             removeDisallowedElementsFromSubtree(*cloneParent);
 
-        RefPtr<Node> replacingElement(cloneParent.get());
+        RefPtr<SVGElement> replacingElement(cloneParent.get());

[haraken, 2014/06/05 05:49:00]

RefPtrWillBeRawPtr

 
         // Replace <use> with referenced content.
         ASSERT(use->parentNode());
         use->parentNode()->replaceChild(cloneParent.release(), use);
 
         // Expand the siblings because the *element* is replaced and we will
         // lose the sibling chain when we are back from recursion.
         element = replacingElement.get();
-        for (RefPtr<Node> sibling = element->nextSibling(); sibling; sibling = sibling->nextSibling())
-            expandUseElementsInShadowTree(sibling.get());
+        for (RefPtr<SVGElement> sibling = Traversal<SVGElement>::nextSibling(*element); sibling; sibling = Traversal<SVGElement>::nextSibling(*sibling)) {

[pdr., 2014/06/05 04:40:01]

I think you want:
RefPtr<SVGElement> sibling = Traversal<SVGElement>::firstChild(*element)

[haraken, 2014/06/05 05:49:00]

RefPtrWillBeRawPtr

+            expandUseElementsInShadowTree(sibling.get(), foundProblem);
+            if (foundProblem)
+                return;
+        }
     }
 
-    for (RefPtr<Node> child = element->firstChild(); child; child = child->nextSibling())
-        expandUseElementsInShadowTree(child.get());
+    for (RefPtr<SVGElement> child = Traversal<SVGElement>::firstChild(*element); child; child = Traversal<SVGElement>::nextSibling(*child)) {

[kouhei (in TOK), 2014/06/05 05:41:37]

I think you can also use SVGElement* here.
If not, oilpan nit: RefPtrWillBeRawPtr.

+        expandUseElementsInShadowTree(child.get(), foundProblem);
+        if (foundProblem)
+            return;
+    }
 }
 
-void SVGUseElement::expandSymbolElementsInShadowTree(Node* element)
+void SVGUseElement::expandSymbolElementsInShadowTree(SVGElement* element)
 {
     ASSERT(element);
     if (isSVGSymbolElement(*element)) {
         // Spec: The referenced 'symbol' and its contents are deep-cloned into the generated tree,
         // with the exception that the 'symbol' is replaced by an 'svg'. This generated 'svg' will
         // always have explicit values for attributes width and height. If attributes width and/or
         // height are provided on the 'use' element, then these attributes will be transferred to
         // the generated 'svg'. If attributes width and/or height are not specified, the generated
         // 'svg' element will use values of 100% for these attributes.
         ASSERT(referencedScope());
         RefPtrWillBeRawPtr<SVGSVGElement> svgElement = SVGSVGElement::create(referencedScope()->document());
 
         // Transfer all data (attributes, etc.) from <symbol> to the new <svg> element.
-        svgElement->cloneDataFromElement(*toElement(element));
+        svgElement->cloneDataFromElement(*element);
+        svgElement->setCorrespondingElement(element->correspondingElement());
 
         // Only clone symbol children, and add them to the new <svg> element
-        for (Node* child = element->firstChild(); child; child = child->nextSibling()) {
-            RefPtr<Node> newChild = child->cloneNode(true);
-            svgElement->appendChild(newChild.release());
+        for (Node* child = element->firstChild(); child; ) {

[pdr., 2014/06/05 04:40:01]

Same issue with this loop as the one above. I don't think the oldChild part is
necessary.

+            Node* oldChild = child;
+            child = child->nextSibling();
+            svgElement->appendChild(oldChild);
         }
 
         // We don't walk the target tree element-by-element, and clone each element,
         // but instead use cloneNode(deep=true). This is an optimization for the common
         // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
         // Though if there are disallowed elements in the subtree, we have to remove them.
         // For instance: <use> on <g> containing <foreignObject> (indirect case).
         if (subtreeContainsDisallowedElement(svgElement.get()))
             removeDisallowedElementsFromSubtree(*svgElement);
 
-        RefPtr<Node> replacingElement(svgElement.get());
+        RefPtr<SVGElement> replacingElement(svgElement.get());

[haraken, 2014/06/05 05:49:00]

RefPtrWillBeRawPtr

 
         // Replace <symbol> with <svg>.
+        ASSERT(element->parentNode());
         element->parentNode()->replaceChild(svgElement.release(), element);
 
         // Expand the siblings because the *element* is replaced and we will
         // lose the sibling chain when we are back from recursion.
         element = replacingElement.get();
-        for (RefPtr<Node> sibling = element->nextSibling(); sibling; sibling = sibling->nextSibling())
+        for (RefPtr<SVGElement> sibling = Traversal<SVGElement>::nextSibling(*element); sibling; sibling = Traversal<SVGElement>::nextSibling(*sibling))

[pdr., 2014/06/05 04:40:01]

Should the first part of the foor lop be firstChild and not nextSibling as well?

[kouhei (in TOK), 2014/06/05 05:41:37]

Ditto: RefPtr<SVGElement> -> SVGElement*

             expandSymbolElementsInShadowTree(sibling.get());
     }
 
-    for (RefPtr<Node> child = element->firstChild(); child; child = child->nextSibling())
+    for (RefPtr<SVGElement> child = Traversal<SVGElement>::firstChild(*element); child; child = Traversal<SVGElement>::nextSibling(*child))

[kouhei (in TOK), 2014/06/05 05:41:37]

Ditto.

         expandSymbolElementsInShadowTree(child.get());
 }
 
-void SVGUseElement::transferEventListenersToShadowTree(SVGElement* shadowTreeTargetElement)
-{
-    if (!shadowTreeTargetElement)
-        return;
-
-    SVGElement* originalElement = shadowTreeTargetElement->correspondingElement();
-    ASSERT(originalElement);
-    if (EventTargetData* data = originalElement->eventTargetData())
-        data->eventListenerMap.copyEventListenersNotCreatedFromMarkupToTarget(shadowTreeTargetElement);
-
-    for (SVGElement* child = Traversal<SVGElement>::firstChild(*shadowTreeTargetElement); child; child = Traversal<SVGElement>::nextSibling(*child))
-        transferEventListenersToShadowTree(child);
-}
-
-void SVGUseElement::associateInstancesWithShadowTreeElements(Node* target, SVGElementInstance* targetInstance)
-{
-    if (!target || !targetInstance)
-        return;
-
-    SVGElement* originalElement = targetInstance->correspondingElement();
-    ASSERT(originalElement);
-    if (isSVGUseElement(*originalElement)) {
-        // <use> gets replaced by <g>
-        ASSERT(AtomicString(target->nodeName()) == SVGNames::gTag);
-    } else if (isSVGSymbolElement(*originalElement)) {
-        // <symbol> gets replaced by <svg>
-        ASSERT(AtomicString(target->nodeName()) == SVGNames::svgTag);
-    } else {
-        ASSERT(AtomicString(target->nodeName()) == originalElement->nodeName());
-    }
-
-    SVGElement* element = 0;
-    if (target->isSVGElement())
-        element = toSVGElement(target);
-
-    ASSERT(!targetInstance->shadowTreeElement());
-    targetInstance->setShadowTreeElement(element);
-    element->setCorrespondingElement(originalElement);
-
-    SVGElement* child = Traversal<SVGElement>::firstChild(*target);
-    for (SVGElementInstance* instance = targetInstance->firstChild(); child && instance; instance = instance->nextSibling()) {
-        associateInstancesWithShadowTreeElements(child, instance);
-        child = Traversal<SVGElement>::nextSibling(*child);
-    }
-}
-
 void SVGUseElement::invalidateShadowTree()
 {
     if (!inActiveDocument() || m_needsShadowTreeRecreation)
         return;
     scheduleShadowTreeRecreation();
     invalidateDependentShadowTrees();
 }
 
 void SVGUseElement::invalidateDependentShadowTrees()
 {
@@ skipping 26 matching lines @@
 {
     if (m_x->currentValue()->isRelative()
         || m_y->currentValue()->isRelative()
         || m_width->currentValue()->isRelative()
         || m_height->currentValue()->isRelative())
         return true;
 
     if (!m_targetElementInstance)
         return false;
 
-    SVGElement* element = m_targetElementInstance->shadowTreeElement();
-    if (!element)
-        return false;
-
-    return element->hasRelativeLengths();
+    return m_targetElementInstance->hasRelativeLengths();
 }
 
 void SVGUseElement::notifyFinished(Resource* resource)
 {
     if (!inDocument())
         return;
 
     invalidateShadowTree();
     if (resource->errorOccurred())
         dispatchEvent(Event::create(EventTypeNames::error));
     else if (!resource->wasCanceled()) {
         if (m_haveFiredLoadEvent)
             return;
         if (!isStructurallyExternal())
             return;
         ASSERT(!m_haveFiredLoadEvent);
         m_haveFiredLoadEvent = true;
         sendSVGLoadEventIfPossibleAsynchronously();
     }
 }
 
 bool SVGUseElement::resourceIsStillLoading()
 {
     if (m_resource && m_resource->isLoading())
         return true;
     return false;
 }
 
-bool SVGUseElement::instanceTreeIsLoading(SVGElementInstance* targetElementInstance)
+bool SVGUseElement::instanceTreeIsLoading(SVGElement* targetInstance)
 {
-    for (SVGElementInstance* instance = targetElementInstance->firstChild(); instance; instance = instance->nextSibling()) {
-        if (SVGUseElement* use = instance->correspondingUseElement()) {
+    for (SVGElement* element = Traversal<SVGElement>::firstChild(*targetInstance); element; element = Traversal<SVGElement>::nextSibling(*element)) {
+        if (SVGUseElement* use = element->correspondingUseElement()) {
             if (use->resourceIsStillLoading())
                 return true;
         }
-        if (instance->hasChildren())
-            instanceTreeIsLoading(instance);
+        if (element->hasChildren())
+            instanceTreeIsLoading(element);

[esprehn, 2014/06/05 06:29:16]

You're missing a return or something here, this does nothing since it ignores
the return value.

I'm pretty sure this was supposed to be:

if (element->hasChildren() && instanceTreeIsLoading(element))
  return true;

     }
     return false;
 }
 
 void SVGUseElement::setDocumentResource(ResourcePtr<DocumentResource> resource)
 {
     if (m_resource == resource)
         return;
 
     if (m_resource)
         m_resource->removeClient(this);
 
     m_resource = resource;
     if (m_resource)
         m_resource->addClient(this);
 }
 
 void SVGUseElement::trace(Visitor* visitor)
 {
     visitor->trace(m_targetElementInstance);
     SVGGraphicsElement::trace(visitor);
 }
 
 }