Use IdTargetObserver in SVGUseElement

Change SVGUseElement to use the observeTarget() helper from
SVGURIReference. Since SVGUseElement has some additional requirements
for its reference management, a more low-level observeTarget() variant
is exposed.
To facilitate this change, clearShadowTree() is renamed to
clearResourceReference(), and the shadow tree tear-down is hoisted out
of it.

BUG=661598

Review-Url: https://codereview.chromium.org/2744613002
Cr-Commit-Position: refs/heads/master@{#455769}

third_party/WebKit/Source/core/svg/SVGURIReference.cpp

@@ -21,6 +21,7 @@
 #include "core/svg/SVGURIReference.h"
 
 #include "core/XLinkNames.h"
+#include "core/dom/Document.h"
 #include "core/dom/IdTargetObserver.h"
 #include "core/html/parser/HTMLParserIdioms.h"
 #include "core/svg/SVGElement.h"
@@ -117,15 +118,21 @@ Element* SVGURIReference::targetElementFromIRIString(
 
 Element* SVGURIReference::observeTarget(Member<IdTargetObserver>& observer,
                                         SVGElement& contextElement) {
-  DCHECK(!observer);
   TreeScope& treeScope = contextElement.treeScope();
   AtomicString id = fragmentIdentifierFromIRIString(hrefString(), treeScope);
+  return observeTarget(observer, treeScope, id,
+                       WTF::bind(&SVGElement::buildPendingResource,
+                                 wrapWeakPersistent(&contextElement)));
+}
+
+Element* SVGURIReference::observeTarget(Member<IdTargetObserver>& observer,
+                                        TreeScope& treeScope,
+                                        const AtomicString& id,
+                                        std::unique_ptr<WTF::Closure> closure) {
+  DCHECK(!observer);
   if (id.isEmpty())
     return nullptr;
-  observer = new SVGElementReferenceObserver(
-      treeScope, id,
-      WTF::bind(&SVGElement::buildPendingResource,
-                wrapWeakPersistent(&contextElement)));
+  observer = new SVGElementReferenceObserver(treeScope, id, std::move(closure));
   return treeScope.getElementById(id);
 }
 

third_party/WebKit/Source/core/svg/SVGURIReference.h

@@ -23,13 +23,13 @@
 
 #include <memory>
 #include "core/CoreExport.h"
-#include "core/dom/Document.h"
 #include "core/svg/SVGAnimatedHref.h"
 #include "platform/heap/Handle.h"
 #include "wtf/Functional.h"
 
 namespace blink {
 
+class Document;
 class Element;
 class IdTargetObserver;
 
@@ -61,6 +61,12 @@ class CORE_EXPORT SVGURIReference : public GarbageCollectedMixin {
   // |contextElement|.) Will call buildPendingResource() on |contextElement|
   // when changes to the 'id' are noticed.
   Element* observeTarget(Member<IdTargetObserver>&, SVGElement&);
+  // Create an 'id' observer for |id| in the specified TreeScope. On changes,
+  // the passed Closure will be called.
+  static Element* observeTarget(Member<IdTargetObserver>&,
+                                TreeScope&,
+                                const AtomicString& id,
+                                std::unique_ptr<WTF::Closure>);
   // Unregister and destroy the observer.
   static void unobserveTarget(Member<IdTargetObserver>&);
 

third_party/WebKit/Source/core/svg/SVGUseElement.cpp

@@ -25,14 +25,13 @@
 
 #include "core/svg/SVGUseElement.h"
 
-#include "bindings/core/v8/ExceptionState.h"
 #include "core/SVGNames.h"
 #include "core/XLinkNames.h"
 #include "core/dom/Document.h"
 #include "core/dom/ElementTraversal.h"
+#include "core/dom/IdTargetObserver.h"
 #include "core/dom/StyleChangeReason.h"
 #include "core/dom/TaskRunnerHelper.h"
-#include "core/dom/shadow/ElementShadow.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/events/Event.h"
 #include "core/layout/svg/LayoutSVGTransformableContainer.h"
@@ -41,7 +40,6 @@
 #include "core/svg/SVGSVGElement.h"
 #include "core/svg/SVGSymbolElement.h"
 #include "core/svg/SVGTitleElement.h"
-#include "core/svg/SVGTreeScopeResources.h"
 #include "core/xml/parser/XMLDocumentParser.h"
 #include "platform/loader/fetch/FetchRequest.h"
 #include "platform/loader/fetch/ResourceFetcher.h"
@@ -98,6 +96,7 @@ DEFINE_TRACE(SVGUseElement) {
   visitor->trace(m_width);
   visitor->trace(m_height);
   visitor->trace(m_targetElementInstance);
+  visitor->trace(m_targetIdObserver);
   visitor->trace(m_resource);
   SVGGraphicsElement::trace(visitor);
   SVGURIReference::trace(visitor);
@@ -128,8 +127,7 @@ Node::InsertionNotificationRequest SVGUseElement::insertedInto(
 void SVGUseElement::removedFrom(ContainerNode* rootParent) {
   SVGGraphicsElement::removedFrom(rootParent);
   if (rootParent->isConnected()) {
-    clearInstanceRoot();
-    removeAllOutgoingReferences();
+    clearResourceReference();
     cancelShadowTreeRecreation();
   }
 }
@@ -287,52 +285,40 @@ void SVGUseElement::cancelShadowTreeRecreation() {
 }
 
 void SVGUseElement::clearInstanceRoot() {
-  if (m_targetElementInstance)
-    m_targetElementInstance = nullptr;
+  m_targetElementInstance = nullptr;
 }
 
-void SVGUseElement::clearShadowTree() {
+void SVGUseElement::clearResourceReference() {
+  unobserveTarget(m_targetIdObserver);
   clearInstanceRoot();
-
-  // FIXME: We should try to optimize this, to at least allow partial reclones.
-  if (ShadowRoot* shadowTreeRootElement = userAgentShadowRoot())
-    shadowTreeRootElement->removeChildren(OmitSubtreeModifiedEvent);
-
   removeAllOutgoingReferences();
 }
 
 Element* SVGUseElement::resolveTargetElement() {
   if (m_elementIdentifier.isEmpty())
     return nullptr;
-  const TreeScope* lookupScope = nullptr;
-  if (m_elementIdentifierIsLocal)
-    lookupScope = &treeScope();
-  else if (resourceIsValid())
-    lookupScope = m_resource->document();
-  else
-    return nullptr;
-  Element* target = lookupScope->getElementById(m_elementIdentifier);
-  // TODO(fs): Why would the Element not be "connected" at this point?
-  if (target && target->isConnected())
-    return target;
-  // Don't record any pending references for external resources.
-  if (!m_resource) {
-    treeScope().ensureSVGTreeScopedResources().addPendingResource(
-        m_elementIdentifier, *this);
-    DCHECK(hasPendingResources());
+  if (m_elementIdentifierIsLocal) {
+    return observeTarget(m_targetIdObserver, treeScope(), m_elementIdentifier,
+                         WTF::bind(&SVGUseElement::invalidateShadowTree,
+                                   wrapWeakPersistent(this)));
   }
-  return nullptr;
+  if (!resourceIsValid())
+    return nullptr;
+  return m_resource->document()->getElementById(m_elementIdentifier);
 }
 
 void SVGUseElement::buildPendingResource() {
   if (inUseShadowTree())
     return;
-  clearShadowTree();
+  // FIXME: We should try to optimize this, to at least allow partial reclones.
+  userAgentShadowRoot()->removeChildren(OmitSubtreeModifiedEvent);
+  clearResourceReference();
   cancelShadowTreeRecreation();
   if (!isConnected())
     return;
   Element* target = resolveTargetElement();
-  if (target && target->isSVGElement()) {
+  // TODO(fs): Why would the Element not be "connected" at this point?
+  if (target && target->isConnected() && target->isSVGElement()) {
     buildShadowAndInstanceTree(toSVGElement(*target));
     invalidateDependentShadowTrees();
   }
@@ -463,7 +449,8 @@ void SVGUseElement::buildShadowAndInstanceTree(SVGElement& target) {
   // Expand all <use> elements in the shadow tree.
   // Expand means: replace the actual <use> element by what it references.
   if (!expandUseElementsInShadowTree()) {
-    clearShadowTree();
+    shadowTreeRootElement->removeChildren(OmitSubtreeModifiedEvent);
+    clearResourceReference();
     return;
   }
 

third_party/WebKit/Source/core/svg/SVGUseElement.h

@@ -94,7 +94,7 @@ class SVGUseElement final : public SVGGraphicsElement,
   void buildShadowAndInstanceTree(SVGElement& target);
   void clearInstanceRoot();
   Element* createInstanceTree(SVGElement& targetRoot) const;
-  void clearShadowTree();
+  void clearResourceReference();
   bool hasCycleUseReferencing(const SVGUseElement&,
                               const ContainerNode& targetInstance,
                               SVGElement*& newTarget) const;
@@ -121,6 +121,7 @@ class SVGUseElement final : public SVGGraphicsElement,
   bool m_haveFiredLoadEvent;
   bool m_needsShadowTreeRecreation;
   Member<SVGElement> m_targetElementInstance;
+  Member<IdTargetObserver> m_targetIdObserver;
   Member<DocumentResource> m_resource;
 };