Fixing bug 413774. Don't suspend/resume requests when making native calls into C++ in XPConnect, do the suspend/resume when pushing/popping JS contexts off of the context stack instead. r=shaver@mozilla.org, sr=brendan@mozilla.org

js/src/xpconnect/src/xpcprivate.h

@@ -2983,14 +2983,20 @@ private:
 /***************************************************************************/
 // XPCJSContextStack is not actually an xpcom object, but xpcom calls are
 // delegated to it as an implementation detail.
-struct JSContextAndFrame {
+struct XPCJSContextInfo {
-    JSContextAndFrame(JSContext* aCx) :
+    XPCJSContextInfo(JSContext* aCx) :
         cx(aCx),
-        frame(nsnull)
+        frame(nsnull),
+        requestDepth(0)
     {}
     JSContext* cx;
-    JSStackFrame* frame;  // Frame to be restored when this JSContext becomes
+
-                          // the topmost one.
+    // Frame to be restored when this JSContext becomes the topmost
+    // one.
+    JSStackFrame* frame;
+
+    // Greater than 0 if a request was suspended
+    jsrefcount requestDepth;
 };
 
 class XPCJSContextStack
@@ -3006,14 +3012,14 @@ public:
     JSBool DEBUG_StackHasJSContext(JSContext*  aJSContext);
 #endif
 
-    const nsTArray<JSContextAndFrame>* GetStack()
+    const nsTArray<XPCJSContextInfo>* GetStack()
     { return &mStack; }
 
 private:
     void SyncJSContexts();
 
 private:
-    nsAutoTArray<JSContextAndFrame, 16> mStack;
+    nsAutoTArray<XPCJSContextInfo, 16> mStack;
     JSContext*  mSafeJSContext;
 
     // If non-null, we own it; same as mSafeJSContext if SetSafeJSContext
@@ -3034,7 +3040,7 @@ public:
     NS_DECL_NSIJSCONTEXTSTACKITERATOR
 
 private:
-    const nsTArray<JSContextAndFrame> *mStack;
+    const nsTArray<XPCJSContextInfo> *mStack;
     PRUint32 mPosition;
 };
 

js/src/xpconnect/src/xpcthreadcontext.cpp

@@ -106,13 +106,18 @@ XPCJSContextStack::Pop(JSContext * *_retval)
     if(idx > 0)
     {
         --idx; // Advance to new top of the stack
-        JSContextAndFrame & e = mStack[idx];
+        XPCJSContextInfo & e = mStack[idx];
         NS_ASSERTION(!e.frame || e.cx, "Shouldn't have frame without a cx!");
         if(e.cx && e.frame)
         {
             JS_RestoreFrameChain(e.cx, e.frame);
             e.frame = nsnull;
         }
+
+        if(e.requestDepth)
+            JS_ResumeRequest(e.cx, e.requestDepth);
+
+        e.requestDepth = 0;
     }
     return NS_OK;
 }
@@ -125,9 +130,14 @@ XPCJSContextStack::Push(JSContext * cx)
         return NS_ERROR_OUT_OF_MEMORY;
     if(mStack.Length() > 1)
     {
-        JSContextAndFrame & e = mStack[mStack.Length() - 2];
+        XPCJSContextInfo & e = mStack[mStack.Length() - 2];
         if(e.cx && e.cx != cx)
+        {
             e.frame = JS_SaveFrameChain(e.cx);
+
+            if(JS_GetContextThread(e.cx))
+                e.requestDepth = JS_SuspendRequest(e.cx);
+        }
     }
     return NS_OK;
 }

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -1958,10 +1958,7 @@ XPCWrappedNative::CallMethod(XPCCallContext& ccx,
         }
 
         nsISupports* qiresult = nsnull;
-        {
+        invokeResult = callee->QueryInterface(*iid, (void**) &qiresult);
-            AutoJSSuspendRequest req(ccx);
-            invokeResult = callee->QueryInterface(*iid, (void**) &qiresult);
-        }
 
         xpcc->SetLastResult(invokeResult);
 
@@ -2337,16 +2334,9 @@ XPCWrappedNative::CallMethod(XPCCallContext& ccx,
     }
 
 
-    {
+    // do the invoke
-        // avoid deadlock in case the native method blocks somehow
+    invokeResult = NS_InvokeByIndex(callee, vtblIndex, paramCount,
-        AutoJSSuspendRequest req(ccx);  // scoped suspend of request
+                                    dispatchParams);
-
-        // do the invoke
-        invokeResult = NS_InvokeByIndex(callee, vtblIndex,
-                                        paramCount, dispatchParams);
-        // resume non-blocking JS operations now
-    }
-
 
     xpcc->SetLastResult(invokeResult);