Bug 1612444. Improve some error messages for exceptions in layout. r=…

…emilio

I used ThrowNotSupportedError for the not-implemented case.

Differential Revision: https://phabricator.services.mozilla.com/D61324

--HG--
extra : moz-landing-system : lando

dom/base/AnonymousContent.cpp

@@ -216,7 +216,7 @@ void AnonymousContent::SetStyle(const nsACString& aProperty,
 
   nsGenericHTMLElement* element = nsGenericHTMLElement::FromNode(mContentNode);
   nsCOMPtr<nsICSSDeclaration> declaration = element->Style();
-  declaration->SetProperty(aProperty, aValue, EmptyString());
+  declaration->SetProperty(aProperty, aValue, EmptyString(), IgnoreErrors());
 }
 
 }  // namespace dom

dom/smil/SMILCSSProperty.cpp

@@ -102,7 +102,7 @@ nsresult SMILCSSProperty::SetAnimValue(const SMILValue& aValue) {
 void SMILCSSProperty::ClearAnimValue() {
   // Put empty string in override style for our property
   mElement->SMILOverrideStyle()->SetPropertyValue(mPropID, EmptyCString(),
-                                                  nullptr);
+                                                  nullptr, IgnoreErrors());
 }
 
 // Based on http://www.w3.org/TR/SVG/propidx.html

editor/libeditor/ChangeStyleTransaction.cpp

@@ -171,18 +171,27 @@ ChangeStyleTransaction::DoTransaction() {
       RemoveValueFromListOfValues(values, NS_LITERAL_STRING("none"));
       RemoveValueFromListOfValues(values, mValue);
       if (values.IsEmpty()) {
-        rv = cssDecl->RemoveProperty(propertyNameString, returnString);
-        NS_ENSURE_SUCCESS(rv, rv);
+        ErrorResult res;
+        cssDecl->RemoveProperty(propertyNameString, returnString, res);
+        if (NS_WARN_IF(res.Failed())) {
+          return res.StealNSResult();
+        }
       } else {
+        ErrorResult res;
         nsAutoString priority;
         cssDecl->GetPropertyPriority(propertyNameString, priority);
-        rv = cssDecl->SetProperty(propertyNameString,
-                                  NS_ConvertUTF16toUTF8(values), priority);
-        NS_ENSURE_SUCCESS(rv, rv);
+        cssDecl->SetProperty(propertyNameString, NS_ConvertUTF16toUTF8(values),
+                             priority, res);
+        if (NS_WARN_IF(res.Failed())) {
+          return res.StealNSResult();
+        }
       }
     } else {
-      rv = cssDecl->RemoveProperty(propertyNameString, returnString);
-      NS_ENSURE_SUCCESS(rv, rv);
+      ErrorResult res;
+      cssDecl->RemoveProperty(propertyNameString, returnString, res);
+      if (NS_WARN_IF(res.Failed())) {
+        return res.StealNSResult();
+      }
     }
   } else {
     nsAutoString priority;
@@ -193,9 +202,12 @@ ChangeStyleTransaction::DoTransaction() {
     } else {
       values.Assign(mValue);
     }
-    rv = cssDecl->SetProperty(propertyNameString, NS_ConvertUTF16toUTF8(values),
-                              priority);
-    NS_ENSURE_SUCCESS(rv, rv);
+    ErrorResult res;
+    cssDecl->SetProperty(propertyNameString, NS_ConvertUTF16toUTF8(values),
+                         priority, res);
+    if (NS_WARN_IF(res.Failed())) {
+      return res.StealNSResult();
+    }
   }
 
   // Let's be sure we don't keep an empty style attribute
@@ -221,16 +233,21 @@ nsresult ChangeStyleTransaction::SetStyle(bool aAttributeWasSet,
     NS_ENSURE_TRUE(inlineStyles, NS_ERROR_NULL_POINTER);
     nsCOMPtr<nsICSSDeclaration> cssDecl = inlineStyles->Style();
 
+    ErrorResult rv;
     if (aValue.IsEmpty()) {
       // An empty value means we have to remove the property
       nsAutoString returnString;
-      return cssDecl->RemoveProperty(propertyNameString, returnString);
+      cssDecl->RemoveProperty(propertyNameString, returnString, rv);
+      if (NS_WARN_IF(rv.Failed())) {
+        return rv.StealNSResult();
+      }
     }
     // Let's recreate the declaration as it was
     nsAutoString priority;
     cssDecl->GetPropertyPriority(propertyNameString, priority);
-    return cssDecl->SetProperty(propertyNameString,
-                                NS_ConvertUTF16toUTF8(aValue), priority);
+    cssDecl->SetProperty(propertyNameString, NS_ConvertUTF16toUTF8(aValue),
+                         priority, rv);
+    return rv.StealNSResult();
   }
   return mElement->UnsetAttr(kNameSpaceID_None, nsGkAtoms::style, true);
 }

layout/base/GeometryUtils.cpp

@@ -283,12 +283,14 @@ void GetBoxQuads(nsINode* aNode, const dom::BoxQuadOptions& aOptions,
     }
   }
   if (!relativeToFrame) {
-    aRv.Throw(NS_ERROR_DOM_NOT_FOUND_ERR);
-    return;
+    // XXXbz There's no spec for this.
+    return aRv.ThrowNotFoundError("No box to get quads relative to");
   }
   if (!CheckFramesInSameTopLevelBrowsingContext(frame, relativeToFrame,
                                                 aCallerType)) {
-    aRv.Throw(NS_ERROR_DOM_NOT_FOUND_ERR);
+    aRv.ThrowNotFoundError(
+        "Can't get quads relative to a box in a different toplevel browsing "
+        "context");
     return;
   }
   // GetBoxRectForFrame can modify relativeToFrame so call it first.
@@ -313,12 +315,15 @@ static void TransformPoints(nsINode* aTo, const GeometryNode& aFrom,
     fromFrame = GetFirstNonAnonymousFrameForGeometryNode(aFrom);
   }
   if (!fromFrame || !toFrame) {
-    aRv.Throw(NS_ERROR_DOM_NOT_FOUND_ERR);
+    aRv.ThrowNotFoundError(
+        "Can't transform coordinates between nonexistent boxes");
     return;
   }
   if (!CheckFramesInSameTopLevelBrowsingContext(fromFrame, toFrame,
                                                 aCallerType)) {
-    aRv.Throw(NS_ERROR_DOM_NOT_FOUND_ERR);
+    aRv.ThrowNotFoundError(
+        "Can't transform coordinates between boxes in different toplevel "
+        "browsing contexts");
     return;
   }
 
@@ -351,7 +356,7 @@ already_AddRefed<DOMQuad> ConvertQuadFromNode(
   for (uint32_t i = 0; i < 4; ++i) {
     DOMPoint* p = aQuad.Point(i);
     if (p->W() != 1.0 || p->Z() != 0.0) {
-      aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+      aRv.ThrowInvalidStateError("Point is not 2d");
       return nullptr;
     }
     points[i] = CSSPoint(p->X(), p->Y());
@@ -387,7 +392,7 @@ already_AddRefed<DOMPoint> ConvertPointFromNode(
     const dom::ConvertCoordinateOptions& aOptions, CallerType aCallerType,
     ErrorResult& aRv) {
   if (aPoint.mW != 1.0 || aPoint.mZ != 0.0) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    aRv.ThrowInvalidStateError("Point is not 2d");
     return nullptr;
   }
   CSSPoint point(aPoint.mX, aPoint.mY);

layout/generic/ScrollbarActivity.cpp

@@ -312,7 +312,8 @@ static void SetOpacityOnElement(nsIContent* aContent, double aOpacity) {
     nsICSSDeclaration* decl = inlineStyleContent->Style();
     nsAutoCString str;
     str.AppendFloat(aOpacity);
-    decl->SetProperty(NS_LITERAL_CSTRING("opacity"), str, EmptyString());
+    decl->SetProperty(NS_LITERAL_CSTRING("opacity"), str, EmptyString(),
+                      IgnoreErrors());
   }
 }
 
@@ -342,7 +343,7 @@ static void UnsetOpacityOnElement(nsIContent* aContent) {
   if (inlineStyleContent) {
     nsICSSDeclaration* decl = inlineStyleContent->Style();
     nsAutoString dummy;
-    decl->RemoveProperty(NS_LITERAL_CSTRING("opacity"), dummy);
+    decl->RemoveProperty(NS_LITERAL_CSTRING("opacity"), dummy, IgnoreErrors());
   }
 }
 

layout/inspector/InspectorUtils.cpp

@@ -673,7 +673,7 @@ void InspectorUtils::ParseStyleSheet(GlobalObject& aGlobalObject,
                                      StyleSheet& aSheet,
                                      const nsACString& aInput,
                                      ErrorResult& aRv) {
-  aRv = aSheet.ReparseSheet(aInput);
+  aSheet.ReparseSheet(aInput, aRv);
 }
 
 bool InspectorUtils::IsCustomElementName(GlobalObject&, const nsAString& aName,

layout/style/CSSFontFaceRule.cpp

@@ -52,7 +52,9 @@ void CSSFontFaceRuleDecl::SetCssText(const nsAString& aCssText,
   if (ContainingRule()->IsReadOnly()) {
     return;
   }
-  aRv.Throw(NS_ERROR_NOT_IMPLEMENTED);  // bug 443978
+  // bug 443978
+  aRv.ThrowNotSupportedError(
+      "Can't set cssText on CSSFontFaceRule declarations");
 }
 
 NS_IMETHODIMP
@@ -66,23 +68,21 @@ CSSFontFaceRuleDecl::GetPropertyValue(const nsACString& aPropName,
   return NS_OK;
 }
 
-NS_IMETHODIMP
-CSSFontFaceRuleDecl::RemoveProperty(const nsACString& aPropName,
-                                    nsAString& aResult) {
+void CSSFontFaceRuleDecl::RemoveProperty(const nsACString& aPropName,
+                                         nsAString& aResult, ErrorResult& aRv) {
   nsCSSFontDesc descID = nsCSSProps::LookupFontDesc(aPropName);
   NS_ASSERTION(descID >= eCSSFontDesc_UNKNOWN && descID < eCSSFontDesc_COUNT,
                "LookupFontDesc returned value out of range");
 
   if (ContainingRule()->IsReadOnly()) {
-    return NS_OK;
+    return;
   }
 
   aResult.Truncate();
   if (descID != eCSSFontDesc_UNKNOWN) {
     GetPropertyValue(descID, aResult);
     Servo_FontFaceRule_ResetDescriptor(mRawRule, descID);
   }
-  return NS_OK;
 }
 
 void CSSFontFaceRuleDecl::GetPropertyPriority(const nsACString& aPropName,
@@ -91,19 +91,21 @@ void CSSFontFaceRuleDecl::GetPropertyPriority(const nsACString& aPropName,
   aResult.Truncate();
 }
 
-NS_IMETHODIMP
-CSSFontFaceRuleDecl::SetProperty(const nsACString& aPropName,
-                                 const nsACString& aValue,
-                                 const nsAString& aPriority,
-                                 nsIPrincipal* aSubjectPrincipal) {
+void CSSFontFaceRuleDecl::SetProperty(const nsACString& aPropName,
+                                      const nsACString& aValue,
+                                      const nsAString& aPriority,
+                                      nsIPrincipal* aSubjectPrincipal,
+                                      ErrorResult& aRv) {
   // FIXME(heycam): If we are changing unicode-range, then a FontFace object
   // representing this rule must have its mUnicodeRange value invalidated.
 
   if (ContainingRule()->IsReadOnly()) {
-    return NS_OK;
+    return;
   }
 
-  return NS_ERROR_NOT_IMPLEMENTED;  // bug 443978
+  // bug 443978
+  aRv.ThrowNotSupportedError(
+      "Can't set properties on CSSFontFaceRule declarations");
 }
 
 uint32_t CSSFontFaceRuleDecl::Length() {

layout/style/FontFace.cpp

@@ -507,7 +507,7 @@ bool FontFace::SetDescriptor(nsCSSFontDesc aFontDesc, const nsAString& aValue,
   bool changed;
   if (!Servo_FontFaceRule_SetDescriptor(GetData(), aFontDesc, &value, url,
                                         &changed)) {
-    aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
+    aRv.ThrowSyntaxError("Invalid font descriptor");
     return false;
   }
 

layout/style/FontFaceSet.cpp

@@ -210,7 +210,7 @@ void FontFaceSet::ParseFontShorthandForMatching(
   RefPtr<URLExtraData> url = ServoCSSParser::GetURLExtraData(mDocument);
   if (!ServoCSSParser::ParseFontShorthandForMatching(aFont, url, aFamilyList,
                                                      style, stretch, weight)) {
-    aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
+    aRv.ThrowSyntaxError("Invalid font shorthand");
     return;
   }
 
@@ -432,7 +432,8 @@ void FontFaceSet::Add(FontFace& aFontFace, ErrorResult& aRv) {
   }
 
   if (aFontFace.HasRule()) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_MODIFICATION_ERR);
+    aRv.ThrowInvalidModificationError(
+        "Can't add face to FontFaceSet that comes from an @font-face rule");
     return;
   }
 

layout/style/GroupRule.cpp

@@ -86,7 +86,9 @@ uint32_t GroupRule::InsertRule(const nsAString& aRule, uint32_t aIndex,
 
   uint32_t count = StyleRuleCount();
   if (aIndex > count) {
-    aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
+    aRv.ThrowIndexSizeError(nsPrintfCString(
+        "Can't insert rule at index %u because rule list length is %u", aIndex,
+        count));
     return 0;
   }
 
@@ -113,7 +115,8 @@ void GroupRule::DeleteRule(uint32_t aIndex, ErrorResult& aRv) {
 
   uint32_t count = StyleRuleCount();
   if (aIndex >= count) {
-    aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
+    aRv.ThrowIndexSizeError(nsPrintfCString(
+        "Index %u is too large for list of length %u", aIndex, count));
     return;
   }
 

layout/style/StyleSheet.cpp

@@ -813,7 +813,7 @@ void StyleSheet::SubjectSubsumesInnerPrincipal(nsIPrincipal& aSubjectPrincipal,
   // Allow access only if CORS mode is not NONE and the security flag
   // is not turned off.
   if (GetCORSMode() == CORS_NONE && !nsContentUtils::BypassCSSOMOriginCheck()) {
-    aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
+    aRv.ThrowSecurityError("Not allowed to access cross-origin stylesheet");
     return;
   }
 
@@ -829,7 +829,8 @@ void StyleSheet::SubjectSubsumesInnerPrincipal(nsIPrincipal& aSubjectPrincipal,
   // if we're not complete yet.  Luckily, all the callers of this method throw
   // anyway if not complete, so we can just do that here too.
   if (!IsComplete()) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
+    aRv.ThrowInvalidAccessError(
+        "Not allowed to access still-loading stylesheet");
     return;
   }
 
@@ -842,7 +843,8 @@ bool StyleSheet::AreRulesAvailable(nsIPrincipal& aSubjectPrincipal,
                                    ErrorResult& aRv) {
   // Rules are not available on incomplete sheets.
   if (!IsComplete()) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
+    aRv.ThrowInvalidAccessError(
+        "Can't access rules of still-loading stylsheet");
     return false;
   }
   //-- Security check: Only scripts whose principal subsumes that of the
@@ -1114,16 +1116,16 @@ void StyleSheet::FinishParse() {
   SetSourceURL(sourceURL);
 }
 
-nsresult StyleSheet::ReparseSheet(const nsACString& aInput) {
+void StyleSheet::ReparseSheet(const nsACString& aInput, ErrorResult& aRv) {
   if (!IsComplete()) {
-    return NS_ERROR_DOM_INVALID_ACCESS_ERR;
+    return aRv.ThrowInvalidAccessError("Cannot reparse still-loading sheet");
   }
 
   // Allowing to modify UA sheets is dangerous (in the sense that C++ code
   // relies on rules in those sheets), plus they're probably going to be shared
   // across processes in which case this is directly a no-go.
   if (IsReadOnly()) {
-    return NS_OK;
+    return;
   }
 
   // Hold strong ref to the CSSLoader in case the document update
@@ -1195,8 +1197,6 @@ nsresult StyleSheet::ReparseSheet(const nsACString& aInput) {
 
   // Our rules are no longer considered modified for devtools.
   mState &= ~State::ModifiedRulesForDevtools;
-
-  return NS_OK;
 }
 
 void StyleSheet::DropRuleList() {
@@ -1266,8 +1266,6 @@ void StyleSheet::DeleteRuleInternal(uint32_t aIndex, ErrorResult& aRv) {
   // event is not enabled.
   RefPtr<css::Rule> rule = mRuleList->GetRule(aIndex);
   aRv = mRuleList->DeleteRule(aIndex);
-  MOZ_ASSERT(!aRv.ErrorCodeIs(NS_ERROR_DOM_INDEX_SIZE_ERR),
-             "IndexSizeError should have been handled earlier");
   if (!aRv.Failed()) {
     RuleRemoved(*rule);
   }

layout/style/StyleSheet.h

@@ -133,7 +133,7 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache {
       css::SheetLoadData* aLoadData, uint32_t aLineNumber,
       css::LoaderReusableStyleSheets* aReusableSheets = nullptr);
 
-  nsresult ReparseSheet(const nsACString& aInput);
+  void ReparseSheet(const nsACString& aInput, ErrorResult& aRv);
 
   const RawServoStyleSheetContents* RawContents() const {
     return Inner().mContents;