Bug 1338272 - Require that the return value of MaybeSetPendingException is used. r=bz
authorAndrew McCreight <continuation@gmail.com>
Tue, 14 Feb 2017 16:17:02 -0800
changeset 352606 5505b53a0acb3dcf2c6a565169d99c23e1c8da52
parent 352605 979d89d569f20b7530885e1f2be88dafd6cce9cc
child 352607 d0e97ee3f273891d178a2fef23897cfaa00f2779
push id89122
push userkwierso@gmail.com
push dateThu, 13 Apr 2017 01:24:36 +0000
treeherdermozilla-inbound@b31c8835bfc3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1338272
milestone55.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 1338272 - Require that the return value of MaybeSetPendingException is used. r=bz Most of the time, the return value of this method should be checked, because behavior should depend on whether or not an exception is thrown. However, if it is called immediately after a throw it doesn't need to be checked because it will always return true. bz said there is no public API that lets you assume there is an exception because it would be "too easy to misuse". MozReview-Commit-ID: CqyicBbcNjW
dom/base/WindowNamedPropertiesHandler.cpp
dom/bindings/Codegen.py
dom/bindings/ErrorResult.h
dom/bindings/ToJSValue.cpp
js/xpconnect/wrappers/AccessCheck.cpp
--- a/dom/base/WindowNamedPropertiesHandler.cpp
+++ b/dom/base/WindowNamedPropertiesHandler.cpp
@@ -158,17 +158,17 @@ bool
 WindowNamedPropertiesHandler::defineProperty(JSContext* aCx,
                                              JS::Handle<JSObject*> aProxy,
                                              JS::Handle<jsid> aId,
                                              JS::Handle<JS::PropertyDescriptor> aDesc,
                                              JS::ObjectOpResult &result) const
 {
   ErrorResult rv;
   rv.ThrowTypeError<MSG_DEFINEPROPERTY_ON_GSP>();
-  rv.MaybeSetPendingException(aCx);
+  MOZ_ALWAYS_TRUE(rv.MaybeSetPendingException(aCx));
   return false;
 }
 
 bool
 WindowNamedPropertiesHandler::ownPropNames(JSContext* aCx,
                                            JS::Handle<JSObject*> aProxy,
                                            unsigned flags,
                                            JS::AutoIdVector& aProps) const
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -5474,17 +5474,17 @@ def getJSToNativeConversionInfo(type, de
               if (!JS_WrapValue(cx, &valueToResolve)) {
                 $*{exceptionCode}
               }
               binding_detail::FastErrorResult promiseRv;
               nsCOMPtr<nsIGlobalObject> global =
                 do_QueryInterface(promiseGlobal.GetAsSupports());
               if (!global) {
                 promiseRv.ThrowWithCustomCleanup(NS_ERROR_UNEXPECTED);
-                promiseRv.MaybeSetPendingException(cx);
+                MOZ_ALWAYS_TRUE(promiseRv.MaybeSetPendingException(cx));
                 $*{exceptionCode}
               }
               $${declName} = Promise::Resolve(global, cx, valueToResolve,
                                               promiseRv);
               if (promiseRv.MaybeSetPendingException(cx)) {
                 $*{exceptionCode}
               }
             }
--- a/dom/bindings/ErrorResult.h
+++ b/dom/bindings/ErrorResult.h
@@ -257,16 +257,17 @@ public:
   //
   // Note that a true return value does NOT mean there is now a pending
   // exception on aCx, due to uncatchable exceptions.  It should still be
   // considered equivalent to a JSAPI failure in terms of what callers should do
   // after true is returned.
   //
   // After this call, the TErrorResult will no longer return true from Failed(),
   // since the exception will have moved to the JSContext.
+  MOZ_MUST_USE
   bool MaybeSetPendingException(JSContext* cx)
   {
     WouldReportJSException();
     if (!Failed()) {
       return false;
     }
 
     SetPendingException(cx);
--- a/dom/bindings/ToJSValue.cpp
+++ b/dom/bindings/ToJSValue.cpp
@@ -51,20 +51,18 @@ ToJSValue(JSContext* aCx,
 bool
 ToJSValue(JSContext* aCx,
           ErrorResult& aArgument,
           JS::MutableHandle<JS::Value> aValue)
 {
   MOZ_ASSERT(aArgument.Failed());
   MOZ_ASSERT(!aArgument.IsUncatchableException(),
              "Doesn't make sense to convert uncatchable exception to a JS value!");
-  DebugOnly<bool> throwResult = aArgument.MaybeSetPendingException(aCx);
-  MOZ_ASSERT(throwResult);
-  DebugOnly<bool> getPendingResult = JS_GetPendingException(aCx, aValue);
-  MOZ_ASSERT(getPendingResult);
+  MOZ_ALWAYS_TRUE(aArgument.MaybeSetPendingException(aCx));
+  MOZ_ALWAYS_TRUE(JS_GetPendingException(aCx, aValue));
   JS_ClearPendingException(aCx);
   return true;
 }
 
 bool
 ToJSValue(JSContext* aCx, Promise& aArgument,
           JS::MutableHandle<JS::Value> aValue)
 {
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -315,17 +315,17 @@ AccessCheck::reportCrossOriginDenial(JSC
         message = NS_LITERAL_CSTRING("Permission denied to ") +
                   accessType +
                   NS_LITERAL_CSTRING(" property ") +
                   NS_ConvertUTF16toUTF8(propName) +
                   NS_LITERAL_CSTRING(" on cross-origin object");
     }
     ErrorResult rv;
     rv.ThrowDOMException(NS_ERROR_DOM_SECURITY_ERR, message);
-    rv.MaybeSetPendingException(cx);
+    MOZ_ALWAYS_TRUE(rv.MaybeSetPendingException(cx));
 }
 
 enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 };
 
 static void
 EnterAndThrowASCII(JSContext* cx, JSObject* wrapper, const char* msg)
 {
     JSAutoCompartment ac(cx, wrapper);