From 56379f8173ea3e4176aedab68e1d5c871a6067fb Mon Sep 17 00:00:00 2001 From: "bzbarsky%mit.edu" Date: Mon, 17 Mar 2003 03:46:17 +0000 Subject: [PATCH] Avoid possible double-delete of CSS declaration. Bug 196271, r+sr=dbaron git-svn-id: svn://10.0.0.236/trunk@139590 18797224-902f-48f8-a5cc-f745e15eee43 --- .../style/src/nsDOMCSSAttrDeclaration.cpp | 20 ++++++++++++------- .../html/style/src/nsDOMCSSAttrDeclaration.h | 5 ++++- .../layout/style/nsDOMCSSAttrDeclaration.cpp | 20 ++++++++++++------- .../layout/style/nsDOMCSSAttrDeclaration.h | 5 ++++- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.cpp b/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.cpp index 68f3f7c16a9..5a8c9d87f46 100644 --- a/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.cpp +++ b/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.cpp @@ -82,7 +82,7 @@ nsDOMCSSAttributeDeclaration::RemoveProperty(const nsAString& aPropertyName, rv = decl->RemoveProperty(prop, val); if (NS_SUCCEEDED(rv)) { - rv = SetCSSDeclaration(decl, PR_TRUE); + rv = SetCSSDeclaration(decl, PR_TRUE, PR_TRUE); } else { // RemoveProperty will throw in all sorts of situations -- eg if // the property is a shorthand one. Do not propagate its return @@ -102,13 +102,20 @@ nsDOMCSSAttributeDeclaration::DropReference() nsresult nsDOMCSSAttributeDeclaration::SetCSSDeclaration(nsCSSDeclaration* aDecl, - PRBool aNotify) + PRBool aNotify, + PRBool aDeclOwnedByRule) { NS_ASSERTION(mContent, "Must have content node to set the decl!"); + NS_PRECONDITION(aDecl, "Null decl!"); nsCOMPtr cssRule; nsresult rv = NS_NewCSSStyleRule(getter_AddRefs(cssRule), nsCSSSelector()); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_FAILED(rv)) { + if (!aDeclOwnedByRule) { + aDecl->RuleAbort(); + } + return rv; + } cssRule->SetDeclaration(aDecl); cssRule->SetWeight(PR_INT32_MAX); @@ -138,9 +145,8 @@ nsDOMCSSAttributeDeclaration::GetCSSDeclaration(nsCSSDeclaration **aDecl, else if (aAllocate) { result = NS_NewCSSDeclaration(aDecl); if (NS_SUCCEEDED(result)) { - result = SetCSSDeclaration(*aDecl, PR_FALSE); + result = SetCSSDeclaration(*aDecl, PR_FALSE, PR_FALSE); if (NS_FAILED(result)) { - (*aDecl)->RuleAbort(); *aDecl = nsnull; } } @@ -229,7 +235,7 @@ nsDOMCSSAttributeDeclaration::ParsePropertyValue(const nsAString& aPropName, result = cssParser->ParseProperty(aPropName, aPropValue, baseURI, decl, &uselessHint); if (NS_SUCCEEDED(result)) { - result = SetCSSDeclaration(decl, PR_TRUE); + result = SetCSSDeclaration(decl, PR_TRUE, PR_TRUE); } if (cssLoader) { @@ -287,7 +293,7 @@ nsDOMCSSAttributeDeclaration::ParseDeclaration(const nsAString& aDecl, &uselessHint); if (NS_SUCCEEDED(result)) { - result = SetCSSDeclaration(decl, PR_TRUE); + result = SetCSSDeclaration(decl, PR_TRUE, PR_TRUE); } if (cssLoader) { diff --git a/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.h b/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.h index d080f2c0be8..1d2abb93c0e 100644 --- a/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.h +++ b/mozilla/content/html/style/src/nsDOMCSSAttrDeclaration.h @@ -59,6 +59,8 @@ public: nsAString& aReturn); virtual void DropReference(); + // If GetCSSDeclaration returns non-null, then the decl it returns + // is owned by our current style rule. virtual nsresult GetCSSDeclaration(nsCSSDeclaration **aDecl, PRBool aAllocate); virtual nsresult GetCSSParsingEnvironment(nsIContent* aContent, @@ -73,7 +75,8 @@ public: virtual nsresult GetParent(nsISupports **aParent); protected: - nsresult SetCSSDeclaration(nsCSSDeclaration* aDecl, PRBool aNotify); + nsresult SetCSSDeclaration(nsCSSDeclaration* aDecl, PRBool aNotify, + PRBool aDeclOwnedByRule); nsIHTMLContent *mContent; }; diff --git a/mozilla/layout/style/nsDOMCSSAttrDeclaration.cpp b/mozilla/layout/style/nsDOMCSSAttrDeclaration.cpp index 68f3f7c16a9..5a8c9d87f46 100644 --- a/mozilla/layout/style/nsDOMCSSAttrDeclaration.cpp +++ b/mozilla/layout/style/nsDOMCSSAttrDeclaration.cpp @@ -82,7 +82,7 @@ nsDOMCSSAttributeDeclaration::RemoveProperty(const nsAString& aPropertyName, rv = decl->RemoveProperty(prop, val); if (NS_SUCCEEDED(rv)) { - rv = SetCSSDeclaration(decl, PR_TRUE); + rv = SetCSSDeclaration(decl, PR_TRUE, PR_TRUE); } else { // RemoveProperty will throw in all sorts of situations -- eg if // the property is a shorthand one. Do not propagate its return @@ -102,13 +102,20 @@ nsDOMCSSAttributeDeclaration::DropReference() nsresult nsDOMCSSAttributeDeclaration::SetCSSDeclaration(nsCSSDeclaration* aDecl, - PRBool aNotify) + PRBool aNotify, + PRBool aDeclOwnedByRule) { NS_ASSERTION(mContent, "Must have content node to set the decl!"); + NS_PRECONDITION(aDecl, "Null decl!"); nsCOMPtr cssRule; nsresult rv = NS_NewCSSStyleRule(getter_AddRefs(cssRule), nsCSSSelector()); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_FAILED(rv)) { + if (!aDeclOwnedByRule) { + aDecl->RuleAbort(); + } + return rv; + } cssRule->SetDeclaration(aDecl); cssRule->SetWeight(PR_INT32_MAX); @@ -138,9 +145,8 @@ nsDOMCSSAttributeDeclaration::GetCSSDeclaration(nsCSSDeclaration **aDecl, else if (aAllocate) { result = NS_NewCSSDeclaration(aDecl); if (NS_SUCCEEDED(result)) { - result = SetCSSDeclaration(*aDecl, PR_FALSE); + result = SetCSSDeclaration(*aDecl, PR_FALSE, PR_FALSE); if (NS_FAILED(result)) { - (*aDecl)->RuleAbort(); *aDecl = nsnull; } } @@ -229,7 +235,7 @@ nsDOMCSSAttributeDeclaration::ParsePropertyValue(const nsAString& aPropName, result = cssParser->ParseProperty(aPropName, aPropValue, baseURI, decl, &uselessHint); if (NS_SUCCEEDED(result)) { - result = SetCSSDeclaration(decl, PR_TRUE); + result = SetCSSDeclaration(decl, PR_TRUE, PR_TRUE); } if (cssLoader) { @@ -287,7 +293,7 @@ nsDOMCSSAttributeDeclaration::ParseDeclaration(const nsAString& aDecl, &uselessHint); if (NS_SUCCEEDED(result)) { - result = SetCSSDeclaration(decl, PR_TRUE); + result = SetCSSDeclaration(decl, PR_TRUE, PR_TRUE); } if (cssLoader) { diff --git a/mozilla/layout/style/nsDOMCSSAttrDeclaration.h b/mozilla/layout/style/nsDOMCSSAttrDeclaration.h index d080f2c0be8..1d2abb93c0e 100644 --- a/mozilla/layout/style/nsDOMCSSAttrDeclaration.h +++ b/mozilla/layout/style/nsDOMCSSAttrDeclaration.h @@ -59,6 +59,8 @@ public: nsAString& aReturn); virtual void DropReference(); + // If GetCSSDeclaration returns non-null, then the decl it returns + // is owned by our current style rule. virtual nsresult GetCSSDeclaration(nsCSSDeclaration **aDecl, PRBool aAllocate); virtual nsresult GetCSSParsingEnvironment(nsIContent* aContent, @@ -73,7 +75,8 @@ public: virtual nsresult GetParent(nsISupports **aParent); protected: - nsresult SetCSSDeclaration(nsCSSDeclaration* aDecl, PRBool aNotify); + nsresult SetCSSDeclaration(nsCSSDeclaration* aDecl, PRBool aNotify, + PRBool aDeclOwnedByRule); nsIHTMLContent *mContent; };