Bug 614131 - Don't leave wrapped exception in the context on failure to enter compartment (r=gal)

--HG--
extra : rebase_source : 325d545ef79b88f3983b86a7dde262acf9e60e1f

js/src/jscntxt.cpp

@@ -2019,14 +2019,8 @@ JSContext::resetCompartment()
 
     compartment = scopeobj->compartment();
 
-    /*
-     * If wrapException fails, it overrides this->exception and
-     * reports OOM. The upshot is that we silently turn the exception
-     * into an uncatchable OOM error. A bit surprising, but the
-     * caller is just going to return false either way.
-     */
     if (isExceptionPending())
-        (void) compartment->wrapException(this);
+        wrapPendingException();
     return;
 
 error:
@@ -2038,6 +2032,21 @@ error:
     compartment = NULL;
 }
 
+/*
+ * Since this function is only called in the context of a pending exception,
+ * the caller must subsequently take an error path. If wrapping fails, we leave
+ * the exception cleared, which, in the context of an error path, will be
+ * interpreted as an uncatchable exception.
+ */
+void
+JSContext::wrapPendingException()
+{
+    Value v = getPendingException();
+    clearPendingException();
+    if (compartment->wrap(this, &v))
+        setPendingException(v);
+}
+
 void
 JSContext::pushSegmentAndFrame(js::StackSegment *newseg, JSFrameRegs &newregs)
 {

js/src/jscntxt.h

@@ -1768,6 +1768,7 @@ struct JSContext
     friend bool js::Interpret(JSContext *, JSStackFrame *, uintN, JSInterpMode);
 
     void resetCompartment();
+    void wrapPendingException();
 
     /* 'regs' must only be changed by calling this function. */
     void setCurrentRegs(JSFrameRegs *regs) {

js/src/jscompartment.cpp

@@ -353,21 +353,6 @@ JSCompartment::wrap(JSContext *cx, AutoIdVector &props)
     return true;
 }
 
-bool
-JSCompartment::wrapException(JSContext *cx)
-{
-    JS_ASSERT(cx->compartment == this);
-
-    if (cx->isExceptionPending()) {
-        Value v = cx->getPendingException();
-        cx->clearPendingException();
-        if (wrap(cx, &v))
-            cx->setPendingException(v);
-        return false;
-    }
-    return true;
-}
-
 #if defined JS_METHODJIT && defined JS_MONOIC
 /*
  * Check if the pool containing the code for jit should be destroyed, per the

js/src/jscompartment.h

@@ -327,7 +327,6 @@ struct JS_FRIEND_API(JSCompartment) {
     bool wrap(JSContext *cx, js::PropertyOp *op);
     bool wrap(JSContext *cx, js::PropertyDescriptor *desc);
     bool wrap(JSContext *cx, js::AutoIdVector &props);
-    bool wrapException(JSContext *cx);
 
     void sweep(JSContext *cx, uint32 releaseInterval);
     void purge(JSContext *cx);

js/src/jswrapper.cpp

@@ -348,19 +348,15 @@ AutoCompartment::enter()
     if (origin != destination) {
         LeaveTrace(context);
 
-#ifdef DEBUG
-        JSCompartment *oldCompartment = context->compartment;
-        context->resetCompartment();
-        wasSane = (context->compartment == oldCompartment);
-#endif
+        if (context->isExceptionPending())
+            return false;
 
         context->compartment = destination;
         JSObject *scopeChain = target->getGlobal();
         JS_ASSERT(scopeChain->isNative());
+
         frame.construct();
-        if (!context->stack().pushDummyFrame(context, *scopeChain, &frame.ref()) ||
-            !destination->wrapException(context)) {
-            frame.destroy();
+        if (!context->stack().pushDummyFrame(context, *scopeChain, &frame.ref())) {
             context->compartment = origin;
             return false;
         }
@@ -376,8 +372,6 @@ AutoCompartment::leave()
     if (origin != destination) {
         frame.destroy();
         context->resetCompartment();
-        JS_ASSERT_IF(wasSane && context->hasfp(), context->compartment == origin);
-        context->compartment->wrapException(context);
     }
     entered = false;
 }
@@ -642,8 +636,7 @@ JSCrossCompartmentWrapper::construct(JSContext *cx, JSObject *wrapper, uintN arg
         return false;
 
     call.leave();
-    return call.origin->wrap(cx, rval) &&
-           call.origin->wrapException(cx);
+    return call.origin->wrap(cx, rval);
 }
 
 bool

js/src/jswrapper.h

@@ -168,9 +168,6 @@ class AutoCompartment
     JSFrameRegs regs;
     AutoStringRooter input;
     bool entered;
-#ifdef DEBUG
-    bool wasSane;
-#endif
 
   public:
     AutoCompartment(JSContext *cx, JSObject *target);