Bug 635511 - Improve performance getting parent SVG elements r=jwatt

content/svg/content/src/nsSVGAnimationElement.cpp

@@ -270,16 +270,14 @@ nsSVGAnimationElement::BindToTree(nsIDocument* aDocument,
                                                       aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv,rv);
 
-  // XXXdholbert is ownerDOMSVG (as a check for SVG parent) still needed here?
-  nsCOMPtr<nsIDOMSVGSVGElement> ownerDOMSVG;
-  rv = GetOwnerSVGElement(getter_AddRefs(ownerDOMSVG));
-
-  if (NS_FAILED(rv) || !ownerDOMSVG)
+  // XXXdholbert is GetCtx (as a check for SVG parent) still needed here?
+  if (!GetCtx()) {
     // No use proceeding. We don't have an SVG parent (yet) so we won't be able
     // to register ourselves etc. Maybe next time we'll have more luck.
     // (This sort of situation will arise a lot when trees are being constructed
     // piece by piece via script)
     return NS_OK;
+  }
 
   // Add myself to the animation controller's master set of animation elements.
   if (aDocument) {
@@ -420,18 +418,13 @@ nsSVGAnimationElement::IsNodeOfType(PRUint32 aFlags) const
 nsSMILTimeContainer*
 nsSVGAnimationElement::GetTimeContainer()
 {
-  nsSMILTimeContainer *result = nsnull;
-  nsCOMPtr<nsIDOMSVGSVGElement> ownerDOMSVG;
+  nsSVGSVGElement *element = nsSVGUtils::GetOuterSVGElement(this);
 
-  nsresult rv = GetOwnerSVGElement(getter_AddRefs(ownerDOMSVG));
-
-  if (NS_SUCCEEDED(rv) && ownerDOMSVG) {
-    nsSVGSVGElement *ownerSVG =
-      static_cast<nsSVGSVGElement*>(ownerDOMSVG.get());
-    result = ownerSVG->GetTimedDocumentRoot();
+  if (element) {
+    return element->GetTimedDocumentRoot();
   }
 
-  return result;
+  return nsnull;
 }
 
 // nsIDOMElementTimeControl

content/svg/content/src/nsSVGElement.cpp

@@ -1024,34 +1024,13 @@ NS_IMETHODIMP nsSVGElement::SetId(const nsAString & aId)
 NS_IMETHODIMP
 nsSVGElement::GetOwnerSVGElement(nsIDOMSVGSVGElement * *aOwnerSVGElement)
 {
-  *aOwnerSVGElement = nsnull;
+  NS_IF_ADDREF(*aOwnerSVGElement = GetCtx());
 
-  nsIContent* ancestor = nsSVGUtils::GetParentElement(this);
-
-  while (ancestor && ancestor->GetNameSpaceID() == kNameSpaceID_SVG) {
-    nsIAtom* tag = ancestor->Tag();
-    if (tag == nsGkAtoms::foreignObject) {
-      // SVG in a foreignObject must have its own <svg> (nsSVGOuterSVGFrame).
-      // Leave *aOwnerSVGElement nulled out, but don't throw.
-      return NS_OK;
-    }
-    if (tag == nsGkAtoms::svg) {
-      *aOwnerSVGElement = static_cast<nsSVGSVGElement*>(ancestor);
-      NS_ADDREF(*aOwnerSVGElement);
-      return NS_OK;
-    }
-    ancestor = nsSVGUtils::GetParentElement(ancestor);
-  }
-
-  // we don't have a parent SVG element...
-
-  // are _we_ the outermost SVG element? If yes, return nsnull, but don't fail
-  if (Tag() == nsGkAtoms::svg) {
+  if (*aOwnerSVGElement || Tag() == nsGkAtoms::svg) {
+    // If we found something or we're the outermost SVG element, that's OK.
     return NS_OK;
   }
-  
-  // no owner found and we aren't the outermost SVG element either.
-  // this situation can e.g. occur during content tree teardown. 
+  // Otherwise, we've got an invalid structure
   return NS_ERROR_FAILURE;
 }
 
@@ -1432,9 +1411,21 @@ nsIAtom* nsSVGElement::GetEventNameForAttr(nsIAtom* aAttr)
 nsSVGSVGElement *
 nsSVGElement::GetCtx()
 {
-  nsCOMPtr<nsIDOMSVGSVGElement> svg;
-  GetOwnerSVGElement(getter_AddRefs(svg));
-  return static_cast<nsSVGSVGElement*>(svg.get());
+  dom::Element* ancestor = nsSVGUtils::GetParentElement(this);
+
+  while (ancestor && ancestor->GetNameSpaceID() == kNameSpaceID_SVG) {
+    nsIAtom* tag = ancestor->Tag();
+    if (tag == nsGkAtoms::foreignObject) {
+      return nsnull;
+    }
+    if (tag == nsGkAtoms::svg) {
+      return static_cast<nsSVGSVGElement*>(ancestor);
+    }
+    ancestor = nsSVGUtils::GetParentElement(ancestor);
+  }
+
+  // we don't have an ancestor <svg> element...
+  return nsnull;
 }
 
 /* virtual */ gfxMatrix

content/svg/content/src/nsSVGGraphicElement.cpp

@@ -38,6 +38,7 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsSVGGraphicElement.h"
+#include "nsSVGSVGElement.h"
 #include "nsSVGTransformList.h"
 #include "nsSVGAnimatedTransformList.h"
 #include "nsGkAtoms.h"
@@ -82,7 +83,7 @@ NS_IMETHODIMP nsSVGGraphicElement::GetNearestViewportElement(nsIDOMSVGElement *
 /* readonly attribute nsIDOMSVGElement farthestViewportElement; */
 NS_IMETHODIMP nsSVGGraphicElement::GetFarthestViewportElement(nsIDOMSVGElement * *aFarthestViewportElement)
 {
-  *aFarthestViewportElement = nsSVGUtils::GetFarthestViewportElement(this).get();
+  NS_IF_ADDREF(*aFarthestViewportElement = nsSVGUtils::GetOuterSVGElement(this));
   return NS_OK;
 }
 

content/svg/content/src/nsSVGSVGElement.cpp

@@ -639,7 +639,7 @@ nsSVGSVGElement::CreateSVGNumber(nsIDOMSVGNumber **_retval)
 NS_IMETHODIMP
 nsSVGSVGElement::CreateSVGLength(nsIDOMSVGLength **_retval)
 {
-  NS_IF_ADDREF(*_retval = new DOMSVGLength());
+  NS_ADDREF(*_retval = new DOMSVGLength());
   return NS_OK;
 }
 
@@ -734,7 +734,7 @@ nsSVGSVGElement::GetNearestViewportElement(nsIDOMSVGElement * *aNearestViewportE
 NS_IMETHODIMP
 nsSVGSVGElement::GetFarthestViewportElement(nsIDOMSVGElement * *aFarthestViewportElement)
 {
-  *aFarthestViewportElement = nsSVGUtils::GetFarthestViewportElement(this).get();
+  NS_IF_ADDREF(*aFarthestViewportElement = nsSVGUtils::GetOuterSVGElement(this));
   return NS_OK;
 }
 
@@ -882,24 +882,19 @@ nsSVGSVGElement::SetCurrentTranslate(float x, float y)
 nsSMILTimeContainer*
 nsSVGSVGElement::GetTimedDocumentRoot()
 {
-  nsSMILTimeContainer *result = nsnull;
-
   if (mTimedDocumentRoot) {
-    result = mTimedDocumentRoot;
-  } else {
-    // We must not be the outermost SVG element, try to find it
-    nsCOMPtr<nsIDOMSVGSVGElement> outerSVGDOM;
-
-    nsresult rv = GetOwnerSVGElement(getter_AddRefs(outerSVGDOM));
-
-    if (NS_SUCCEEDED(rv) && outerSVGDOM) {
-      nsSVGSVGElement *outerSVG =
-        static_cast<nsSVGSVGElement*>(outerSVGDOM.get());
-      result = outerSVG->GetTimedDocumentRoot();
-    }
+    return mTimedDocumentRoot;
   }
 
-  return result;
+  // We must not be the outermost <svg> element, try to find it
+  nsSVGSVGElement *outerSVGElement =
+    nsSVGUtils::GetOuterSVGElement(this);
+
+  if (outerSVGElement) {
+    return outerSVGElement->GetTimedDocumentRoot();
+  }
+  // invalid structure
+  return nsnull;
 }
 #endif // MOZ_SMIL
 
@@ -996,7 +991,7 @@ nsSVGSVGElement::GetViewBoxTransform()
   SVGPreserveAspectRatio tmpPAR;
 
   float viewportWidth, viewportHeight;
-  if (nsSVGUtils::IsInnerSVG(this)) {
+  if (IsInner()) {
     nsSVGSVGElement *ctx = GetCtx();
     viewportWidth = mLengthAttributes[WIDTH].GetAnimValue(ctx);
     viewportHeight = mLengthAttributes[HEIGHT].GetAnimValue(ctx);
@@ -1169,7 +1164,7 @@ nsSVGSVGElement::GetLength(PRUint8 aCtxType)
     const nsSVGViewBoxRect& viewbox = mViewBox.GetAnimValue();
     w = viewbox.width;
     h = viewbox.height;
-  } else if (nsSVGUtils::IsInnerSVG(this)) {
+  } else if (IsInner()) {
     nsSVGSVGElement *ctx = GetCtx();
     w = mLengthAttributes[WIDTH].GetAnimValue(ctx);
     h = mLengthAttributes[HEIGHT].GetAnimValue(ctx);
@@ -1203,7 +1198,7 @@ nsSVGSVGElement::GetLength(PRUint8 aCtxType)
 /* virtual */ gfxMatrix
 nsSVGSVGElement::PrependLocalTransformTo(const gfxMatrix &aMatrix)
 {
-  if (nsSVGUtils::IsInnerSVG(this)) {
+  if (IsInner()) {
     float x, y;
     GetAnimatedLengthValues(&x, &y, nsnull);
     return GetViewBoxTransform() * gfxMatrix().Translate(gfxPoint(x, y)) * aMatrix;

content/svg/content/src/nsSVGSVGElement.h

@@ -57,10 +57,6 @@
 class nsSMILTimeContainer;
 #endif // MOZ_SMIL
 
-#define QI_AND_CAST_TO_NSSVGSVGELEMENT(base)                                  \
-  (nsCOMPtr<nsIDOMSVGSVGElement>(do_QueryInterface(base)) ?                   \
-   static_cast<nsSVGSVGElement*>(base.get()) : nsnull)
-
 typedef nsSVGStylableElement nsSVGSVGElementBase;
 
 class nsSVGSVGElement;
@@ -271,6 +267,16 @@ protected:
     return IsInDoc() && !GetParent();
   }
 
+  /**
+   * Returns true if this is an SVG <svg> element that is the child of
+   * another non-foreignObject SVG element.
+   */
+  PRBool IsInner() {
+    const mozilla::dom::Element *parent = nsSVGUtils::GetParentElement(this);
+    return parent && parent->GetNameSpaceID() == kNameSpaceID_SVG &&
+           parent->Tag() != nsGkAtoms::foreignObject;
+  }
+
 #ifdef MOZ_SMIL
   /* 
    * While binding to the tree we need to determine if we will be the outermost

layout/svg/base/src/nsSVGUtils.cpp

@@ -223,6 +223,24 @@ nsSVGUtils::GetParentElement(nsIContent *aContent)
   return parent && parent->IsElement() ? parent->AsElement() : nsnull;
 }
 
+nsSVGSVGElement*
+nsSVGUtils::GetOuterSVGElement(nsSVGElement *aSVGElement)
+{
+  nsIContent *element = nsnull;
+  nsIContent *ancestor = GetParentElement(aSVGElement);
+
+  while (ancestor && ancestor->GetNameSpaceID() == kNameSpaceID_SVG &&
+                     ancestor->Tag() != nsGkAtoms::foreignObject) {
+    element = ancestor;
+    ancestor = GetParentElement(element);
+  }
+
+  if (element && element->Tag() == nsGkAtoms::svg) {
+    return static_cast<nsSVGSVGElement*>(element);
+  }
+  return nsnull;
+}
+
 float
 nsSVGUtils::GetFontSize(Element *aElement)
 {
@@ -461,24 +479,6 @@ nsSVGUtils::GetNearestViewportElement(nsIContent *aContent)
   return nsnull;
 }
 
-already_AddRefed<nsIDOMSVGElement>
-nsSVGUtils::GetFarthestViewportElement(nsIContent *aContent)
-{
-  nsIContent *element = nsnull;
-  nsIContent *ancestor = GetParentElement(aContent);
-
-  while (ancestor && ancestor->GetNameSpaceID() == kNameSpaceID_SVG &&
-                     ancestor->Tag() != nsGkAtoms::foreignObject) {
-    element = ancestor;
-    ancestor = GetParentElement(element);
-  }
-
-  if (element && element->Tag() == nsGkAtoms::svg) {
-    return nsCOMPtr<nsIDOMSVGElement>(do_QueryInterface(element)).forget();
-  }
-  return nsnull;
-}
-
 gfxMatrix
 nsSVGUtils::GetCTM(nsSVGElement *aElement, PRBool aScreenCTM)
 {
@@ -1468,17 +1468,6 @@ nsSVGUtils::PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
   return strokeExtents;
 }
 
-/* static */ PRBool
-nsSVGUtils::IsInnerSVG(nsIContent* aContent)
-{
-  if (!aContent->NodeInfo()->Equals(nsGkAtoms::svg, kNameSpaceID_SVG)) {
-    return PR_FALSE;
-  }
-  nsIContent *ancestor = GetParentElement(aContent);
-  return ancestor && ancestor->GetNameSpaceID() == kNameSpaceID_SVG &&
-                     ancestor->Tag() != nsGkAtoms::foreignObject;
-}
-
 // ----------------------------------------------------------------------
 
 nsSVGRenderState::nsSVGRenderState(nsIRenderingContext *aContext) :

layout/svg/base/src/nsSVGUtils.h

@@ -228,6 +228,11 @@ public:
    */
   static mozilla::dom::Element *GetParentElement(nsIContent *aContent);
 
+  /*
+   * Get the outer SVG element of an nsIContent
+   */
+  static nsSVGSVGElement *GetOuterSVGElement(nsSVGElement *aSVGElement);
+
   /*
    * Get the number of CSS px (user units) per em (i.e. the em-height in user
    * units) for an nsIContent
@@ -302,9 +307,6 @@ public:
   static already_AddRefed<nsIDOMSVGElement>
   GetNearestViewportElement(nsIContent *aContent);
 
-  static already_AddRefed<nsIDOMSVGElement>
-  GetFarthestViewportElement(nsIContent *aContent);
-
   /**
    * Gets the nearest nsSVGInnerSVGFrame or nsSVGOuterSVGFrame frame. aFrame
    * must be an SVG frame. If aFrame is of type nsGkAtoms::svgOuterSVGFrame,
@@ -586,12 +588,6 @@ public:
   static gfxRect PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
                                                nsSVGGeometryFrame* aFrame);
 
-  /**
-   * Returns true if aContent is an SVG <svg> element that is the child of
-   * another non-foreignObject SVG element.
-   */
-  static PRBool IsInnerSVG(nsIContent* aContent);
-
   /**
    * Convert a floating-point value to a 32-bit integer value, clamping to
    * the range of valid integers.