bug 452278: CSSParserImpl::InitScanner() never fails; remove checks. r+sr=dbaron
authorZack Weinberg <zweinberg@mozilla.com>
Tue, 09 Sep 2008 21:38:26 -0700
changeset 19102 72b22ac9abe46a61a454e1a20ea201ebc7f6ac83
parent 19101 992805309acbe3e74782b52efb6fb7a18fb67c2f
child 19103 867b7ab68589470fe0e4b592169ec41cd9480281
push id1936
push userdbaron@mozilla.com
push dateWed, 10 Sep 2008 14:22:00 +0000
treeherderautoland@34982905a292 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs452278
milestone1.9.1b1pre
bug 452278: CSSParserImpl::InitScanner() never fails; remove checks. r+sr=dbaron
layout/style/nsCSSParser.cpp
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -237,24 +237,24 @@ protected:
       ~nsAutoParseCompoundProperty()
       {
         mParser->SetParsingCompoundProperty(PR_FALSE);
       }
     private:
       CSSParserImpl* mParser;
   };
 
-  nsresult InitScanner(nsIUnicharInputStream* aInput, nsIURI* aSheetURI,
-                       PRUint32 aLineNumber, nsIURI* aBaseURI,
-                       nsIPrincipal* aSheetPrincipal);
+  void InitScanner(nsIUnicharInputStream* aInput, nsIURI* aSheetURI,
+                   PRUint32 aLineNumber, nsIURI* aBaseURI,
+                   nsIPrincipal* aSheetPrincipal);
   // the caller must hold on to aBuffer until parsing is done
-  nsresult InitScanner(const nsSubstring& aString, nsIURI* aSheetURI,
-                       PRUint32 aLineNumber, nsIURI* aBaseURI,
-                       nsIPrincipal* aSheetPrincipal);
-  nsresult ReleaseScanner(void);
+  void InitScanner(const nsSubstring& aString, nsIURI* aSheetURI,
+                   PRUint32 aLineNumber, nsIURI* aBaseURI,
+                   nsIPrincipal* aSheetPrincipal);
+  void ReleaseScanner(void);
 #ifdef MOZ_SVG
   PRBool IsSVGMode() const {
     return mScanner.IsSVGMode();
   }
 #endif
 
   PRBool GetToken(PRBool aSkipWS);
   PRBool GetURLToken();
@@ -708,37 +708,35 @@ CSSParserImpl::SetSVGMode(PRBool aSVGMod
 
 NS_IMETHODIMP
 CSSParserImpl::SetChildLoader(nsICSSLoader* aChildLoader)
 {
   mChildLoader = aChildLoader;  // not ref counted, it owns us
   return NS_OK;
 }
 
-nsresult
+void
 CSSParserImpl::InitScanner(nsIUnicharInputStream* aInput, nsIURI* aSheetURI,
                            PRUint32 aLineNumber, nsIURI* aBaseURI,
                            nsIPrincipal* aSheetPrincipal)
 {
   NS_ASSERTION(! mScannerInited, "already have scanner");
 
   mScanner.Init(aInput, nsnull, 0, aSheetURI, aLineNumber);
 #ifdef DEBUG
   mScannerInited = PR_TRUE;
 #endif
   mBaseURL = aBaseURI;
   mSheetURL = aSheetURI;
   mSheetPrincipal = aSheetPrincipal;
 
   mHavePushBack = PR_FALSE;
-
-  return NS_OK;
-}
-
-nsresult
+}
+
+void
 CSSParserImpl::InitScanner(const nsSubstring& aString, nsIURI* aSheetURI,
                            PRUint32 aLineNumber, nsIURI* aBaseURI,
                            nsIPrincipal* aSheetPrincipal)
 {
   // Having it not own the string is OK since the caller will hold on to
   // the stream until we're done parsing.
   NS_ASSERTION(! mScannerInited, "already have scanner");
 
@@ -747,31 +745,28 @@ CSSParserImpl::InitScanner(const nsSubst
 #ifdef DEBUG
   mScannerInited = PR_TRUE;
 #endif
   mBaseURL = aBaseURI;
   mSheetURL = aSheetURI;
   mSheetPrincipal = aSheetPrincipal;
 
   mHavePushBack = PR_FALSE;
-
-  return NS_OK;
-}
-
-nsresult
+}
+
+void
 CSSParserImpl::ReleaseScanner(void)
 {
   mScanner.Close();
 #ifdef DEBUG
   mScannerInited = PR_FALSE;
 #endif
   mBaseURL = nsnull;
   mSheetURL = nsnull;
   mSheetPrincipal = nsnull;
-  return NS_OK;
 }
 
 
 NS_IMETHODIMP
 CSSParserImpl::Parse(nsIUnicharInputStream* aInput,
                      nsIURI*                aSheetURI,
                      nsIURI*                aBaseURI,
                      nsIPrincipal*          aSheetPrincipal,
@@ -794,21 +789,17 @@ CSSParserImpl::Parse(nsIUnicharInputStre
   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 result = InitScanner(aInput, aSheetURI, aLineNumber, aBaseURI,
-                                aSheetPrincipal);
-  if (! NS_SUCCEEDED(result)) {
-    return result;
-  }
+  InitScanner(aInput, aSheetURI, aLineNumber, aBaseURI, aSheetPrincipal);
 
   PRInt32 ruleCount = 0;
   mSheet->StyleRuleCount(ruleCount);
   if (0 < ruleCount) {
     nsICSSRule* lastRule = nsnull;
     mSheet->GetStyleRuleAt(ruleCount - 1, lastRule);
     if (lastRule) {
       PRInt32 type;
@@ -882,20 +873,17 @@ CSSParserImpl::ParseStyleAttribute(const
                                    nsICSSStyleRule**        aResult)
 {
   NS_PRECONDITION(aNodePrincipal, "Must have principal here!");
   AssertInitialState();
 
   NS_ASSERTION(nsnull != aBaseURL, "need base URL");
 
   // XXX line number?
-  nsresult rv = InitScanner(aAttributeValue, aDocURL, 0, aBaseURL, aNodePrincipal);
-  if (! NS_SUCCEEDED(rv)) {
-    return rv;
-  }
+  InitScanner(aAttributeValue, aDocURL, 0, aBaseURL, aNodePrincipal);
 
   mSection = eCSSSection_General;
 
   // In quirks mode, allow style declarations to have braces or not
   // (bug 99554).
   PRBool haveBraces;
   if (mNavQuirkMode && GetToken(PR_TRUE)) {
     haveBraces = eCSSToken_Symbol == mToken.mType &&
@@ -905,17 +893,17 @@ CSSParserImpl::ParseStyleAttribute(const
   else {
     haveBraces = PR_FALSE;
   }
 
   nsCSSDeclaration* declaration = ParseDeclarationBlock(haveBraces);
   if (declaration) {
     // Create a style rule for the declaration
     nsICSSStyleRule* rule = nsnull;
-    rv = NS_NewCSSStyleRule(&rule, nsnull, declaration);
+    nsresult rv = NS_NewCSSStyleRule(&rule, nsnull, declaration);
     if (NS_FAILED(rv)) {
       declaration->RuleAbort();
       ReleaseScanner();
       return rv;
     }
     *aResult = rule;
   }
   else {
@@ -936,35 +924,32 @@ CSSParserImpl::ParseAndAppendDeclaration
                                          nsCSSDeclaration* aDeclaration,
                                          PRBool            aParseOnlyOneDecl,
                                          PRBool*           aChanged,
                                          PRBool            aClearOldDecl)
 {
   NS_PRECONDITION(aSheetPrincipal, "Must have principal here!");
   AssertInitialState();
 
-//  NS_ASSERTION(nsnull != aBaseURL, "need base URL");
   *aChanged = PR_FALSE;
 
-  nsresult rv = InitScanner(aBuffer, aSheetURL, 0, aBaseURL, aSheetPrincipal);
-  if (! NS_SUCCEEDED(rv)) {
-    return rv;
-  }
+  InitScanner(aBuffer, aSheetURL, 0, aBaseURL, aSheetPrincipal);
 
   mSection = eCSSSection_General;
 
   if (aClearOldDecl) {
     mData.AssertInitialState();
     aDeclaration->ClearData();
     // We could check if it was already empty, but...
     *aChanged = PR_TRUE;
   } else {
     aDeclaration->ExpandTo(&mData);
   }
 
+  nsresult rv = NS_OK;
   do {
     // If we cleared the old decl, then we want to be calling
     // ValueAppended as we parse.
     if (!ParseDeclaration(aDeclaration, PR_FALSE, aClearOldDecl, aChanged)) {
       rv = mScanner.GetLowLevelError();
       if (NS_FAILED(rv))
         break;
 
@@ -987,20 +972,17 @@ CSSParserImpl::ParseRule(const nsAString
                          nsIPrincipal*           aSheetPrincipal,
                          nsCOMArray<nsICSSRule>& aResult)
 {
   NS_PRECONDITION(aSheetPrincipal, "Must have principal here!");
   AssertInitialState();
 
   NS_ASSERTION(nsnull != aBaseURL, "need base URL");
 
-  nsresult rv = InitScanner(aRule, aSheetURL, 0, aBaseURL, aSheetPrincipal);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
+  InitScanner(aRule, aSheetURL, 0, aBaseURL, aSheetPrincipal);
 
   mSection = eCSSSection_Charset; // callers are responsible for rejecting invalid rules.
 
   nsCSSToken* tk = &mToken;
   // Get first non-whitespace token
   if (!GetToken(PR_TRUE)) {
     REPORT_UNEXPECTED(PEParseRuleWSOnly);
     OUTPUT_ERROR();
@@ -1028,20 +1010,17 @@ CSSParserImpl::ParseProperty(const nsCSS
 {
   NS_PRECONDITION(aSheetPrincipal, "Must have principal here!");
   AssertInitialState();
 
   NS_ASSERTION(nsnull != aBaseURL, "need base URL");
   NS_ASSERTION(nsnull != aDeclaration, "Need declaration to parse into!");
   *aChanged = PR_FALSE;
 
-  nsresult rv = InitScanner(aPropValue, aSheetURL, 0, aBaseURL, aSheetPrincipal);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
+  InitScanner(aPropValue, aSheetURL, 0, aBaseURL, aSheetPrincipal);
 
   mSection = eCSSSection_General;
 
   if (eCSSProperty_UNKNOWN == aPropID) { // unknown property
     NS_ConvertASCIItoUTF16 propName(nsCSSProps::GetStringValue(aPropID));
     const PRUnichar *params[] = {
       propName.get()
     };
@@ -1089,20 +1068,17 @@ CSSParserImpl::ParseMediaList(const nsSu
                               nsMediaList* aMediaList,
                               PRBool aHTMLMode)
 {
   // XXX Are there cases where the caller wants to keep what it already
   // has in case of parser error?
   aMediaList->Clear();
 
   // fake base URL since media lists don't have URLs in them
-  nsresult rv = InitScanner(aBuffer, aURL, aLineNumber, aURL, nsnull);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
+  InitScanner(aBuffer, aURL, aLineNumber, aURL, nsnull);
 
   AssertInitialState();
   NS_ASSERTION(aHTMLMode == PR_TRUE || aHTMLMode == PR_FALSE,
                "invalid PRBool");
   mHTMLMediaMode = aHTMLMode;
 
     // XXXldb We need to make the scanner not skip CSS comments!  (Or
     // should we?)
@@ -1118,38 +1094,36 @@ CSSParserImpl::ParseMediaList(const nsSu
 
   if (!GatherMedia(aMediaList, PRUnichar(0))) {
     aMediaList->Clear();
     aMediaList->SetNonEmpty(); // don't match anything
     if (!mHTMLMediaMode) {
       OUTPUT_ERROR();
     }
   }
-  rv = mScanner.GetLowLevelError();
+  nsresult rv = mScanner.GetLowLevelError();
   CLEAR_ERROR();
   ReleaseScanner();
   mHTMLMediaMode = PR_FALSE;
 
   return rv;
 }
 
 NS_IMETHODIMP
 CSSParserImpl::ParseColorString(const nsSubstring& aBuffer,
                                 nsIURI* aURL, // for error reporting
                                 PRUint32 aLineNumber, // for error reporting
                                 nscolor* aColor)
 {
   AssertInitialState();
-  nsresult rv = InitScanner(aBuffer, aURL, aLineNumber, aURL, nsnull);
-  if (NS_FAILED(rv))
-    return rv;
+  InitScanner(aBuffer, aURL, aLineNumber, aURL, nsnull);
 
   nsCSSValue value;
   PRBool colorParsed = ParseColor(value);
-  rv = mScanner.GetLowLevelError();
+  nsresult rv = mScanner.GetLowLevelError();
   OUTPUT_ERROR();
   ReleaseScanner();
 
   if (!colorParsed) {
     return NS_FAILED(rv) ? rv : NS_ERROR_FAILURE;
   }
 
   if (value.GetUnit() == eCSSUnit_String) {
@@ -1183,26 +1157,24 @@ CSSParserImpl::ParseColorString(const ns
 }
 
 NS_IMETHODIMP
 CSSParserImpl::ParseSelectorString(const nsSubstring& aSelectorString,
                                    nsIURI* aURL, // for error reporting
                                    PRUint32 aLineNumber, // for error reporting
                                    nsCSSSelectorList **aSelectorList)
 {
-  nsresult rv = InitScanner(aSelectorString, aURL, aLineNumber, aURL, nsnull);
-  if (NS_FAILED(rv))
-    return rv;
+  InitScanner(aSelectorString, aURL, aLineNumber, aURL, nsnull);
 
   AssertInitialState();
 
   mUnresolvablePrefixException = PR_TRUE;
 
   PRBool success = ParseSelectorList(*aSelectorList, PR_FALSE);
-  rv = mScanner.GetLowLevelError();
+  nsresult rv = mScanner.GetLowLevelError();
   OUTPUT_ERROR();
   ReleaseScanner();
 
   mUnresolvablePrefixException = PR_FALSE;
 
   if (success) {
     NS_ASSERTION(*aSelectorList, "Should have list!");
     return NS_OK;