From 5e148f5001bb2c1e7c67e8fe78ad87de3cb5a0bd Mon Sep 17 00:00:00 2001 From: "bzbarsky%mit.edu" Date: Fri, 15 Feb 2008 04:21:57 +0000 Subject: [PATCH] Change the nsICSSParser::Parse() API so that we don't hand out a stylesheet; require a stylesheet to be set before calling Parse(). Bug 404315, r+sr=dbaron, a=beltzner --- layout/style/nsCSSLoader.cpp | 9 +++--- layout/style/nsCSSParser.cpp | 52 ++++++++++++++--------------------- layout/style/nsCSSScanner.cpp | 12 +++++++- layout/style/nsICSSParser.h | 24 ++++++++++++---- 4 files changed, 53 insertions(+), 44 deletions(-) diff --git a/layout/style/nsCSSLoader.cpp b/layout/style/nsCSSLoader.cpp index b304ca11c40..dc9467cfdb9 100644 --- a/layout/style/nsCSSLoader.cpp +++ b/layout/style/nsCSSLoader.cpp @@ -438,6 +438,9 @@ CSSLoaderImpl::RecycleParser(nsICSSParser* aParser) if (!gParsers->AppendObject(aParser)) { return NS_ERROR_FAILURE; } + + // Make sure the parser doesn't keep the last sheet it parsed alive + aParser->SetStyleSheet(nsnull); return NS_OK; } @@ -1497,9 +1500,6 @@ CSSLoaderImpl::ParseSheet(nsIUnicharInputStream* aStream, return rv; } - // The parser insists on passing out a strong ref to the sheet it - // parsed. We don't care. - nsCOMPtr dummySheet; // Push our load data on the stack so any kids can pick it up mParsingDatas.AppendElement(aLoadData); nsCOMPtr sheetURI, baseURI; @@ -1507,8 +1507,7 @@ CSSLoaderImpl::ParseSheet(nsIUnicharInputStream* aStream, aLoadData->mSheet->GetBaseURI(getter_AddRefs(baseURI)); rv = parser->Parse(aStream, sheetURI, baseURI, aLoadData->mSheet->Principal(), aLoadData->mLineNumber, - aLoadData->mAllowUnsafeRules, - *getter_AddRefs(dummySheet)); + aLoadData->mAllowUnsafeRules); mParsingDatas.RemoveElementAt(mParsingDatas.Count() - 1); RecycleParser(parser); diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index c4c802d4f37..4c3452ba6db 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -110,8 +110,7 @@ public: nsIURI* aBaseURI, nsIPrincipal* aSheetPrincipal, PRUint32 aLineNumber, - PRBool aAllowUnsafeRules, - nsICSSStyleSheet*& aResult); + PRBool aAllowUnsafeRules); NS_IMETHOD ParseStyleAttribute(const nsAString& aAttributeValue, nsIURI* aDocURL, @@ -584,16 +583,15 @@ CSSParserImpl::~CSSParserImpl() NS_IMETHODIMP CSSParserImpl::SetStyleSheet(nsICSSStyleSheet* aSheet) { - NS_PRECONDITION(nsnull != aSheet, "null ptr"); - if (nsnull == aSheet) { - return NS_ERROR_NULL_POINTER; - } - if (aSheet != mSheet) { - // Switch to using the new sheet + // Switch to using the new sheet, if any mGroupStack.Clear(); mSheet = aSheet; - mNameSpaceMap = mSheet->GetNameSpaceMap(); + if (mSheet) { + mNameSpaceMap = mSheet->GetNameSpaceMap(); + } else { + mNameSpaceMap = nsnull; + } } return NS_OK; @@ -684,6 +682,7 @@ CSSParserImpl::ReleaseScanner(void) #endif mBaseURL = nsnull; mSheetURL = nsnull; + mSheetPrincipal = nsnull; return NS_OK; } @@ -694,34 +693,26 @@ CSSParserImpl::Parse(nsIUnicharInputStream* aInput, nsIURI* aBaseURI, nsIPrincipal* aSheetPrincipal, PRUint32 aLineNumber, - PRBool aAllowUnsafeRules, - nsICSSStyleSheet*& aResult) + PRBool aAllowUnsafeRules) { NS_PRECONDITION(aSheetPrincipal, "Must have principal here!"); NS_ASSERTION(nsnull != aBaseURI, "need base URL"); NS_ASSERTION(nsnull != aSheetURI, "need sheet URL"); - if (! mSheet) { - NS_NewCSSStyleSheet(getter_AddRefs(mSheet)); - NS_ENSURE_TRUE(mSheet, NS_ERROR_OUT_OF_MEMORY); + NS_PRECONDITION(mSheet, "Must have sheet to parse into"); + NS_ENSURE_STATE(mSheet); - mSheet->SetURIs(aSheetURI, aSheetURI, aBaseURI); - mSheet->SetPrincipal(aSheetPrincipal); - mNameSpaceMap = nsnull; - } #ifdef DEBUG - else { - nsCOMPtr uri; - mSheet->GetSheetURI(getter_AddRefs(uri)); - PRBool equal; - NS_ASSERTION(NS_SUCCEEDED(aSheetURI->Equals(uri, &equal)) && equal, - "Sheet URI does not match passed URI"); - NS_ASSERTION(NS_SUCCEEDED(mSheet->Principal()->Equals(aSheetPrincipal, - &equal)) && - equal, - "Sheet principal does not match passed principal"); - } + nsCOMPtr uri; + mSheet->GetSheetURI(getter_AddRefs(uri)); + PRBool equal; + NS_ASSERTION(NS_SUCCEEDED(aSheetURI->Equals(uri, &equal)) && equal, + "Sheet URI does not match passed URI"); + NS_ASSERTION(NS_SUCCEEDED(mSheet->Principal()->Equals(aSheetPrincipal, + &equal)) && + equal, + "Sheet principal does not match passed principal"); #endif nsresult errorCode = NS_OK; @@ -784,9 +775,6 @@ CSSParserImpl::Parse(nsIUnicharInputStream* aInput, mUnsafeRulesEnabled = PR_FALSE; - aResult = mSheet; - NS_ADDREF(aResult); - return NS_OK; } diff --git a/layout/style/nsCSSScanner.cpp b/layout/style/nsCSSScanner.cpp index 8cd5674bf69..a9dda463fe3 100644 --- a/layout/style/nsCSSScanner.cpp +++ b/layout/style/nsCSSScanner.cpp @@ -190,7 +190,7 @@ nsCSSScanner::nsCSSScanner() BuildLexTable(); } mPushback = mLocalPushback; - mPushbackSize = 4; + mPushbackSize = NS_ARRAY_LENGTH(mLocalPushback); // No need to init the other members, since they represent state // which can get cleared. We'll init them every time Init() is // called. @@ -456,6 +456,16 @@ void nsCSSScanner::Close() { mInputStream = nsnull; mReadPointer = nsnull; + + // Clean things up so we don't hold on to memory if our parser gets recycled. + mFileName.Truncate(); + mURI = nsnull; + mError.Truncate(); + if (mPushback != mLocalPushback) { + delete [] mPushback; + mPushback = mLocalPushback; + mPushbackSize = NS_ARRAY_LENGTH(mLocalPushback); + } } #ifdef CSS_REPORT_PARSE_ERRORS diff --git a/layout/style/nsICSSParser.h b/layout/style/nsICSSParser.h index dd872f716c1..73c8aae4004 100644 --- a/layout/style/nsICSSParser.h +++ b/layout/style/nsICSSParser.h @@ -57,8 +57,8 @@ class nsMediaList; class nsIPrincipal; #define NS_ICSS_PARSER_IID \ -{ 0x2cb34728, 0x0f17, 0x4753, \ - {0x8e, 0xad, 0xec, 0x73, 0xe5, 0x69, 0xcd, 0xcd} } +{ 0xad4a3778, 0xdae0, 0x4640, \ + { 0xb2, 0x5a, 0x24, 0xff, 0x09, 0xc3, 0x70, 0xef } } // Rule processing function typedef void (*PR_CALLBACK RuleAppendFunc) (nsICSSRule* aRule, void* aData); @@ -69,7 +69,8 @@ public: NS_DECLARE_STATIC_IID_ACCESSOR(NS_ICSS_PARSER_IID) // Set a style sheet for the parser to fill in. The style sheet must - // implement the nsICSSStyleSheet interface + // implement the nsICSSStyleSheet interface. Null can be passed in to clear + // out an existing stylesheet reference. NS_IMETHOD SetStyleSheet(nsICSSStyleSheet* aSheet) = 0; // Set whether or not tags & classes are case sensitive or uppercased @@ -87,16 +88,27 @@ public: NS_IMETHOD SetChildLoader(nsICSSLoader* aChildLoader) = 0; /** + * Parse aInput into the stylesheet that was previously set by calling + * SetStyleSheet. Calling this method without calling SetStyleSheet first is + * an error. + * + * @param aInput the data to parse + * @param aSheetURL the URI to use as the sheet URI (for error reporting). + * This must match the URI of the sheet passed to + * SetStyleSheet. + * @param aBaseURI the URI to use for relative URI resolution + * @param aSheetPrincipal the principal of the stylesheet. This must match + * the principal of the sheet passed to SetStyleSheet. + * @param aLineNumber the line number of the first line of the sheet. * @param aAllowUnsafeRules see aEnableUnsafeRules in - * nsICSSLoader::LoadSheetSync + * nsICSSLoader::LoadSheetSync */ NS_IMETHOD Parse(nsIUnicharInputStream* aInput, nsIURI* aSheetURL, nsIURI* aBaseURI, nsIPrincipal* aSheetPrincipal, PRUint32 aLineNumber, - PRBool aAllowUnsafeRules, - nsICSSStyleSheet*& aResult) = 0; + PRBool aAllowUnsafeRules) = 0; // Parse HTML style attribute or its equivalent in other markup // languages. aBaseURL is the base url to use for relative links in