bug 196241 - Refactors script evaluation pre and post logic into a class. r=timeless, sr=brendan

js/src/xpconnect/src/XPCDispTearOff.cpp

@@ -321,19 +321,15 @@ STDMETHODIMP XPCDispatchTearOff::Invoke(DISPID dispIdMember, REFIID riid,
         void* mark;
         JSBool foundDependentParam;
         JSObject* thisObj;
-        JSExceptionState* saved_exception = nsnull;
+        AutoScriptEvaluate scriptEval(ccx);
         XPCJSRuntime* rt = ccx.GetRuntime();
 
         thisObj = obj = GetJSObject();;
 
-        if(cx)
-            older = JS_SetErrorReporter(cx, xpcWrappedJSErrorReporter);
-
-        // dispID's for us are 1 based not zero
         if(!cx || !xpcc)
             goto pre_call_clean_up;
 
-        saved_exception = xpc_DoPreScriptEvaluated(cx);
+        scriptEval.StartEvaluating(xpcWrappedJSErrorReporter);
 
         xpcc->SetPendingResult(pending_result);
         xpcc->SetException(nsnull);
@@ -535,11 +531,6 @@ done:
         if(sp)
             js_FreeStack(cx, mark);
 
-        if(cx)
-        {
-            JS_SetErrorReporter(cx, older);
-            xpc_DoPostScriptEvaluated(cx, saved_exception);
-        }
         // TODO: I think we may need to translate this error, 
         // for now we'll pass through
         return retval;

js/src/xpconnect/src/xpcprivate.h

@@ -3079,8 +3079,46 @@ private:
     JSContext*        mCX;
     JSErrorReporter   mOldErrorReporter;
     JSExceptionState* mOldExceptionState;
- };
+};
 
+/******************************************************************************
+ * Handles pre/post script processing and the setting/resetting the error
+ * reporter
+ */
+class AutoScriptEvaluate
+{
+public:
+    /**
+     * Saves the JSContext as well as initializing our state
+     * @param cx The JSContext, this can be null, we don't do anything then
+     */
+    AutoScriptEvaluate(JSContext * cx)
+         : mJSContext(cx), mState(0), mEvaluated(PR_FALSE), 
+           mContextHasThread(0) {}
+
+    /**
+     * Does the pre script evaluation and sets the error reporter if given
+     * This function should only be called once, and will assert if called
+     * more than once
+     * @param errorReporter the error reporter callback function to set
+     */
+
+    void StartEvaluating(JSErrorReporter errorReporter = nsnull);
+    /**
+     * Does the post script evaluation and resets the error reporter
+     */
+    ~AutoScriptEvaluate();
+private:
+    JSContext* mJSContext;
+    JSExceptionState* mState;
+    JSErrorReporter mOldErrorReporter;
+    PRBool mEvaluated;
+    jsword mContextHasThread;
+
+    // No copying or assignment allowed
+    AutoScriptEvaluate(const AutoScriptEvaluate &);
+    AutoScriptEvaluate & operator =(const AutoScriptEvaluate &);
+};
 
 /***************************************************************************/
 class AutoResolveName
@@ -3263,8 +3301,6 @@ protected:
 /***************************************************************************/
 // Utilities
 
-JSExceptionState* xpc_DoPreScriptEvaluated(JSContext* cx);
-void xpc_DoPostScriptEvaluated(JSContext* cx, JSExceptionState* state);
 JSBool xpc_IsReportableErrorCode(nsresult code);
 
 /***************************************************************************/

js/src/xpconnect/src/xpcwrappedjsclass.cpp

@@ -43,10 +43,17 @@ NS_IMPL_THREADSAFE_ISUPPORTS1(nsXPCWrappedJSClass, nsIXPCWrappedJSClass)
 // the value of this variable is never used - we use its address as a sentinel
 static uint32 zero_methods_descriptor;
 
-JSExceptionState* xpc_DoPreScriptEvaluated(JSContext* cx)
+void AutoScriptEvaluate::StartEvaluating(JSErrorReporter errorReporter)
 {
-    if(JS_GetContextThread(cx))
+    NS_PRECONDITION(!mEvaluated, "AutoScriptEvaluate::Evaluate should only be called once");
-        JS_BeginRequest(cx);
+
+    if(!mJSContext)
+        return;
+    mEvaluated = PR_TRUE;
+    mOldErrorReporter = JS_SetErrorReporter(mJSContext, errorReporter);
+    mContextHasThread = JS_GetContextThread(mJSContext);
+    if (mContextHasThread)
+        JS_BeginRequest(mJSContext);
 
     // Saving the exception state keeps us from interfering with another script
     // that may also be running on this context.  This occurred first with the
@@ -58,24 +65,24 @@ JSExceptionState* xpc_DoPreScriptEvaluated(JSContext* cx)
     // and addroot, we avoid them if possible by returning null (as opposed to
     // a JSExceptionState with no information) when there is no pending
     // exception.
-    if(JS_IsExceptionPending(cx))
+    if(JS_IsExceptionPending(mJSContext))
     {
-        JSExceptionState* state = JS_SaveExceptionState(cx);
+        mState = JS_SaveExceptionState(mJSContext);
-        JS_ClearPendingException(cx);
+        JS_ClearPendingException(mJSContext);
-        return state;
     }
-    return nsnull;
 }
 
-void xpc_DoPostScriptEvaluated(JSContext* cx, JSExceptionState* state)
+AutoScriptEvaluate::~AutoScriptEvaluate()
 {
-    if(state)
+    if(!mJSContext || !mEvaluated)
-        JS_RestoreExceptionState(cx, state);
+        return;
+    if(mState)
+        JS_RestoreExceptionState(mJSContext, mState);
     else
-        JS_ClearPendingException(cx);
+        JS_ClearPendingException(mJSContext);
 
-    if(JS_GetContextThread(cx))
+    if(mContextHasThread)
-        JS_EndRequest(cx);
+        JS_EndRequest(mJSContext);
 
     // If this is a JSContext that has a private context that provides a
     // nsIXPCScriptNotify interface, then notify the object the script has
@@ -85,17 +92,15 @@ void xpc_DoPostScriptEvaluated(JSContext* cx, JSExceptionState* state)
     // private data that points to an nsISupports subclass, it has also set
     // the JSOPTION_PRIVATE_IS_NSISUPPORTS option.
 
-    nsISupports *supports =
+    if (JS_GetOptions(mJSContext) & JSOPTION_PRIVATE_IS_NSISUPPORTS)
-        (JS_GetOptions(cx) & JSOPTION_PRIVATE_IS_NSISUPPORTS)
-        ? NS_STATIC_CAST(nsISupports*, JS_GetContextPrivate(cx))
-        : nsnull;
-    if(supports)
     {
         nsCOMPtr<nsIXPCScriptNotify> scriptNotify = 
-            do_QueryInterface(supports);
+            do_QueryInterface(NS_STATIC_CAST(nsISupports*,
+                                             JS_GetContextPrivate(mJSContext)));
         if(scriptNotify)
             scriptNotify->ScriptExecuted();
     }
+    JS_SetErrorReporter(mJSContext, mOldErrorReporter);
 }
 
 // It turns out that some errors may be not worth reporting. So, this
@@ -248,12 +253,12 @@ nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject(XPCCallContext& ccx,
 
     // OK, it looks like we'll be calling into JS code.
 
-    JSExceptionState* saved_exception = xpc_DoPreScriptEvaluated(cx);
+    AutoScriptEvaluate scriptEval(cx);
 
-    // XXX we should install an error reporter that will sent reports to
+    // XXX we should install an error reporter that will send reports to
     // the JS error console service.
+    scriptEval.StartEvaluating();
 
-    JSErrorReporter older = JS_SetErrorReporter(cx, nsnull);
     id = xpc_NewIDObject(cx, jsobj, aIID);
     if(id)
     {
@@ -263,9 +268,6 @@ nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject(XPCCallContext& ccx,
 
     if(success)
         success = JS_ValueToObject(cx, retval, &retObj);
-    JS_SetErrorReporter(cx, older);
-
-    xpc_DoPostScriptEvaluated(cx, saved_exception);
 
     return success ? retObj : nsnull;
 }
@@ -299,15 +301,12 @@ nsXPCWrappedJSClass::GetNamedPropertyAsVariant(XPCCallContext& ccx,
     jsid id;
     nsresult rv;
 
-    JSExceptionState* saved_exception = xpc_DoPreScriptEvaluated(cx);
+    AutoScriptEvaluate scriptEval(cx);
-    JSErrorReporter older = JS_SetErrorReporter(cx, nsnull);
+    scriptEval.StartEvaluating();
 
     ok = JS_ValueToId(cx, aName, &id) && 
          GetNamedPropertyAsVariantRaw(ccx, aJSObj, id, aResult, &rv);
 
-    JS_SetErrorReporter(cx, older);
-    xpc_DoPostScriptEvaluated(cx, saved_exception);
-
     return ok ? NS_OK : NS_FAILED(rv) ? rv : NS_ERROR_FAILURE;
 }
 
@@ -326,8 +325,8 @@ nsXPCWrappedJSClass::BuildPropertyEnumerator(XPCCallContext& ccx,
     int i;
 
     // Saved state must be restored, all exits through 'out'...
-    JSExceptionState* saved_exception = xpc_DoPreScriptEvaluated(cx);
+    AutoScriptEvaluate scriptEval(cx);
-    JSErrorReporter older = JS_SetErrorReporter(cx, nsnull);
+    scriptEval.StartEvaluating();
 
     idArray = JS_Enumerate(cx, aJSObj);
     if(!idArray)
@@ -378,8 +377,6 @@ out:
     NS_IF_RELEASE(enumerator);
     if(idArray)
         JS_DestroyIdArray(cx, idArray);
-    JS_SetErrorReporter(cx, older);
-    xpc_DoPostScriptEvaluated(cx, saved_exception);
 
     return retval;
 }
@@ -955,7 +952,6 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16 methodIndex,
     uint8 paramCount=0;
     nsresult retval = NS_ERROR_FAILURE;
     nsresult pending_result = NS_OK;
-    JSErrorReporter older;
     JSBool success;
     JSBool readyToDoTheCall = JS_FALSE;
     nsID  param_iid;
@@ -968,7 +964,6 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16 methodIndex,
     XPCContext* xpcc;
     JSContext* cx;
     JSObject* thisObj;
-    JSExceptionState* saved_exception = nsnull;
 
     XPCCallContext ccx(NATIVE_CALLER);
     if(ccx.IsValid())
@@ -982,6 +977,7 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16 methodIndex,
         cx = nsnull;
     }
 
+    AutoScriptEvaluate scriptEval(cx);
 #ifdef DEBUG_stats_jband
     PRIntervalTime startTime = PR_IntervalNow();
     PRIntervalTime endTime = 0;
@@ -1001,13 +997,10 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16 methodIndex,
     argc = paramCount -
             (paramCount && info->GetParam(paramCount-1).IsRetval() ? 1 : 0);
 
-    if(cx)
-        older = JS_SetErrorReporter(cx, xpcWrappedJSErrorReporter);
-
     if(!cx || !xpcc || !IsReflectable(methodIndex))
         goto pre_call_clean_up;
 
-    saved_exception = xpc_DoPreScriptEvaluated(cx);
+    scriptEval.StartEvaluating(xpcWrappedJSErrorReporter);
 
     xpcc->SetPendingResult(pending_result);
     xpcc->SetException(nsnull);
@@ -1579,12 +1572,6 @@ done:
     if(sp)
         js_FreeStack(cx, mark);
 
-    if(cx)
-    {
-        JS_SetErrorReporter(cx, older);
-        xpc_DoPostScriptEvaluated(cx, saved_exception);
-    }
-
 #ifdef DEBUG_stats_jband
     endTime = PR_IntervalNow();
     printf("%s::%s %d ( c->js ) \n", GetInterfaceName(), info->GetName(), PR_IntervalToMilliseconds(endTime-startTime));