Revert "Remove SVGUseElement::target_element_instance_"

This reverts commit d030e251.

Reason for revert: Tickled ClusterFuzz

Original change's description:
> Remove SVGUseElement::target_element_instance_
> 
> This reference should always be the same as the first child of the
> shadow root. Since we don't make heavy use of it though - and it
> shouldn't be too slow to access via the ShadowRoot, just get rid of it
> to have less state to keep up-to-date.
> 
> Bug: 997176
> Change-Id: I798bbc7ac64934d5eda1250b32150b50b4d2acdb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771960
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: Fredrik Söderquist <fs@opera.com>
> Cr-Commit-Position: refs/heads/master@{#691722}

TBR=pdr@chromium.org,fs@opera.com

Change-Id: Ia7c4b5fde76e97a50e6c670622029750672ca0f0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 997176
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1777927Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#692034}

third_party/blink/renderer/core/svg/svg_use_element.cc

@@ -98,6 +98,7 @@ void SVGUseElement::Trace(blink::Visitor* visitor) {
   visitor->Trace(y_);
   visitor->Trace(width_);
   visitor->Trace(height_);
+  visitor->Trace(target_element_instance_);
   visitor->Trace(target_id_observer_);
   SVGGraphicsElement::Trace(visitor);
   SVGURIReference::Trace(visitor);
@@ -114,13 +115,15 @@ static inline bool IsWellFormedDocument(const Document& document) {
 
 Node::InsertionNotificationRequest SVGUseElement::InsertedInto(
     ContainerNode& root_parent) {
+  // This functions exists to assure assumptions made in the code regarding
+  // SVGElementInstance creation/destruction are satisfied.
   SVGGraphicsElement::InsertedInto(root_parent);
-  if (root_parent.isConnected()) {
-    InvalidateShadowTree();
+  if (!root_parent.isConnected())
+    return kInsertionDone;
 #if DCHECK_IS_ON()
-    DCHECK(!InstanceRoot() || !IsWellFormedDocument(GetDocument()));
+  DCHECK(!target_element_instance_ || !IsWellFormedDocument(GetDocument()));
 #endif
-  }
+  InvalidateShadowTree();
   return kInsertionDone;
 }
 
@@ -220,10 +223,11 @@ void SVGUseElement::SvgAttributeChanged(const QualifiedName& attr_name) {
     }
 
     UpdateRelativeLengthsInformation();
-    if (SVGElement* instance_root = InstanceRoot()) {
-      DCHECK(instance_root->CorrespondingElement());
-      TransferUseWidthAndHeightIfNeeded(*this, *instance_root,
-                                        *instance_root->CorrespondingElement());
+    if (target_element_instance_) {
+      DCHECK(target_element_instance_->CorrespondingElement());
+      TransferUseWidthAndHeightIfNeeded(
+          *this, *target_element_instance_,
+          *target_element_instance_->CorrespondingElement());
     }
 
     if (LayoutObject* object = GetLayoutObject())
@@ -280,8 +284,13 @@ void SVGUseElement::CancelShadowTreeRecreation() {
   GetDocument().UnscheduleUseShadowTreeUpdate(*this);
 }
 
+void SVGUseElement::ClearInstanceRoot() {
+  target_element_instance_ = nullptr;
+}
+
 void SVGUseElement::ClearResourceReference() {
   UnobserveTarget(target_id_observer_);
+  ClearInstanceRoot();
   RemoveAllOutgoingReferences();
 }
 
@@ -305,14 +314,6 @@ Element* SVGUseElement::ResolveTargetElement(ObserveBehavior observe_behavior) {
       ->getElementById(element_identifier);
 }
 
-SVGElement* SVGUseElement::InstanceRoot() const {
-  // The shadow tree is torn down lazily, so check if there's a pending rebuild
-  // or if we're disconnected from the document.
-  if (needs_shadow_tree_recreation_ || !isConnected())
-    return nullptr;
-  return To<SVGElement>(UseShadowRoot().firstChild());
-}
-
 void SVGUseElement::BuildPendingResource() {
   // This runs just before computed style is updated, so this SVGUseElement
   // should always be connected to the Document. It should also not be an
@@ -341,9 +342,9 @@ String SVGUseElement::title() const {
 
   // If there is no <title> child in <use>, we lookup first <title> child in
   // shadow tree.
-  if (SVGElement* instance_root = InstanceRoot()) {
+  if (target_element_instance_) {
     if (Element* title_element =
-            Traversal<SVGTitleElement>::FirstChild(*instance_root))
+            Traversal<SVGTitleElement>::FirstChild(*target_element_instance_))
       return title_element->innerText();
   }
   // Otherwise return a null string.
@@ -423,7 +424,7 @@ SVGElement* SVGUseElement::CreateInstanceTree(SVGElement& target_root) const {
 }
 
 void SVGUseElement::AttachShadowTree(SVGElement& target) {
-  DCHECK(!InstanceRoot());
+  DCHECK(!target_element_instance_);
   DCHECK(!needs_shadow_tree_recreation_);
   DCHECK(!InUseShadowTree());
 
@@ -434,7 +435,9 @@ void SVGUseElement::AttachShadowTree(SVGElement& target) {
   // Set up root SVG element in shadow tree.
   // Clone the target subtree into the shadow tree, not handling <use> and
   // <symbol> yet.
-  UseShadowRoot().AppendChild(CreateInstanceTree(target));
+  target_element_instance_ = CreateInstanceTree(target);
+  ShadowRoot& shadow_root = UseShadowRoot();
+  shadow_root.AppendChild(target_element_instance_);
 
   AddReferencesToFirstDegreeNestedUseElements(target);
 
@@ -446,14 +449,19 @@ void SVGUseElement::AttachShadowTree(SVGElement& target) {
   }
 
   // Assure shadow tree building was successful.
-  DCHECK(InstanceRoot());
-  DCHECK_EQ(InstanceRoot()->CorrespondingUseElement(), this);
-  DCHECK_EQ(InstanceRoot()->CorrespondingElement(), &target);
+  DCHECK(target_element_instance_);
+  DCHECK_EQ(target_element_instance_->CorrespondingUseElement(), this);
+  DCHECK_EQ(target_element_instance_->CorrespondingElement(), &target);
 
   // Expand all <use> elements in the shadow tree.
   // Expand means: replace the actual <use> element by what it references.
   ExpandUseElementsInShadowTree();
 
+  // If the instance root was a <use>, it could have been replaced now, so
+  // reset |m_targetElementInstance|.
+  target_element_instance_ = To<SVGElement>(shadow_root.firstChild());
+  DCHECK_EQ(target_element_instance_->parentNode(), shadow_root);
+
   PostProcessInstanceTree();
 
   // Update relative length information.
@@ -502,7 +510,8 @@ Path SVGUseElement::ToClipPath() const {
 
 SVGGraphicsElement* SVGUseElement::VisibleTargetGraphicsElementForClipping()
     const {
-  auto* svg_graphics_element = DynamicTo<SVGGraphicsElement>(InstanceRoot());
+  auto* svg_graphics_element =
+      DynamicTo<SVGGraphicsElement>(UseShadowRoot().firstChild());
   if (!svg_graphics_element)
     return nullptr;
 
@@ -628,6 +637,7 @@ void SVGUseElement::ExpandUseElementsInShadowTree() {
 void SVGUseElement::InvalidateShadowTree() {
   if (!InActiveDocument() || needs_shadow_tree_recreation_)
     return;
+  ClearInstanceRoot();
   ScheduleShadowTreeRecreation();
   InvalidateDependentShadowTrees();
 }
@@ -651,8 +661,11 @@ bool SVGUseElement::SelfHasRelativeLengths() const {
       width_->CurrentValue()->IsRelative() ||
       height_->CurrentValue()->IsRelative())
     return true;
-  SVGElement* instance_root = InstanceRoot();
-  return instance_root && instance_root->HasRelativeLengths();
+
+  if (!target_element_instance_)
+    return false;
+
+  return target_element_instance_->HasRelativeLengths();
 }
 
 FloatRect SVGUseElement::GetBBox() {

third_party/blink/renderer/core/svg/svg_use_element.h

@@ -22,8 +22,6 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_USE_ELEMENT_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_USE_ELEMENT_H_
 
-#include "base/gtest_prod_util.h"
-#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/loader/resource/document_resource.h"
 #include "third_party/blink/renderer/core/svg/svg_animated_length.h"
 #include "third_party/blink/renderer/core/svg/svg_geometry_element.h"
@@ -107,7 +105,7 @@ class SVGUseElement final : public SVGGraphicsElement,
   Element* ResolveTargetElement(ObserveBehavior);
   void AttachShadowTree(SVGElement& target);
   void DetachShadowTree();
-  CORE_EXPORT SVGElement* InstanceRoot() const;
+  void ClearInstanceRoot();
   SVGElement* CreateInstanceTree(SVGElement& target_root) const;
   void ClearResourceReference();
   bool HasCycleUseReferencing(const ContainerNode& target_instance,
@@ -134,12 +132,8 @@ class SVGUseElement final : public SVGGraphicsElement,
   bool element_url_is_local_;
   bool have_fired_load_event_;
   bool needs_shadow_tree_recreation_;
+  Member<SVGElement> target_element_instance_;
   Member<IdTargetObserver> target_id_observer_;
-
-  FRIEND_TEST_ALL_PREFIXES(SVGUseElementTest,
-                           NullInstanceRootWhenNotConnectedToDocument);
-  FRIEND_TEST_ALL_PREFIXES(SVGUseElementTest,
-                           NullInstanceRootWhenShadowTreePendingRebuild);
 };
 
 }  // namespace blink

third_party/blink/renderer/core/svg/svg_use_element_test.cc

@@ -74,44 +74,4 @@ TEST_F(SVGUseElementTest,
   ASSERT_FALSE(use->GetShadowRoot()->getElementById("target"));
 }
 
-TEST_F(SVGUseElementTest, NullInstanceRootWhenNotConnectedToDocument) {
-  GetDocument().body()->SetInnerHTMLFromString(R"HTML(
-    <svg>
-      <defs>
-        <rect id="r" width="100" height="100" fill="blue"/>
-      </defs>
-      <use id="target" href="#r"/>
-    </svg>
-  )HTML");
-  GetDocument().View()->UpdateAllLifecyclePhases(LifecycleUpdateReason::kTest);
-
-  auto* target = To<SVGUseElement>(GetDocument().getElementById("target"));
-  ASSERT_TRUE(target);
-  ASSERT_TRUE(target->InstanceRoot());
-
-  target->remove();
-
-  ASSERT_FALSE(target->InstanceRoot());
-}
-
-TEST_F(SVGUseElementTest, NullInstanceRootWhenShadowTreePendingRebuild) {
-  GetDocument().body()->SetInnerHTMLFromString(R"HTML(
-    <svg>
-      <defs>
-        <rect id="r" width="100" height="100" fill="blue"/>
-      </defs>
-      <use id="target" href="#r"/>
-    </svg>
-  )HTML");
-  GetDocument().View()->UpdateAllLifecyclePhases(LifecycleUpdateReason::kTest);
-
-  auto* target = To<SVGUseElement>(GetDocument().getElementById("target"));
-  ASSERT_TRUE(target);
-  ASSERT_TRUE(target->InstanceRoot());
-
-  GetDocument().getElementById("r")->setAttribute(html_names::kWidthAttr, "50");
-
-  ASSERT_FALSE(target->InstanceRoot());
-}
-
 }  // namespace blink