Bug 455839: simplify the CSS error reporting API. r=dbaron
authorZack Weinberg <zackw@panix.com>
Thu, 15 Nov 2012 11:36:15 -0500
changeset 113390 0f01d41ab835d4c916f2b92f9db75b3d0ede92ef
parent 113389 03b607d3bca6b680ae59d780e064c880752e2e5d
child 113391 5c0ce5518c74e413692247db44f556d4dd00d8aa
push id23870
push userryanvm@gmail.com
push dateFri, 16 Nov 2012 01:21:36 +0000
treeherdermozilla-central@58ebb638a7ea [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs455839
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 455839: simplify the CSS error reporting API. r=dbaron
layout/style/ErrorReporter.cpp
layout/style/ErrorReporter.h
layout/style/nsCSSParser.cpp
layout/style/nsCSSScanner.cpp
--- a/layout/style/ErrorReporter.cpp
+++ b/layout/style/ErrorReporter.cpp
@@ -216,127 +216,121 @@ ErrorReporter::OutputError()
 
 void
 ErrorReporter::ClearError()
 {
   mError.Truncate();
 }
 
 void
-ErrorReporter::AddToError(const nsAString &aErrorText)
+ErrorReporter::AddToError(const nsString &aErrorText)
 {
   if (!ShouldReportErrors()) return;
 
   if (mError.IsEmpty()) {
     mErrorLineNumber = mScanner->GetLineNumber();
     mErrorColNumber = mScanner->GetColumnNumber();
     mError = aErrorText;
   } else {
-    mError.Append(NS_LITERAL_STRING("  ") + aErrorText);
+    mError.AppendLiteral("  ");
+    mError.Append(aErrorText);
   }
 }
 
 void
 ErrorReporter::ReportUnexpected(const char *aMessage)
 {
   if (!ShouldReportErrors()) return;
 
   nsAutoString str;
   sStringBundle->GetStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(),
                                    getter_Copies(str));
   AddToError(str);
 }
 
 void
-ErrorReporter::ReportUnexpectedParams(const char *aMessage,
-                                      const PRUnichar **aParams,
-                                      uint32_t aParamsLength)
+ErrorReporter::ReportUnexpected(const char *aMessage,
+                                const nsString &aParam)
+{
+  if (!ShouldReportErrors()) return;
+
+  const PRUnichar *params[1] = { aParam.get() };
+  nsAutoString str;
+  sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(),
+                                      params, ArrayLength(params),
+                                      getter_Copies(str));
+  AddToError(str);
+}
+
+void
+ErrorReporter::ReportUnexpected(const char *aMessage,
+                                const nsCSSToken &aToken)
 {
   if (!ShouldReportErrors()) return;
 
+  nsAutoString tokenString;
+  aToken.AppendToString(tokenString);
+  const PRUnichar *params[1] = { tokenString.get() };
+
   nsAutoString str;
   sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(),
-                                      aParams, aParamsLength,
+                                      params, ArrayLength(params),
+                                      getter_Copies(str));
+  AddToError(str);
+}
+
+void
+ErrorReporter::ReportUnexpected(const char *aMessage,
+                                const nsCSSToken &aToken,
+                                PRUnichar aChar)
+{
+  if (!ShouldReportErrors()) return;
+
+  nsAutoString tokenString;
+  aToken.AppendToString(tokenString);
+  const PRUnichar charStr[2] = { aChar, 0 };
+  const PRUnichar *params[2] = { tokenString.get(), charStr };
+
+  nsAutoString str;
+  sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(),
+                                      params, ArrayLength(params),
                                       getter_Copies(str));
   AddToError(str);
 }
 
 void
 ErrorReporter::ReportUnexpectedEOF(const char *aMessage)
 {
   if (!ShouldReportErrors()) return;
 
   nsAutoString innerStr;
   sStringBundle->GetStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(),
                                    getter_Copies(innerStr));
-  const PRUnichar *params[] = {
-    innerStr.get()
-  };
+  const PRUnichar *params[1] = { innerStr.get() };
 
   nsAutoString str;
   sStringBundle->FormatStringFromName(NS_LITERAL_STRING("PEUnexpEOF2").get(),
-                                      params, NS_ARRAY_LENGTH(params),
+                                      params, ArrayLength(params),
                                       getter_Copies(str));
   AddToError(str);
 }
 
 void
 ErrorReporter::ReportUnexpectedEOF(PRUnichar aExpected)
 {
   if (!ShouldReportErrors()) return;
 
   const PRUnichar expectedStr[] = {
     PRUnichar('\''), aExpected, PRUnichar('\''), PRUnichar(0)
   };
-  const PRUnichar *params[] = { expectedStr };
+  const PRUnichar *params[1] = { expectedStr };
 
   nsAutoString str;
   sStringBundle->FormatStringFromName(NS_LITERAL_STRING("PEUnexpEOF2").get(),
-                                      params, NS_ARRAY_LENGTH(params),
-                                      getter_Copies(str));
-  AddToError(str);
-}
-
-void
-ErrorReporter::ReportUnexpectedToken(const char *aMessage,
-                                     const nsCSSToken &aToken)
-{
-  if (!ShouldReportErrors()) return;
-
-  nsAutoString tokenString;
-  aToken.AppendToString(tokenString);
-  const PRUnichar *params[] = {
-    tokenString.get()
-  };
-
-  nsAutoString str;
-  sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(),
-                                      params, NS_ARRAY_LENGTH(params),
+                                      params, ArrayLength(params),
                                       getter_Copies(str));
   AddToError(str);
 }
 
-void
-ErrorReporter::ReportUnexpectedTokenParams(const char *aMessage,
-                                           const nsCSSToken &aToken,
-                                           const PRUnichar **aParams,
-                                           uint32_t aParamsLength)
-{
-  NS_ABORT_IF_FALSE(aParams[0] == nullptr, "first param should be empty");
-
-  if (!ShouldReportErrors()) return;
-
-  nsAutoString tokenString;
-  aToken.AppendToString(tokenString);
-  aParams[0] = tokenString.get();
-
-  nsAutoString str;
-  sStringBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(aMessage).get(),
-                                      aParams, aParamsLength,
-                                      getter_Copies(str));
-  AddToError(str);
-
-}
-
 } // namespace css
 } // namespace mozilla
 
 #endif
--- a/layout/style/ErrorReporter.h
+++ b/layout/style/ErrorReporter.h
@@ -33,57 +33,38 @@ public:
                 nsIURI *aURI);
   ~ErrorReporter();
 
   static void ReleaseGlobals();
 
   void OutputError();
   void ClearError();
 
-  // aMessage must take no parameters
-  void ReportUnexpected(const char *aMessage);
-  // aLookingFor is a plain string, not a format string
-  void ReportUnexpectedEOF(const char *aLookingFor);
-  // aLookingFor is a single character
-  void ReportUnexpectedEOF(PRUnichar aLookingFor);
-
-  // aMessage is a format string with 1 parameter (for the string
-  // representation of the unexpected token)
-  void ReportUnexpectedToken(const char *aMessage, const nsCSSToken &aToken);
+  // In all overloads of ReportUnexpected, aMessage is a stringbundle
+  // name, which will be processed as a format string with the
+  // indicated number of parameters.
 
-  // aMessage is a format string
-  template<uint32_t N>
-  void ReportUnexpectedParams(const char* aMessage,
-                              const PRUnichar* (&aParams)[N])
-  {
-    MOZ_STATIC_ASSERT(N > 0, "use ReportUnexpected instead");
-    ReportUnexpectedParams(aMessage, aParams, N);
-  }
+  // no parameters
+  void ReportUnexpected(const char *aMessage);
+  // one parameter, a string
+  void ReportUnexpected(const char *aMessage, const nsString& aParam);
+  // one parameter, a token
+  void ReportUnexpected(const char *aMessage, const nsCSSToken& aToken);
+  // two parameters, a token and a character, in that order
+  void ReportUnexpected(const char *aMessage, const nsCSSToken& aToken,
+                        PRUnichar aChar);
 
-  // aMessage is a format string
-  // aParams's first entry must be null, and we'll fill in the token
-  template<uint32_t N>
-  void ReportUnexpectedTokenParams(const char* aMessage,
-                                   const nsCSSToken &aToken,
-                                   const PRUnichar* (&aParams)[N])
-  {
-    MOZ_STATIC_ASSERT(N > 1, "use ReportUnexpectedToken instead");
-    ReportUnexpectedTokenParams(aMessage, aToken, aParams, N);
-  }
+  // for ReportUnexpectedEOF, aExpected can be either a stringbundle
+  // name or a single character.  In the former case there may not be
+  // any format parameters.
+  void ReportUnexpectedEOF(const char *aExpected);
+  void ReportUnexpectedEOF(PRUnichar aExpected);
 
 private:
-  void ReportUnexpectedParams(const char *aMessage,
-                              const PRUnichar **aParams,
-                              uint32_t aParamsLength);
-  void ReportUnexpectedTokenParams(const char *aMessage,
-                                   const nsCSSToken &aToken,
-                                   const PRUnichar **aParams,
-                                   uint32_t aParamsLength);
-
-  void AddToError(const nsAString &aErrorText);
+  void AddToError(const nsString &aErrorText);
 
 #ifdef CSS_REPORT_PARSE_ERRORS
   nsAutoString mError;
   nsString mFileName;
   const nsCSSScanner *mScanner;
   const nsCSSStyleSheet *mSheet;
   const Loader *mLoader;
   nsIURI *mURI;
@@ -101,27 +82,23 @@ inline ErrorReporter::ErrorReporter(cons
 inline ErrorReporter::~ErrorReporter() {}
 
 inline void ErrorReporter::ReleaseGlobals() {}
 
 inline void ErrorReporter::OutputError() {}
 inline void ErrorReporter::ClearError() {}
 
 inline void ErrorReporter::ReportUnexpected(const char *) {}
-inline void ErrorReporter::ReportUnexpectedParams(const char *,
-                                                  const PRUnichar **,
-                                                  uint32_t) {}
+inline void ErrorReporter::ReportUnexpected(const char *, const nsString &) {}
+inline void ErrorReporter::ReportUnexpected(const char *, const nsCSSToken &) {}
+inline void ErrorReporter::ReportUnexpected(const char *, const nsCSSToken &,
+                                            PRUnichar) {}
+
 inline void ErrorReporter::ReportUnexpectedEOF(const char *) {}
 inline void ErrorReporter::ReportUnexpectedEOF(PRUnichar) {}
-inline void ErrorReporter::ReportUnexpectedToken(const char *,
-                                                 const nsCSSToken &) {}
-inline void ErrorReporter::ReportUnexpectedTokenParams(const char *,
-                                                       const nsCSSToken &,
-                                                       const PRUnichar **,
-                                                       uint32_t) {}
 
-inline void ErrorReporter::AddToError(const nsAString &) {}
+inline void ErrorReporter::AddToError(const nsString &) {}
 #endif
 
 } // namespace css
 } // namespace mozilla
 
 #endif // mozilla_css_ErrorReporter_h_
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -716,31 +716,31 @@ static void AppendRuleToSheet(css::Rule*
 {
   CSSParserImpl* parser = (CSSParserImpl*) aParser;
   parser->AppendRule(aRule);
 }
 
 #define REPORT_UNEXPECTED(msg_) \
   mReporter->ReportUnexpected(#msg_)
 
-#define REPORT_UNEXPECTED_P(msg_, params_) \
-  mReporter->ReportUnexpectedParams(#msg_, params_)
+#define REPORT_UNEXPECTED_P(msg_, param_) \
+  mReporter->ReportUnexpected(#msg_, param_)
+
+#define REPORT_UNEXPECTED_TOKEN(msg_) \
+  mReporter->ReportUnexpected(#msg_, mToken)
+
+#define REPORT_UNEXPECTED_TOKEN_CHAR(msg_, ch_) \
+  mReporter->ReportUnexpected(#msg_, mToken, ch_)
 
 #define REPORT_UNEXPECTED_EOF(lf_) \
   mReporter->ReportUnexpectedEOF(#lf_)
 
 #define REPORT_UNEXPECTED_EOF_CHAR(ch_) \
   mReporter->ReportUnexpectedEOF(ch_)
 
-#define REPORT_UNEXPECTED_TOKEN(msg_) \
-  mReporter->ReportUnexpectedToken(#msg_, mToken)
-
-#define REPORT_UNEXPECTED_TOKEN_P(msg_, params_) \
-  mReporter->ReportUnexpectedTokenParams(#msg_, mToken, params_)
-
 #define OUTPUT_ERROR() \
   mReporter->OutputError()
 
 #define CLEAR_ERROR() \
   mReporter->ClearError()
 
 CSSParserImpl::CSSParserImpl()
   : mToken(),
@@ -1083,39 +1083,33 @@ CSSParserImpl::ParseProperty(const nsCSS
   mSection = eCSSSection_General;
   scanner.SetSVGMode(aIsSVGMode);
 
   *aChanged = false;
 
   // Check for unknown or preffed off properties
   if (eCSSProperty_UNKNOWN == aPropID || !nsCSSProps::IsEnabled(aPropID)) {
     NS_ConvertASCIItoUTF16 propName(nsCSSProps::GetStringValue(aPropID));
-    const PRUnichar *params[] = {
-      propName.get()
-    };
-    REPORT_UNEXPECTED_P(PEUnknownProperty, params);
+    REPORT_UNEXPECTED_P(PEUnknownProperty, propName);
     REPORT_UNEXPECTED(PEDeclDropped);
     OUTPUT_ERROR();
     ReleaseScanner();
     return NS_OK;
   }
 
   bool parsedOK = ParseProperty(aPropID);
   // We should now be at EOF
   if (parsedOK && GetToken(true)) {
     REPORT_UNEXPECTED_TOKEN(PEExpectEndValue);
     parsedOK = false;
   }
 
   if (!parsedOK) {
     NS_ConvertASCIItoUTF16 propName(nsCSSProps::GetStringValue(aPropID));
-    const PRUnichar *params[] = {
-      propName.get()
-    };
-    REPORT_UNEXPECTED_P(PEValueParsingError, params);
+    REPORT_UNEXPECTED_P(PEValueParsingError, propName);
     REPORT_UNEXPECTED(PEDeclDropped);
     OUTPUT_ERROR();
     mTempData.ClearProperty(aPropID);
   } else {
 
     // We know we don't need to force a ValueAppended call for the new
     // value.  So if we are not processing a shorthand, and there's
     // already a value for this property in the declaration at the
@@ -1924,20 +1918,17 @@ CSSParserImpl::ProcessImport(const nsStr
   // Diagnose bad URIs even if we don't have a child loader.
   nsCOMPtr<nsIURI> url;
   // Charset will be deduced from mBaseURI, which is more or less correct.
   nsresult rv = NS_NewURI(getter_AddRefs(url), aURLSpec, nullptr, mBaseURI);
 
   if (NS_FAILED(rv)) {
     if (rv == NS_ERROR_MALFORMED_URI) {
       // import url is bad
-      const PRUnichar *params[] = {
-        aURLSpec.get()
-      };
-      REPORT_UNEXPECTED_P(PEImportBadURI, params);
+      REPORT_UNEXPECTED_P(PEImportBadURI, aURLSpec);
       OUTPUT_ERROR();
     }
     return;
   }
 
   if (mChildLoader) {
     mChildLoader->LoadChildSheet(mSheet, url, aMedia, rule);
   }
@@ -2208,29 +2199,23 @@ CSSParserImpl::ParseFontDescriptor(nsCSS
   nsCSSValue value;
 
   if (descID == eCSSFontDesc_UNKNOWN) {
     if (NonMozillaVendorIdentifier(descName)) {
       // silently skip other vendors' extensions
       SkipDeclaration(true);
       return true;
     } else {
-      const PRUnichar *params[] = {
-        descName.get()
-      };
-      REPORT_UNEXPECTED_P(PEUnknownFontDesc, params);
+      REPORT_UNEXPECTED_P(PEUnknownFontDesc, descName);
       return false;
     }
   }
 
   if (!ParseFontDescriptorValue(descID, value)) {
-    const PRUnichar *params[] = {
-      descName.get()
-    };
-    REPORT_UNEXPECTED_P(PEValueParsingError, params);
+    REPORT_UNEXPECTED_P(PEValueParsingError, descName);
     return false;
   }
 
   if (!ExpectEndProperty())
     return false;
 
   aRule->SetDesc(descID, value);
   return true;
@@ -2486,20 +2471,17 @@ CSSParserImpl::ParseSupportsConditionInP
     if (!mToken.mIdent.LowerCaseEqualsLiteral("not")) {
       nsAutoString propertyName = mToken.mIdent;
       if (!ExpectSymbol(':', true)) {
         REPORT_UNEXPECTED_TOKEN(PEParseDeclarationNoColon);
         return false;
       }
 
       if (ExpectSymbol(')', true)) {
-        const PRUnichar *params[] = {
-          propertyName.get()
-        };
-        REPORT_UNEXPECTED_P(PEValueParsingError, params);
+        REPORT_UNEXPECTED_P(PEValueParsingError, propertyName);
         UngetToken();
         return false;
       }
 
       nsCSSProperty propID = nsCSSProps::LookupProperty(propertyName,
                                                         nsCSSProps::eEnabled);
       if (propID == eCSSProperty_UNKNOWN) {
         aConditionMet = false;
@@ -4129,22 +4111,17 @@ CSSParserImpl::ParseColorComponent(uint8
     return false;
   }
   if (ExpectSymbol(aStop, true)) {
     if (value < 0.0f) value = 0.0f;
     if (value > 255.0f) value = 255.0f;
     aComponent = NSToIntRound(value);
     return true;
   }
-  const PRUnichar stopString[] = { PRUnichar(aStop), PRUnichar(0) };
-  const PRUnichar *params[] = {
-    nullptr,
-    stopString
-  };
-  REPORT_UNEXPECTED_TOKEN_P(PEColorComponentBadTerm, params);
+  REPORT_UNEXPECTED_TOKEN_CHAR(PEColorComponentBadTerm, aStop);
   return false;
 }
 
 
 bool
 CSSParserImpl::ParseHSLColor(nscolor& aColor,
                              char aStop)
 {
@@ -4203,22 +4180,17 @@ CSSParserImpl::ParseHSLColor(nscolor& aC
   if (l < 0.0f) l = 0.0f;
   if (l > 1.0f) l = 1.0f;
 
   if (ExpectSymbol(aStop, true)) {
     aColor = NS_HSL2RGB(h, s, l);
     return true;
   }
 
-  const PRUnichar stopString[] = { PRUnichar(aStop), PRUnichar(0) };
-  const PRUnichar *params[] = {
-    nullptr,
-    stopString
-  };
-  REPORT_UNEXPECTED_TOKEN_P(PEColorComponentBadTerm, params);
+  REPORT_UNEXPECTED_TOKEN_CHAR(PEColorComponentBadTerm, aStop);
   return false;
 }
 
 
 bool
 CSSParserImpl::ParseColorOpacity(uint8_t& aOpacity)
 {
   if (!GetToken(true)) {
@@ -4336,32 +4308,26 @@ CSSParserImpl::ParseDeclaration(css::Dec
 
   // Map property name to its ID and then parse the property
   nsCSSProperty propID = nsCSSProps::LookupProperty(propertyName,
                                                     nsCSSProps::eEnabled);
   if (eCSSProperty_UNKNOWN == propID ||
      (aContext == nsCSSContextType::eCSSContext_Page &&
       !nsCSSProps::PropHasFlags(propID, CSS_PROPERTY_APPLIES_TO_PAGE_RULE))) { // unknown property
     if (!NonMozillaVendorIdentifier(propertyName)) {
-      const PRUnichar *params[] = {
-        propertyName.get()
-      };
-      REPORT_UNEXPECTED_P(PEUnknownProperty, params);
+      REPORT_UNEXPECTED_P(PEUnknownProperty, propertyName);
       REPORT_UNEXPECTED(PEDeclDropped);
       OUTPUT_ERROR();
     }
 
     return false;
   }
   if (! ParseProperty(propID)) {
     // XXX Much better to put stuff in the value parsers instead...
-    const PRUnichar *params[] = {
-      propertyName.get()
-    };
-    REPORT_UNEXPECTED_P(PEValueParsingError, params);
+    REPORT_UNEXPECTED_P(PEValueParsingError, propertyName);
     REPORT_UNEXPECTED(PEDeclDropped);
     OUTPUT_ERROR();
     mTempData.ClearProperty(propID);
     mTempData.AssertInitialState();
     return false;
   }
   CLEAR_ERROR();
 
@@ -9856,20 +9822,17 @@ CSSParserImpl::GetNamespaceIdForPrefix(c
     if (!prefix) {
       NS_RUNTIMEABORT("do_GetAtom failed - out of memory?");
     }
     nameSpaceID = mNameSpaceMap->FindNameSpaceID(prefix);
   }
   // else no declared namespaces
 
   if (nameSpaceID == kNameSpaceID_Unknown) {   // unknown prefix, dump it
-    const PRUnichar *params[] = {
-      aPrefix.get()
-    };
-    REPORT_UNEXPECTED_P(PEUnknownNamespacePrefix, params);
+    REPORT_UNEXPECTED_P(PEUnknownNamespacePrefix, aPrefix);
   }
 
   return nameSpaceID;
 }
 
 void
 CSSParserImpl::SetDefaultNamespaceOnSelector(nsCSSSelector& aSelector)
 {
--- a/layout/style/nsCSSScanner.cpp
+++ b/layout/style/nsCSSScanner.cpp
@@ -985,31 +985,31 @@ nsCSSScanner::ParseString(int32_t aStop,
       }
     }
     int32_t ch = Read();
     if (ch < 0 || ch == aStop) {
       break;
     }
     if (ch == '\n') {
       aToken.mType = eCSSToken_Bad_String;
-      mReporter->ReportUnexpectedToken("SEUnterminatedString", aToken);
+      mReporter->ReportUnexpected("SEUnterminatedString", aToken);
       break;
     }
     if (ch == '\\') {
       if (!ParseAndAppendEscape(aToken.mIdent, true)) {
         aToken.mType = eCSSToken_Bad_String;
         Pushback(ch);
         // For strings, the only case where ParseAndAppendEscape will
         // return false is when there's a backslash to start an escape
         // immediately followed by end-of-stream.  In that case, the
         // correct tokenization is badstring *followed* by a DELIM for
         // the backslash, but as far as the author is concerned, it
         // works pretty much the same as an unterminated string, so we
         // use the same error message.
-        mReporter->ReportUnexpectedToken("SEUnterminatedString", aToken);
+        mReporter->ReportUnexpected("SEUnterminatedString", aToken);
         break;
       }
     } else {
       aToken.mIdent.Append(ch);
     }
   }
   return true;
 }