Bug 561871 part B: don't use Windows events to enter/exit a nested event loop within an RPC stack frame, because delivery of those events isn't always associated with the correct stack frame, and also because the events can be lost if they race with an incoming RPC reply message. Instead, keep a linked list (on the stack) of RPC frames globally and per-channel, to associate requests to enter a nested event loop with the correct frame, r=jimm
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 28 Apr 2010 11:01:09 -0400
changeset 41474 c5e9ea1e9b06352642a0d773ce00efe45225b0f9
parent 41473 c055ab35a22f1f393423db9bb8bec8f3483a7448
child 41475 68d7be8a83756a08a7ee30986b66b9b64106fce7
child 41822 fa061c750ac13ea849b7be6831dc710c6c1e2574
push id13033
push userbsmedberg@mozilla.com
push dateWed, 28 Apr 2010 15:12:42 +0000
treeherdermozilla-central@c5e9ea1e9b06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs561871
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 561871 part B: don't use Windows events to enter/exit a nested event loop within an RPC stack frame, because delivery of those events isn't always associated with the correct stack frame, and also because the events can be lost if they race with an incoming RPC reply message. Instead, keep a linked list (on the stack) of RPC frames globally and per-channel, to associate requests to enter a nested event loop with the correct frame, r=jimm
dom/plugins/PPluginInstance.ipdl
dom/plugins/PluginInstanceChild.cpp
dom/plugins/PluginInstanceChild.h
dom/plugins/PluginInstanceParent.cpp
dom/plugins/PluginInstanceParent.h
dom/plugins/PluginModuleParent.h
ipc/glue/RPCChannel.cpp
ipc/glue/RPCChannel.h
ipc/glue/SyncChannel.cpp
ipc/glue/SyncChannel.h
ipc/glue/WindowsMessageLoop.cpp
--- a/dom/plugins/PPluginInstance.ipdl
+++ b/dom/plugins/PPluginInstance.ipdl
@@ -172,16 +172,18 @@ child:
 parent:
   /* NPN_NewStream */
   rpc PPluginStream(nsCString mimeType,
                     nsCString target)
     returns (NPError result);
 
 parent:
   rpc PluginGotFocus();
-  sync SetNestedEventState(bool aState);
+
+  async ProcessNativeEventsInRPCCall();
+
 child:
   rpc SetPluginFocus();
   rpc UpdateWindow();
 };
 
 } // namespace plugins
 } // namespace mozilla
--- a/dom/plugins/PluginInstanceChild.cpp
+++ b/dom/plugins/PluginInstanceChild.cpp
@@ -98,17 +98,16 @@ PluginInstanceChild::PluginInstanceChild
     , mCachedWindowActor(nsnull)
     , mCachedElementActor(nsnull)
 #if defined(OS_WIN)
     , mPluginWindowHWND(0)
     , mPluginWndProc(0)
     , mPluginParentHWND(0)
     , mNestedEventHook(0)
     , mNestedEventLevelDepth(0)
-    , mNestedEventState(false)
     , mCachedWinlessPluginHWND(0)
     , mWinlessPopupSurrogateHWND(0)
     , mWinlessThrottleOldWndProc(0)
 #endif // OS_WIN
     , mAsyncCallMutex("PluginInstanceChild::mAsyncCallMutex")
 #if defined(OS_MACOSX)  
     , mShColorSpace(nsnull)
     , mShContext(nsnull)
@@ -1093,33 +1092,33 @@ PluginInstanceChild::PluginWindowProc(HW
     return res;
 }
 
 /* winless modal ui loop logic */
 
 // gTempChildPointer is only in use from the time we enter handle event, to the
 // point where ui might be created by that call. If ui isn't created, there's
 // no issue. If ui is created, the parent can't start processing messages in
-// spin loop until InternalCallSetNestedEventState is set, at which point,
+// spin loop until SendSetNestedEventState, at which point,
 // gTempChildPointer is no longer needed.
 static PluginInstanceChild* gTempChildPointer;
 
 LRESULT CALLBACK
 PluginInstanceChild::NestedInputEventHook(int nCode,
                                           WPARAM wParam,
                                           LPARAM lParam)
 {
     if (!gTempChildPointer) {
         return CallNextHookEx(NULL, nCode, wParam, lParam);
     }
 
     if (nCode >= 0) {
         NS_ASSERTION(gTempChildPointer, "Never should be null here!");
         gTempChildPointer->ResetNestedEventHook();
-        gTempChildPointer->InternalCallSetNestedEventState(true);
+        gTempChildPointer->SendProcessNativeEventsInRPCCall();
 
         gTempChildPointer = NULL;
     }
     return CallNextHookEx(NULL, nCode, wParam, lParam);
 }
 
 void
 PluginInstanceChild::SetNestedInputEventHook()
@@ -1143,28 +1142,16 @@ void
 PluginInstanceChild::ResetNestedEventHook()
 {
     PLUGIN_LOG_DEBUG(("%s", FULLFUNCTION));
     if (mNestedEventHook)
         UnhookWindowsHookEx(mNestedEventHook);
     mNestedEventHook = NULL;
 }
 
-void
-PluginInstanceChild::InternalCallSetNestedEventState(bool aState)
-{
-    if (aState != mNestedEventState) {
-        PLUGIN_LOG_DEBUG(
-            ("PluginInstanceChild::InternalCallSetNestedEventState(%i)",
-            (int)aState));
-        mNestedEventState = aState;
-        SendSetNestedEventState(mNestedEventState);
-    }
-}
-
 /* windowless track popup menu helpers */
 
 BOOL
 WINAPI
 PluginInstanceChild::TrackPopupHookProc(HMENU hMenu,
                                         UINT uFlags,
                                         int x,
                                         int y,
@@ -1370,17 +1357,16 @@ PluginInstanceChild::WinlessHandleEvent(
     sWinlessPopupSurrogateHWND = NULL;
 
     mNestedEventLevelDepth--;
     PLUGIN_LOG_DEBUG(("WinlessHandleEvent end depth: %i", mNestedEventLevelDepth));
 
     NS_ASSERTION(!(mNestedEventLevelDepth < 0), "mNestedEventLevelDepth < 0?");
     if (mNestedEventLevelDepth <= 0) {
         ResetNestedEventHook();
-        InternalCallSetNestedEventState(false);
     }
     return handled;
 }
 
 /* windowless drawing helpers */
 
 bool
 PluginInstanceChild::SharedSurfaceSetWindow(const NPRemoteWindow& aWindow)
--- a/dom/plugins/PluginInstanceChild.h
+++ b/dom/plugins/PluginInstanceChild.h
@@ -235,17 +235,16 @@ private:
     int16_t WinlessHandleEvent(NPEvent& event);
     void SetNestedInputEventHook();
     void ResetNestedEventHook();
     void SetNestedInputPumpHook();
     void ResetPumpHooks();
     void CreateWinlessPopupSurrogate();
     void DestroyWinlessPopupSurrogate();
     void InitPopupMenuHook();
-    void InternalCallSetNestedEventState(bool aState);
     void SetupFlashMsgThrottle();
     void FlashThrottleMessage(HWND, UINT, WPARAM, LPARAM, bool);
     static LRESULT CALLBACK DummyWindowProc(HWND hWnd,
                                             UINT message,
                                             WPARAM wParam,
                                             LPARAM lParam);
     static LRESULT CALLBACK PluginWindowProc(HWND hWnd,
                                              UINT message,
@@ -321,17 +320,16 @@ private:
 #if defined(MOZ_X11) && defined(XP_UNIX) && !defined(XP_MACOSX)
     NPSetWindowCallbackStruct mWsInfo;
 #elif defined(OS_WIN)
     HWND mPluginWindowHWND;
     WNDPROC mPluginWndProc;
     HWND mPluginParentHWND;
     HHOOK mNestedEventHook;
     int mNestedEventLevelDepth;
-    bool mNestedEventState;
     HWND mCachedWinlessPluginHWND;
     HWND mWinlessPopupSurrogateHWND;
     nsIntPoint mPluginSize;
     nsIntPoint mPluginOffset;
     WNDPROC mWinlessThrottleOldWndProc;
 #endif
 
     friend class ChildAsyncCall;
--- a/dom/plugins/PluginInstanceParent.cpp
+++ b/dom/plugins/PluginInstanceParent.cpp
@@ -148,44 +148,30 @@ void
 PluginInstanceParent::ActorDestroy(ActorDestroyReason why)
 {
 #if defined(OS_WIN)
     if (why == AbnormalShutdown) {
         // If the plugin process crashes, this is the only
         // chance we get to destroy resources.
         SharedSurfaceRelease();
         UnsubclassPluginWindow();
-        // If we crashed in a modal loop in the child, reset
-        // the rpc event spin loop state.
-        if (mNestedEventState) {
-            mNestedEventState = false;
-            PostThreadMessage(GetCurrentThreadId(),
-                              gOOPPStopNativeLoopEvent,
-                              0, 0);
-        }
     }
 #endif
 }
 
 NPError
 PluginInstanceParent::Destroy()
 {
     NPError retval;
     if (!CallNPP_Destroy(&retval))
         retval = NPERR_GENERIC_ERROR;
 
 #if defined(OS_WIN)
     SharedSurfaceRelease();
     UnsubclassPluginWindow();
-    if (mNestedEventState) {
-        mNestedEventState = false;
-        PostThreadMessage(GetCurrentThreadId(),
-                          gOOPPStopNativeLoopEvent,
-                          0, 0);
-    }
 #endif
 
     return retval;
 }
 
 PBrowserStreamParent*
 PluginInstanceParent::AllocPBrowserStream(const nsCString& url,
                                           const uint32_t& length,
@@ -1323,23 +1309,22 @@ PluginInstanceParent::AnswerPluginGotFoc
     return true;
 #else
     NS_NOTREACHED("PluginInstanceParent::AnswerPluginGotFocus not implemented!");
     return false;
 #endif
 }
 
 bool
-PluginInstanceParent::RecvSetNestedEventState(const bool& aState)
+PluginInstanceParent::RecvProcessNativeEventsInRPCCall()
 {
-    PLUGIN_LOG_DEBUG(("%s state=%i", FULLFUNCTION, (int)aState));
+    PLUGIN_LOG_DEBUG(("%s", FULLFUNCTION));
 #if defined(OS_WIN)
-    PostThreadMessage(GetCurrentThreadId(), aState ?
-        gOOPPSpinNativeLoopEvent : gOOPPStopNativeLoopEvent, 0, 0);
-    mNestedEventState = aState;
+    static_cast<PluginModuleParent*>(Manager())
+        ->ProcessNativeEventsInRPCCall();
     return true;
 #else
     NS_NOTREACHED(
         "PluginInstanceParent::AnswerSetNestedEventState not implemented!");
     return false;
 #endif
 }
 
--- a/dom/plugins/PluginInstanceParent.h
+++ b/dom/plugins/PluginInstanceParent.h
@@ -241,18 +241,18 @@ public:
     GetNPP()
     {
       return mNPP;
     }
 
     virtual bool
     AnswerPluginGotFocus();
 
-    virtual bool
-    RecvSetNestedEventState(const bool& aState);
+    NS_OVERRIDE virtual bool
+    RecvProcessNativeEventsInRPCCall();
 
 #if defined(OS_MACOSX)
     void Invalidate();
 #endif // definied(OS_MACOSX)
 
 private:
     // Quirks mode support for various plugin mime types
     enum PluginQuirks {
--- a/dom/plugins/PluginModuleParent.h
+++ b/dom/plugins/PluginModuleParent.h
@@ -121,16 +121,17 @@ public:
      * and may or may not be very expensive.
      */
     static PluginLibrary* LoadModule(const char* aFilePath);
 
     const NPNetscapeFuncs* GetNetscapeFuncs() {
         return mNPNIface;
     }
 
+    PluginProcessParent* Process() const { return mSubprocess; }
     base::ProcessHandle ChildProcessHandle() { return mSubprocess->GetChildProcessHandle(); }
 
     bool OkToCleanup() const {
         return !IsOnCxxStack();
     }
 
     PPluginIdentifierParent*
     GetIdentifierForNPIdentifier(NPIdentifier aIdentifier);
--- a/ipc/glue/RPCChannel.cpp
+++ b/ipc/glue/RPCChannel.cpp
@@ -115,22 +115,16 @@ RPCChannel::~RPCChannel()
 void
 RPCChannel::Clear()
 {
     mDequeueOneTask->Cancel();
 
     AsyncChannel::Clear();
 }
 
-#ifdef OS_WIN
-// static
-int RPCChannel::sInnerEventLoopDepth = 0;
-int RPCChannel::sModalEventCount = 0;
-#endif
-
 bool
 RPCChannel::EventOccurred() const
 {
     AssertWorkerThread();
     mMutex.AssertCurrentThreadOwns();
     RPC_ASSERT(StackDepth() > 0, "not in wait loop");
 
     return (!Connected() ||
@@ -160,16 +154,20 @@ bool
 RPCChannel::Call(Message* msg, Message* reply)
 {
     AssertWorkerThread();
     mMutex.AssertNotCurrentThreadOwns();
     RPC_ASSERT(!ProcessingSyncMessage(),
                "violation of sync handler invariant");
     RPC_ASSERT(msg->is_rpc(), "can only Call() RPC messages here");
 
+#ifdef OS_WIN
+    SyncStackFrame frame(this, true);
+#endif
+
     Message copy = *msg;
     CxxStackFrame f(*this, OUT_MESSAGE, &copy);
 
     MutexAutoLock lock(mMutex);
 
     if (!Connected()) {
         ReportConnectionError("RPCChannel");
         return false;
--- a/ipc/glue/RPCChannel.h
+++ b/ipc/glue/RPCChannel.h
@@ -152,45 +152,21 @@ public:
     // Override the SyncChannel handler so we can dispatch RPC
     // messages.  Called on the IO thread only.
     NS_OVERRIDE
     virtual void OnMessageReceived(const Message& msg);
     NS_OVERRIDE
     virtual void OnChannelError();
 
 #ifdef OS_WIN
-    static bool IsSpinLoopActive() {
-        return (sModalEventCount > 0);
-    }
+    void ProcessNativeEventsInRPCCall();
+
 protected:
     bool WaitForNotify();
     void SpinInternalEventLoop();
-    static bool WaitNeedsSpinLoop() {
-        return (IsSpinLoopActive() && 
-                (sModalEventCount > sInnerEventLoopDepth));
-    }
-    static void EnterSpinLoop() {
-        sInnerEventLoopDepth++;
-    }
-    static void ExitSpinLoop() {
-        sInnerEventLoopDepth--;
-        NS_ASSERTION(sInnerEventLoopDepth >= 0,
-            "sInnerEventLoopDepth dropped below zero!");
-    }
-    static void IncModalLoopCnt() {
-        sModalEventCount++;
-    }
-    static void DecModalLoopCnt() {
-        sModalEventCount--;
-        NS_ASSERTION(sModalEventCount >= 0,
-            "sModalEventCount dropped below zero!");
-    }
-
-    static int sInnerEventLoopDepth;
-    static int sModalEventCount;
 #endif
 
   private:
     // Called on worker thread only
 
     RPCListener* Listener() const {
         return static_cast<RPCListener*>(mListener);
     }
--- a/ipc/glue/SyncChannel.cpp
+++ b/ipc/glue/SyncChannel.cpp
@@ -52,21 +52,24 @@ struct RunnableMethodTraits<mozilla::ipc
 };
 
 namespace mozilla {
 namespace ipc {
 
 const int32 SyncChannel::kNoTimeout = PR_INT32_MIN;
 
 SyncChannel::SyncChannel(SyncListener* aListener)
-  : AsyncChannel(aListener),
-    mPendingReply(0),
-    mProcessingSyncMessage(false),
-    mNextSeqno(0),
-    mTimeoutMs(kNoTimeout)
+  : AsyncChannel(aListener)
+  , mPendingReply(0)
+  , mProcessingSyncMessage(false)
+  , mNextSeqno(0)
+  , mTimeoutMs(kNoTimeout)
+#ifdef OS_WIN
+  , mTopFrame(NULL)
+#endif
 {
     MOZ_COUNT_CTOR(SyncChannel);
 #ifdef OS_WIN
     mEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
     NS_ASSERTION(mEvent, "CreateEvent failed! Nothing is going to work!");
 #endif
 }
 
@@ -95,16 +98,20 @@ bool
 SyncChannel::Send(Message* msg, Message* reply)
 {
     AssertWorkerThread();
     mMutex.AssertNotCurrentThreadOwns();
     NS_ABORT_IF_FALSE(!ProcessingSyncMessage(),
                       "violation of sync handler invariant");
     NS_ABORT_IF_FALSE(msg->is_sync(), "can only Send() sync messages here");
 
+#ifdef OS_WIN
+    SyncStackFrame frame(this, false);
+#endif
+
     msg->set_seqno(NextSeqno());
 
     MutexAutoLock lock(mMutex);
 
     if (!Connected()) {
         ReportConnectionError("SyncChannel");
         return false;
     }
--- a/ipc/glue/SyncChannel.h
+++ b/ipc/glue/SyncChannel.h
@@ -90,16 +90,52 @@ public:
 
     static bool IsPumpingMessages() {
         return sIsPumpingMessages;
     }
     static void SetIsPumpingMessages(bool aIsPumping) {
         sIsPumpingMessages = aIsPumping;
     }
 
+#ifdef OS_WIN
+    struct NS_STACK_CLASS SyncStackFrame
+    {
+        SyncStackFrame(SyncChannel* channel, bool rpc);
+        ~SyncStackFrame();
+
+        bool mRPC;
+        bool mSpinNestedEvents;
+        SyncChannel* mChannel;
+
+        /* the previous stack frame for this channel */
+        SyncStackFrame* mPrev;
+
+        /* the previous stack frame on any channel */
+        SyncStackFrame* mStaticPrev;
+    };
+    friend struct SyncChannel::SyncStackFrame;
+
+    static bool IsSpinLoopActive() {
+        for (SyncStackFrame* frame = sStaticTopFrame;
+             frame;
+             frame = frame->mPrev) {
+            if (frame->mSpinNestedEvents)
+                return true;
+        }
+        return false;
+    }
+
+protected:
+    /* the deepest sync stack frame for this channel */
+    SyncStackFrame* mTopFrame;
+
+    /* the deepest sync stack frame on any channel */
+    static SyncStackFrame* sStaticTopFrame;
+#endif // OS_WIN
+
 protected:
     // Executed on the worker thread
     bool ProcessingSyncMessage() const {
         return mProcessingSyncMessage;
     }
 
     void OnDispatchMessage(const Message& aMsg);
 
--- a/ipc/glue/WindowsMessageLoop.cpp
+++ b/ipc/glue/WindowsMessageLoop.cpp
@@ -98,22 +98,16 @@ using namespace mozilla::ipc::windows;
  * modal UI related api calls block an RPC in-call in the child. To prevent
  * windows from freezing, and to allow concurrent processing of critical
  * events (such as painting), we spin a native event dispatch loop while
  * these in-calls are blocked.
  */
 
 namespace {
 
-UINT gOOPPSpinNativeLoopEvent =
-    RegisterWindowMessage(L"SyncChannel Spin Inner Loop Message");
-
-UINT gOOPPStopNativeLoopEvent =
-    RegisterWindowMessage(L"SyncChannel Stop Inner Loop Message");
-
 const wchar_t kOldWndProcProp[] = L"MozillaIPCOldWndProc";
 
 // This isn't defined before Windows XP.
 enum { WM_XP_THEMECHANGED = 0x031A };
 
 PRUnichar gAppMessageWindowName[256] = { 0 };
 PRInt32 gAppMessageWindowNameLength = 0;
 
@@ -568,140 +562,151 @@ TimeoutHasExpired(const TimeoutData& aDa
     // Overflow
     return now < aData.startTicks && now >= aData.targetTicks;
   }
   return now >= aData.targetTicks;
 }
 
 } // anonymous namespace
 
+RPCChannel::SyncStackFrame::SyncStackFrame(SyncChannel* channel, bool rpc)
+  : mRPC(rpc)
+  , mSpinNestedEvents(false)
+  , mChannel(channel)
+  , mPrev(mChannel->mTopFrame)
+  , mStaticPrev(sStaticTopFrame)
+{
+  mChannel->mTopFrame = this;
+  sStaticTopFrame = this;
+
+  if (!mStaticPrev) {
+    NS_ASSERTION(!gNeuteredWindows, "Should only set this once!");
+    gNeuteredWindows = new nsAutoTArray<HWND, 20>();
+    NS_ASSERTION(gNeuteredWindows, "Out of memory!");
+  }
+}
+
+RPCChannel::SyncStackFrame::~SyncStackFrame()
+{
+  NS_ASSERTION(this == mChannel->mTopFrame,
+               "Mismatched RPC stack frames");
+  NS_ASSERTION(this == sStaticTopFrame,
+               "Mismatched static RPC stack frames");
+
+  mChannel->mTopFrame = mPrev;
+  sStaticTopFrame = mStaticPrev;
+
+  if (!mStaticPrev) {
+    NS_ASSERTION(gNeuteredWindows, "Bad pointer!");
+    delete gNeuteredWindows;
+    gNeuteredWindows = NULL;
+  }
+}
+
+SyncChannel::SyncStackFrame* SyncChannel::sStaticTopFrame;
+
+void
+RPCChannel::ProcessNativeEventsInRPCCall()
+{
+  if (!mTopFrame) {
+    NS_ERROR("Child logic error: no RPC frame");
+    return;
+  }
+
+  mTopFrame->mSpinNestedEvents = true;
+}
+
 // Spin loop is called in place of WaitForNotify when modal ui is being shown
 // in a child. There are some intricacies in using it however. Spin loop is
-// enabled / disabled through a set of thread messages sent from
-// PluginInstanceParent (gOOPPStartNativeLoopEvent/gOOPPStopNativeLoopEvent).
-// Each time we receive a start/stop spin event, a counter is adjusted to track
-// the number of modal loops children drop into. We can receive multiple
-// matching starts and stops in cases where multiple plugins drop into modal ui
-// loops. (For example, a message dialog in one browser window, a context menu
-// in another.) When the final count drops to zero, we exit out of spin loop
-// and start using WaitForNotify again. However, we don't replace WaitForNotify
-// completely when spin loop is active - we only call SpinInternalEventLoop
-// at the base of the stack. To accomplish this, we use a second counter to
-// limit the number of calls to SpinInternalEventLoop() equal to the number
-// of modal loops entered.
+// enabled for a particular RPC frame by the client calling
+// RPCChannel::ProcessNativeEventsInRPCCall().
+// This call can be nested for multiple RPC frames in a single plugin or 
+// multiple unrelated plugins.
 void
 RPCChannel::SpinInternalEventLoop()
 {
-  EnterSpinLoop();
+  NS_ASSERTION(mTopFrame && mTopFrame->mSpinNestedEvents,
+               "Spinning incorrectly");
 
   // Nested windows event loop we trigger when the child enters into modal
   // event loops.
   
   // Note, when we return, we always reset the notify worker event. So there's
   // no need to reset it on return here.
 
   do {
     MSG msg = { 0 };
 
     // Don't get wrapped up in here if the child connection dies.
     {
       MutexAutoLock lock(mMutex);
       if (!Connected()) {
-        ExitSpinLoop();
         return;
       }
     }
 
-    if (!RPCChannel::IsSpinLoopActive()) {
-      ExitSpinLoop();
-      return;
-    }
-
-    // If a modal loop in the child has exited, disable spin loop and exit.
-    if (PeekMessageW(&msg, (HWND)-1, gOOPPStopNativeLoopEvent,
-                     gOOPPStopNativeLoopEvent, PM_REMOVE)) {
-      DecModalLoopCnt();
-      ExitSpinLoop();
-      return;
-    }
-
     // Retrieve window or thread messages
     if (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) {
-      if (msg.message == gOOPPStopNativeLoopEvent) {
-        DecModalLoopCnt();
-        ExitSpinLoop();
-        return;
-      }
-      else if (msg.message == gOOPPSpinNativeLoopEvent) {
-        // Keep the spin loop counter accurate, multiple plugins can show ui.
-        IncModalLoopCnt();
-        continue;
-      }
-
       // The child UI should have been destroyed before the app is closed, in
       // which case, we should never get this here.
       if (msg.message == WM_QUIT) {
           NS_ERROR("WM_QUIT received in SpinInternalEventLoop!");
       } else {
           TranslateMessage(&msg);
           DispatchMessageW(&msg);
-          ExitSpinLoop();
           return;
       }
     }
 
     // Note, give dispatching windows events priority over checking if
     // mEvent is signaled, otherwise heavy ipc traffic can cause jittery
     // playback of video. We'll exit out on each disaptch above, so ipc
     // won't get starved.
 
     // Wait for UI events or a signal from the io thread.
     DWORD result = MsgWaitForMultipleObjects(1, &mEvent, FALSE, INFINITE,
                                              QS_ALLINPUT);
     if (result == WAIT_OBJECT_0) {
       // Our NotifyWorkerThread event was signaled
-      ExitSpinLoop();
       return;
     }
   } while (true);
 }
 
 bool
 SyncChannel::WaitForNotify()
 {
   mMutex.AssertCurrentThreadOwns();
 
   // Initialize global objects used in deferred messaging.
   Init();
 
+  NS_ASSERTION(mTopFrame && !mTopFrame->mRPC,
+               "Top frame is not a sync frame!");
+
   MutexAutoUnlock unlock(mMutex);
 
   bool retval = true;
 
-  if (++gEventLoopDepth == 1) {
-    NS_ASSERTION(!gNeuteredWindows, "Should only set this once!");
-    gNeuteredWindows = new nsAutoTArray<HWND, 20>();
-    NS_ASSERTION(gNeuteredWindows, "Out of memory!");
-  }
-
   UINT_PTR timerId = NULL;
   TimeoutData timeoutData = { 0 };
 
   if (mTimeoutMs != kNoTimeout) {
     InitTimeoutData(&timeoutData, mTimeoutMs);
 
     // We only do this to ensure that we won't get stuck in
     // MsgWaitForMultipleObjects below.
     timerId = SetTimer(NULL, 0, mTimeoutMs, NULL);
     NS_ASSERTION(timerId, "SetTimer failed!");
   }
 
   // Setup deferred processing of native events while we wait for a response.
   NS_ASSERTION(!SyncChannel::IsPumpingMessages(),
                "Shouldn't be pumping already!");
+
   SyncChannel::SetIsPumpingMessages(true);
   HHOOK windowHook = SetWindowsHookEx(WH_CALLWNDPROC, CallWindowProcedureHook,
                                       NULL, gUIThreadId);
   NS_ASSERTION(windowHook, "Failed to set hook!");
 
   {
     while (1) {
       MSG msg = { 0 };
@@ -768,22 +773,16 @@ SyncChannel::WaitForNotify()
 
   // Unhook the neutered window procedure hook.
   UnhookWindowsHookEx(windowHook);
 
   // Unhook any neutered windows procedures so messages can be delivered
   // normally.
   UnhookNeuteredWindows();
 
-  if (--gEventLoopDepth == 0) {
-    NS_ASSERTION(gNeuteredWindows, "Bad pointer!");
-    delete gNeuteredWindows;
-    gNeuteredWindows = NULL;
-  }
-
   // Before returning we need to set a hook to run any deferred messages that
   // we received during the IPC call. The hook will unset itself as soon as
   // someone else calls GetMessage, PeekMessage, or runs code that generates
   // a "nonqueued" message.
   ScheduleDeferredMessageRun();
 
   if (timerId) {
     KillTimer(NULL, timerId);
@@ -802,160 +801,136 @@ RPCChannel::WaitForNotify()
   if (!StackDepth() && !mBlockedOnParent) {
     // There is currently no way to recover from this condition.
     NS_RUNTIMEABORT("StackDepth() is 0 in call to RPCChannel::WaitForNotify!");
   }
 
   // Initialize global objects used in deferred messaging.
   Init();
 
+  NS_ASSERTION(mTopFrame && mTopFrame->mRPC,
+               "Top frame is not a sync frame!");
+
   MutexAutoUnlock unlock(mMutex);
 
   bool retval = true;
 
-  if (WaitNeedsSpinLoop()) {
-    SpinInternalEventLoop();
-    ResetEvent(mEvent);
-    return true;
-  }
-
-  if (++gEventLoopDepth == 1) {
-    NS_ASSERTION(!gNeuteredWindows, "Should only set this once!");
-    gNeuteredWindows = new nsAutoTArray<HWND, 20>();
-    NS_ASSERTION(gNeuteredWindows, "Out of memory!");
-  }
-
   UINT_PTR timerId = NULL;
   TimeoutData timeoutData = { 0 };
 
-  if (mTimeoutMs != kNoTimeout) {
-    InitTimeoutData(&timeoutData, mTimeoutMs);
-
-    // We only do this to ensure that we won't get stuck in
-    // MsgWaitForMultipleObjects below.
-    timerId = SetTimer(NULL, 0, mTimeoutMs, NULL);
-    NS_ASSERTION(timerId, "SetTimer failed!");
-  }
-
-  // Setup deferred processing of native events while we wait for a response.
-  NS_ASSERTION(!SyncChannel::IsPumpingMessages(),
-               "Shouldn't be pumping already!");
-  SyncChannel::SetIsPumpingMessages(true);
-  HHOOK windowHook = SetWindowsHookEx(WH_CALLWNDPROC, CallWindowProcedureHook,
-                                      NULL, gUIThreadId);
-  NS_ASSERTION(windowHook, "Failed to set hook!");
-
-  {
-    while (1) {
-      MSG msg = { 0 };
-
-      // Don't get wrapped up in here if the child connection dies.
-      {
-        MutexAutoLock lock(mMutex);
-        if (!Connected()) {
-          break;
-        }
-      }
+  // windowHook is used as a flag variable for the loop below: if it is set
+  // and we start to spin a nested event loop, we need to clear the hook and
+  // process deferred/pending messages.
+  // If windowHook is NULL, SyncChannel::IsPumpingMessages should be false.
+  HHOOK windowHook = NULL;
 
-      DWORD result = MsgWaitForMultipleObjects(1, &mEvent, FALSE, INFINITE,
-                                               QS_ALLINPUT);
-      if (result == WAIT_OBJECT_0) {
-        // Our NotifyWorkerThread event was signaled
-        ResetEvent(mEvent);
-        break;
-      } else
-      if (result != (WAIT_OBJECT_0 + 1)) {
-        NS_ERROR("Wait failed!");
-        break;
-      }
+  while (1) {
+    NS_ASSERTION((!!windowHook) == SyncChannel::IsPumpingMessages(),
+                 "windowHook out of sync with reality");
 
-      if (TimeoutHasExpired(timeoutData)) {
-        // A timeout was specified and we've passed it. Break out.
-        retval = false;
-        break;
-      }
-
-      // See SyncChannel's WaitForNotify for details.
-      bool haveSentMessagesPending =
-        (HIWORD(GetQueueStatus(QS_SENDMESSAGE)) & QS_SENDMESSAGE) != 0;
-
-      // This message is received from PluginInstanceParent when the child is
-      // about to drop into a modal event loop. Deferred "nonqueued" events and
-      // backed up queued events must be delivered before this happens, and
-      // normal event processing must resume, otherwise UI lockups can result.
-      // Unhook deferred processing, purge deferred events, and drop down into
-      // our local event dispatch loop.
-      if (PeekMessageW(&msg, (HWND)-1, gOOPPSpinNativeLoopEvent,
-                       gOOPPSpinNativeLoopEvent, PM_REMOVE)) {
-        // Unhook the neutered window procedure hook.
+    if (mTopFrame->mSpinNestedEvents) {
+      if (windowHook) {
         UnhookWindowsHookEx(windowHook);
         windowHook = NULL;
 
         if (timerId) {
           KillTimer(NULL, timerId);
           timerId = NULL;
         }
 
-        // Used by widget to assert on incoming native events.
+        // Used by widget to assert on incoming native events
         SyncChannel::SetIsPumpingMessages(false);
 
-        // Unhook any neutered windows procedures so messages can be delivered
+        // Unhook any neutered windows procedures so messages can be delievered
         // normally.
         UnhookNeuteredWindows();
 
-        // Send all deferred "nonqueued" messages to the intended receiver.
+        // Send all deferred "nonqueued" message to the intended receiver.
         // We're dropping into SpinInternalEventLoop so we should be fairly
-        // certain these will get deliverd soon.
+        // certain these will get delivered soohn.
         ScheduleDeferredMessageRun();
-        
-        // Spin the internal dispatch message loop during calls to WaitForNotify
-        // until the child process tells us the modal loop has closed.
-        IncModalLoopCnt();
-        SpinInternalEventLoop();
-        ResetEvent(mEvent);
-        return true;
       }
+      SpinInternalEventLoop();
+      ResetEvent(mEvent);
+      return true;
+    }
 
-      // If a modal loop in the child has exited, we want to disable the spin
-      // loop.
-      if (PeekMessageW(&msg, (HWND)-1, gOOPPStopNativeLoopEvent,
-                       gOOPPStopNativeLoopEvent, PM_REMOVE)) {
-        DecModalLoopCnt();
+    if (!windowHook) {
+      SyncChannel::SetIsPumpingMessages(true);
+      windowHook = SetWindowsHookEx(WH_CALLWNDPROC, CallWindowProcedureHook,
+                                    NULL, gUIThreadId);
+      NS_ASSERTION(windowHook, "Failed to set hook!");
+
+      NS_ASSERTION(!timerId, "Timer already initialized?");
+
+      if (mTimeoutMs != kNoTimeout) {
+        InitTimeoutData(&timeoutData, mTimeoutMs);
+        timerId = SetTimer(NULL, 0, mTimeoutMs, NULL);
+        NS_ASSERTION(timerId, "SetTimer failed!");
+      }
+    }
+
+    MSG msg = { 0 };
+
+    // Don't get wrapped up in here if the child connection dies.
+    {
+      MutexAutoLock lock(mMutex);
+      if (!Connected()) {
         break;
       }
+    }
 
-      if (!PeekMessageW(&msg, NULL, 0, 0, PM_NOREMOVE) &&
-          !haveSentMessagesPending) {
-        // Message was for child, we should wait a bit.
-        SwitchToThread();
-      }
+    DWORD result = MsgWaitForMultipleObjects(1, &mEvent, FALSE, INFINITE,
+                                             QS_ALLINPUT);
+    if (result == WAIT_OBJECT_0) {
+      // Our NotifyWorkerThread event was signaled
+      ResetEvent(mEvent);
+      break;
+    } else
+    if (result != (WAIT_OBJECT_0 + 1)) {
+      NS_ERROR("Wait failed!");
+      break;
+    }
+
+    if (TimeoutHasExpired(timeoutData)) {
+      // A timeout was specified and we've passed it. Break out.
+      retval = false;
+      break;
+    }
+
+    // See SyncChannel's WaitForNotify for details.
+    bool haveSentMessagesPending =
+      (HIWORD(GetQueueStatus(QS_SENDMESSAGE)) & QS_SENDMESSAGE) != 0;
+
+    // PeekMessage markes the messages as "old" so that they don't wake up
+    // MsgWaitForMultipleObjects every time.
+    if (!PeekMessageW(&msg, NULL, 0, 0, PM_NOREMOVE) &&
+        !haveSentMessagesPending) {
+      // Message was for child, we should wait a bit.
+      SwitchToThread();
     }
   }
 
-  // Unhook the neutered window procedure hook.
-  UnhookWindowsHookEx(windowHook);
+  if (windowHook) {
+    // Unhook the neutered window procedure hook.
+    UnhookWindowsHookEx(windowHook);
 
-  // Unhook any neutered windows procedures so messages can be delivered
-  // normally.
-  UnhookNeuteredWindows();
+    // Unhook any neutered windows procedures so messages can be delivered
+    // normally.
+    UnhookNeuteredWindows();
 
-  if (--gEventLoopDepth == 0) {
-    NS_ASSERTION(gNeuteredWindows, "Bad pointer!");
-    delete gNeuteredWindows;
-    gNeuteredWindows = NULL;
-  }
+    // Before returning we need to set a hook to run any deferred messages that
+    // we received during the IPC call. The hook will unset itself as soon as
+    // someone else calls GetMessage, PeekMessage, or runs code that generates
+    // a "nonqueued" message.
+    ScheduleDeferredMessageRun();
 
-  // Before returning we need to set a hook to run any deferred messages that
-  // we received during the IPC call. The hook will unset itself as soon as
-  // someone else calls GetMessage, PeekMessage, or runs code that generates
-  // a "nonqueued" message.
-  ScheduleDeferredMessageRun();
-
-  if (timerId) {
-    KillTimer(NULL, timerId);
+    if (timerId) {
+      KillTimer(NULL, timerId);
+    }
   }
 
   SyncChannel::SetIsPumpingMessages(false);
 
   return retval;
 }
 
 void