Per roc's review comment, have MappedAttrParser::CreateStyleRule return its creation directly, rather than by reference.
authorDaniel Holbert <dholbert@cs.stanford.edu>
Tue, 26 Jan 2010 03:13:21 -0800
changeset 1051 bbfa9bad84a0
parent 1050 61a18e016846
child 1052 08729fd43e2f
push id860
push userdholbert@mozilla.com
push dateTue, 26 Jan 2010 11:13:24 +0000
Per roc's review comment, have MappedAttrParser::CreateStyleRule return its creation directly, rather than by reference.
smil_mappedAttrParser
smil_mappedAttrs
--- a/smil_mappedAttrParser
+++ b/smil_mappedAttrParser
@@ -1,14 +1,14 @@
 From: Daniel Holbert <dholbert@cs.stanford.edu>
 
 diff --git a/content/svg/content/src/nsSVGElement.cpp b/content/svg/content/src/nsSVGElement.cpp
 --- a/content/svg/content/src/nsSVGElement.cpp
 +++ b/content/svg/content/src/nsSVGElement.cpp
-@@ -994,97 +994,158 @@ nsSVGElement::DidModifySVGObservable(nsI
+@@ -994,97 +994,162 @@ nsSVGElement::DidModifySVGObservable(nsI
  // Implementation Helpers:
  
  PRBool
  nsSVGElement::IsEventName(nsIAtom* aName)
  {
    return PR_FALSE;
  }
  
@@ -20,19 +20,20 @@ diff --git a/content/svg/content/src/nsS
 +                   nsIPrincipal* aNodePrincipal);
 +  ~MappedAttrParser();
 +
 +  // Parses a mapped attribute value. Returns PR_FALSE on failure.
 +  // (i.e. if we fail to allocate a nsCSSDeclaration or to obtain a parser.)
 +  PRBool ParseMappedAttrValue(nsIAtom* aMappedAttrName,
 +                              nsAString& aMappedAttrValue);
 +
-+  // If we've parsed any values for mapped attributes, this method will create
-+  // a nsICSSStyleRule for their parsed values.  Otherwise, it does nothing.
-+  void CreateStyleRule(nsICSSStyleRule** aInstancePtrResult);
++  // If we've parsed any values for mapped attributes, this method returns
++  // a new already_AddRefed nsICSSStyleRule that incorporates the parsed
++  // values. Otherwise, this method returns null. 
++  already_AddRefed<nsICSSStyleRule> CreateStyleRule();
 +
 +private:
 +  // Lazy-initialize mDecl & mParser once we actually need them.
 +  PRBool LazyInit();
 +
 +  // MEMBER DATA
 +  nsIDocument*     mDoc;
 +  // Arguments for nsICSSParser::ParseProperty
@@ -108,27 +109,30 @@ diff --git a/content/svg/content/src/nsS
 +
 +  nsCSSProperty propertyID = nsCSSProps::LookupProperty(mappedAttrNameStr);
 +  PRBool changed; // outparam for ParseProperty. (ignored)
 +  mParser->ParseProperty(propertyID, aValue,
 +                         mDocURI, mBaseURI, mNodePrincipal, mDecl, &changed);
 +  return PR_TRUE;
 +}
 +
-+void
-+MappedAttrParser::CreateStyleRule(nsICSSStyleRule** aInstancePtrResult)
++already_AddRefed<nsICSSStyleRule>
++MappedAttrParser::CreateStyleRule()
 +{
-+  if (mDecl) {
-+    nsresult rv = NS_NewCSSStyleRule(aInstancePtrResult, nsnull, mDecl);
-+    if (NS_FAILED(rv)) {
-+      NS_WARNING("could not create style rule from mapped attributes");
-+      mDecl->RuleAbort();  // deletes declaration
-+    }
-+    mDecl = nsnull;
++  if (!mDecl) {
++    return nsnull; // No mapped attributes were parsed
 +  }
++
++  nsRefPtr<nsICSSStyleRule> rule;
++  if (NS_FAILED(NS_NewCSSStyleRule(getter_AddRefs(rule), nsnull, mDecl))) {
++    NS_WARNING("could not create style rule from mapped attributes");
++    mDecl->RuleAbort(); // deletes declaration
++  }
++  mDecl = nsnull; // We no longer own the declaration -- drop our pointer to it
++  return rule.forget();
 +}
 +
  void
  nsSVGElement::UpdateContentStyleRule()
  {
    NS_ASSERTION(!mContentStyleRule, "we already have a content style rule");
 -  
    nsIDocument* doc = GetOwnerDoc();
@@ -208,17 +212,17 @@ diff --git a/content/svg/content/src/nsS
 -      NS_WARNING("could not create contentstylerule");
 -      declaration->RuleAbort();  // deletes declaration
 -    }
 -
 -    // Recycle the parser
 -    parser->SetSVGMode(PR_FALSE);
 -    cssLoader->RecycleParser(parser);
 -  }
-+  mappedAttrParser.CreateStyleRule(getter_AddRefs(mContentStyleRule));
++  mContentStyleRule = mappedAttrParser.CreateStyleRule();
  }
  
  nsISVGValue*
  nsSVGElement::GetMappedAttribute(PRInt32 aNamespaceID, nsIAtom* aName)
  {
    const nsAttrValue* attrVal = mMappedAttributes.GetAttr(aName, aNamespaceID);
    if (!attrVal)
      return nsnull;
--- a/smil_mappedAttrs
+++ b/smil_mappedAttrs
@@ -376,23 +376,23 @@ diff --git a/content/svg/content/src/nsS
    return NS_OK;
  }
  
  // PresentationAttributes-FillStroke
  /* static */ const nsGenericElement::MappedAttributeEntry
  nsSVGElement::sFillStrokeMap[] = {
    { &nsGkAtoms::fill },
    { &nsGkAtoms::fill_opacity },
-@@ -1139,16 +1166,97 @@ nsSVGElement::UpdateContentStyleRule()
+@@ -1142,16 +1169,97 @@ nsSVGElement::UpdateContentStyleRule()
      if (!mappedAttrParser.ParseMappedAttrValue(attrName->Atom(), value)) {
        // Error initializing declaration
        return;
      }
    }
-   mappedAttrParser.CreateStyleRule(getter_AddRefs(mContentStyleRule));
+   mContentStyleRule = mappedAttrParser.CreateStyleRule();
  }
  
 +#ifdef MOZ_SMIL
 +static void
 +ParseMappedAttrAnimValueCallback(void*    aObject,
 +                                 nsIAtom* aPropertyName,
 +                                 void*    aPropertyValue,
 +                                 void*    aData)
@@ -443,18 +443,18 @@ diff --git a/content/svg/content/src/nsS
 +  }
 +
 +  MappedAttrParser mappedAttrParser(doc, doc->GetDocumentURI(),
 +                                    GetBaseURI(), NodePrincipal());
 +  doc->PropertyTable()->Enumerate(this, SMIL_MAPPED_ATTR_ANIMVAL,
 +                                  ParseMappedAttrAnimValueCallback,
 +                                  &mappedAttrParser);
 + 
-+  nsCOMPtr<nsICSSStyleRule> animContentStyleRule;
-+  mappedAttrParser.CreateStyleRule(getter_AddRefs(animContentStyleRule));
++  nsRefPtr<nsICSSStyleRule>
++    animContentStyleRule(mappedAttrParser.CreateStyleRule());
 +
 +  if (animContentStyleRule) {
 +    nsresult rv = SetProperty(SMIL_MAPPED_ATTR_ANIMVAL,
 +                              SMIL_MAPPED_ATTR_STYLERULE_ATOM,
 +                              animContentStyleRule.get(), ReleaseStyleRule);
 +    animContentStyleRule.forget();
 +    NS_ABORT_IF_FALSE(rv == NS_OK,
 +                      "SetProperty failed (or overwrote something)");
@@ -474,17 +474,17 @@ diff --git a/content/svg/content/src/nsS
  nsISVGValue*
  nsSVGElement::GetMappedAttribute(PRInt32 aNamespaceID, nsIAtom* aName)
  {
    const nsAttrValue* attrVal = mMappedAttributes.GetAttr(aName, aNamespaceID);
    if (!attrVal)
      return nsnull;
  
    return attrVal->GetSVGValue();
-@@ -1806,16 +1914,29 @@ nsSVGElement::GetAnimatedAttr(nsIAtom* a
+@@ -1809,16 +1917,29 @@ nsSVGElement::GetAnimatedAttr(nsIAtom* a
      BooleanAttributesInfo info = GetBooleanInfo();
      for (PRUint32 i = 0; i < info.mBooleanCount; i++) {
        if (aName == *info.mBooleanInfo[i].mName) {
          return info.mBooleans[i].ToSMILAttr(this);
        }
      }
    }