Backed out 4 changesets (bug 1227190) for still having failures after several followups, most recently jit test assertions in jsfriendapi.cpp CLOSED TREE
authorWes Kocher <wkocher@mozilla.com>
Wed, 02 Dec 2015 12:26:23 -0800
changeset 309461 3b3cf9f8e17590d025c44f8a7d00bce46c7ece15
parent 309460 d49a71d3b20a24eccdfc7cb194406d21ffb5d085
child 309462 e257e66b682b6ee02bf61e7ba8bbcf778c38c447
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1227190
milestone45.0a1
backs oute3ae43ee51fa19f4ab685e5dfd8d1e051d011566
2e33a92988cd4c1b33d2b6652b58187f492fae4d
4ebb551d8db2a4a96851b27f1a867a44c18a7a0f
5f84bb42a723a215da4308794cfdf09e0cdb4973
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
Backed out 4 changesets (bug 1227190) for still having failures after several followups, most recently jit test assertions in jsfriendapi.cpp CLOSED TREE Backed out changeset e3ae43ee51fa (bug 1227190) Backed out changeset 2e33a92988cd (bug 1227190) Backed out changeset 4ebb551d8db2 (bug 1227190) Backed out changeset 5f84bb42a723 (bug 1227190)
js/src/ctypes/CTypes.cpp
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/vm/Debugger.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -6927,17 +6927,17 @@ CClosure::ClosureStub(ffi_cif* cif, void
   MOZ_ASSERT(result);
   MOZ_ASSERT(args);
   MOZ_ASSERT(userData);
 
   // Retrieve the essentials from our closure object.
   ArgClosure argClosure(cif, result, args, static_cast<ClosureInfo*>(userData));
   JSRuntime* rt = argClosure.cinfo->rt;
   RootedObject fun(rt, argClosure.cinfo->jsfnObj);
-  js::PrepareScriptEnvironmentAndInvoke(rt, fun, argClosure);
+  (void) js::PrepareScriptEnvironmentAndInvoke(rt, fun, argClosure);
 }
 
 bool CClosure::ArgClosure::operator()(JSContext* cx)
 {
   // Let the runtime callback know that we are about to call into JS again. The end callback will
   // fire automatically when we exit this function.
   js::AutoCTypesActivityCallback autoCallback(cx, js::CTYPES_CALLBACK_BEGIN,
                                               js::CTYPES_CALLBACK_END);
@@ -7020,24 +7020,21 @@ bool CClosure::ArgClosure::operator()(JS
       // the actual return value, and then proceed.
 
       // The buffer we're returning might be larger than the size of the return
       // type, due to libffi alignment issues (see above). But it should never
       // be smaller.
       size_t copySize = CType::GetSize(fninfo->mReturnType);
       MOZ_ASSERT(copySize <= rvSize);
       memcpy(result, cinfo->errResult, copySize);
-
-      // We still want to return false here, so that
-      // PrepareScriptEnvironmentAndInvoke will report the exception.
     } else {
       // Bad case: not much we can do here. The rv is already zeroed out, so we
       // just return and hope for the best.
+      return false;
     }
-    return false;
   }
 
   // Small integer types must be returned as a word-sized ffi_arg. Coerce it
   // back into the size libffi expects.
   switch (typeCode) {
 #define INTEGRAL_CASE(name, type, ffiType)                                     \
   case TYPE_##name:                                                            \
     if (sizeof(type) < sizeof(ffi_arg)) {                                      \
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -1140,39 +1140,34 @@ js::GetDOMProxyShadowsCheck()
 }
 
 bool
 js::detail::IdMatchesAtom(jsid id, JSAtom* atom)
 {
     return id == INTERNED_STRING_TO_JSID(nullptr, atom);
 }
 
-JS_FRIEND_API(void)
+JS_FRIEND_API(bool)
 js::PrepareScriptEnvironmentAndInvoke(JSRuntime* rt, HandleObject scope, ScriptEnvironmentPreparer::Closure& closure)
 {
-    if (rt->scriptEnvironmentPreparer) {
-        rt->scriptEnvironmentPreparer->invoke(scope, closure);
-        return;
-    }
+    if (rt->scriptEnvironmentPreparer)
+        return rt->scriptEnvironmentPreparer->invoke(scope, closure);
 
     MOZ_ASSERT(rt->contextList.getFirst() == rt->contextList.getLast());
     JSContext* cx = rt->contextList.getFirst();
-    MOZ_ASSERT(!cx->isExceptionPending());
     JSAutoCompartment ac(cx, scope);
     bool ok = closure(cx);
 
-    MOZ_ASSERT_IF(ok, !cx->isExceptionPending());
-
     // NB: This does not affect Gecko, which has a prepareScriptEnvironment
     // callback.
-    if (!ok) {
+    if (JS_IsExceptionPending(cx)) {
         JS_ReportPendingException(cx);
     }
 
-    MOZ_ASSERT(!cx->isExceptionPending());
+    return ok;
 }
 
 JS_FRIEND_API(void)
 js::SetScriptEnvironmentPreparer(JSRuntime* rt, ScriptEnvironmentPreparer* preparer)
 {
     rt->scriptEnvironmentPreparer = preparer;
 }
 
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -2572,37 +2572,32 @@ IdToValue(jsid id)
     return JS::UndefinedValue();
 }
 
 /**
  * If the embedder has registered a ScriptEnvironmentPreparer,
  * PrepareScriptEnvironmentAndInvoke will call the preparer's 'invoke' method
  * with the given |closure|, with the assumption that the preparer will set up
  * any state necessary to run script in |scope|, invoke |closure| with a valid
- * JSContext*, report any exceptions thrown from the closure, and return.
+ * JSContext*, and return.
  *
  * If no preparer is registered, PrepareScriptEnvironmentAndInvoke will assert
  * that |rt| has exactly one JSContext associated with it, enter the compartment
  * of |scope| on that context, and invoke |closure|.
- *
- * In both cases, PrepareScriptEnvironmentAndInvoke will report any exceptions
- * that are thrown by the closure.  Consumers who want to propagate back
- * whether the closure succeeded should do so via members of the closure
- * itself.
  */
 
 struct ScriptEnvironmentPreparer {
     struct Closure {
         virtual bool operator()(JSContext* cx) = 0;
     };
 
-    virtual void invoke(JS::HandleObject scope, Closure& closure) = 0;
+    virtual bool invoke(JS::HandleObject scope, Closure& closure) = 0;
 };
 
-extern JS_FRIEND_API(void)
+extern JS_FRIEND_API(bool)
 PrepareScriptEnvironmentAndInvoke(JSRuntime* rt, JS::HandleObject scope,
                                   ScriptEnvironmentPreparer::Closure& closure);
 
 JS_FRIEND_API(void)
 SetScriptEnvironmentPreparer(JSRuntime* rt, ScriptEnvironmentPreparer*
 preparer);
 
 /**
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -970,36 +970,16 @@ Debugger::unwrapPropertyDescriptor(JSCon
                 return false;
         }
         desc.setSetterObject(set);
     }
 
     return true;
 }
 
-namespace {
-class MOZ_STACK_CLASS ReportExceptionClosure : public ScriptEnvironmentPreparer::Closure
-{
-public:
-    explicit ReportExceptionClosure(RootedValue& exn)
-        : exn_(exn)
-    {
-    }
-
-    bool operator()(JSContext* cx) override
-    {
-        cx->setPendingException(exn_);
-        return false;
-    }
-
-private:
-    RootedValue& exn_;
-};
-} // anonymous namespace
-
 JSTrapStatus
 Debugger::handleUncaughtExceptionHelper(Maybe<AutoCompartment>& ac,
                                         MutableHandleValue* vp, bool callHook)
 {
     JSContext* cx = ac->context()->asJSContext();
     if (cx->isExceptionPending()) {
         if (callHook && uncaughtExceptionHook) {
             RootedValue exc(cx);
@@ -1008,33 +988,17 @@ Debugger::handleUncaughtExceptionHelper(
             cx->clearPendingException();
             RootedValue fval(cx, ObjectValue(*uncaughtExceptionHook));
             RootedValue rv(cx);
             if (Invoke(cx, ObjectValue(*object), fval, 1, exc.address(), &rv))
                 return vp ? parseResumptionValue(ac, true, rv, *vp, false) : JSTRAP_CONTINUE;
         }
 
         if (cx->isExceptionPending()) {
-            /*
-             * We want to report the pending exception, but we want to let the
-             * embedding handle it however it wants to.  So pretend like we're
-             * starting a new script execution on our current compartment (which
-             * is the debugger compartment, so reported errors won't get
-             * reported to various onerror handlers in debuggees) and as part of
-             * that "execution" simply throw our exception so the embedding can
-             * deal.
-             */
-            RootedValue exn(cx);
-            if (cx->getPendingException(&exn)) {
-                ReportExceptionClosure reportExn(exn);
-                PrepareScriptEnvironmentAndInvoke(cx->runtime(), cx->global(), reportExn);
-            }
-            // And if not, or if PrepareScriptEnvironmentAndInvoke somehow left
-            // an exception on cx (which it totally shouldn't do), just give
-            // up.
+            JS_ReportPendingException(cx);
             cx->clearPendingException();
         }
     }
     ac.reset();
     return JSTRAP_ERROR;
 }
 
 JSTrapStatus
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -1323,35 +1323,26 @@ XPCJSRuntime::GetWatchdogTimestamp(Watch
 }
 
 void
 xpc::SimulateActivityCallback(bool aActive)
 {
     XPCJSRuntime::ActivityCallback(XPCJSRuntime::Get(), aActive);
 }
 
-void
+// static
+bool
 XPCJSRuntime::EnvironmentPreparer::invoke(HandleObject scope, js::ScriptEnvironmentPreparer::Closure& closure)
 {
     MOZ_ASSERT(NS_IsMainThread());
     nsIGlobalObject* global = NativeGlobal(scope);
-
-    // Not much we can do if we simply don't have a usable global here...
-    NS_ENSURE_TRUE_VOID(global && global->GetGlobalJSObject());
+    NS_ENSURE_TRUE(global && global->GetGlobalJSObject(), false);
     AutoEntryScript aes(global, "JS-engine-initiated execution");
     aes.TakeOwnershipOfErrorReporting();
-
-    MOZ_ASSERT(!JS_IsExceptionPending(aes.cx()));
-
-    DebugOnly<bool> ok = closure(aes.cx());
-
-    MOZ_ASSERT_IF(ok, !JS_IsExceptionPending(aes.cx()));
-
-    // The AutoEntryScript will check for JS_IsExceptionPending on the
-    // JSContext and report it as needed as it comes off the stack.
+    return closure(aes.cx());
 }
 
 // static
 void
 XPCJSRuntime::ActivityCallback(void* arg, bool active)
 {
     if (!active) {
         ProcessHangMonitor::ClearHang();
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -587,17 +587,17 @@ public:
 
     ShortLivedStringBuffer<nsString> mScratchStrings;
     ShortLivedStringBuffer<nsCString> mScratchCStrings;
 
     void AddGCCallback(xpcGCCallback cb);
     void RemoveGCCallback(xpcGCCallback cb);
 
     struct EnvironmentPreparer : public js::ScriptEnvironmentPreparer {
-        void invoke(JS::HandleObject scope, Closure& closure) override;
+        bool invoke(JS::HandleObject scope, Closure& closure) override;
     };
     EnvironmentPreparer mEnvironmentPreparer;
 
     static void ActivityCallback(void* arg, bool active);
     static bool InterruptCallback(JSContext* cx);
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);