Bug 545338 - RPCChannel should use events rather than thread messages for NotifyWokerThread. r=bent.
authorJim Mathies <jmathies@mozilla.com>
Thu, 25 Mar 2010 16:53:10 -0500
changeset 39845 e448bbd5f8a9
parent 39844 6350335e13f2
child 39846 c6cfe544d4b9
push id12435
push userjmathies@mozilla.com
push dateThu, 25 Mar 2010 21:53:42 +0000
treeherdermozilla-central@e448bbd5f8a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs545338
milestone1.9.3a4pre
Bug 545338 - RPCChannel should use events rather than thread messages for NotifyWokerThread. r=bent.
ipc/glue/RPCChannel.h
ipc/glue/SyncChannel.cpp
ipc/glue/SyncChannel.h
ipc/glue/WindowsMessageLoop.cpp
--- a/ipc/glue/RPCChannel.h
+++ b/ipc/glue/RPCChannel.h
@@ -157,17 +157,17 @@ public:
     virtual void OnChannelError();
 
 #ifdef OS_WIN
     static bool IsSpinLoopActive() {
         return (sModalEventCount > 0);
     }
 protected:
     bool WaitForNotify();
-    bool SpinInternalEventLoop();
+    void SpinInternalEventLoop();
     static bool WaitNeedsSpinLoop() {
         return (IsSpinLoopActive() && 
                 (sModalEventCount > sInnerEventLoopDepth));
     }
     static void EnterSpinLoop() {
         sInnerEventLoopDepth++;
     }
     static void ExitSpinLoop() {
--- a/ipc/glue/SyncChannel.cpp
+++ b/ipc/glue/SyncChannel.cpp
@@ -58,22 +58,29 @@ const int32 SyncChannel::kNoTimeout = PR
 
 SyncChannel::SyncChannel(SyncListener* aListener)
   : AsyncChannel(aListener),
     mPendingReply(0),
     mProcessingSyncMessage(false),
     mNextSeqno(0),
     mTimeoutMs(kNoTimeout)
 {
-  MOZ_COUNT_CTOR(SyncChannel);
+    MOZ_COUNT_CTOR(SyncChannel);
+#ifdef OS_WIN
+    mEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
+    NS_ASSERTION(mEvent, "CreateEvent failed! Nothing is going to work!");
+#endif
 }
 
 SyncChannel::~SyncChannel()
 {
     MOZ_COUNT_DTOR(SyncChannel);
+#ifdef OS_WIN
+    CloseHandle(mEvent);
+#endif
 }
 
 // static
 bool SyncChannel::sIsPumpingMessages = false;
 
 bool
 SyncChannel::EventOccurred()
 {
--- a/ipc/glue/SyncChannel.h
+++ b/ipc/glue/SyncChannel.h
@@ -149,16 +149,20 @@ protected:
     // This is only accessed from the worker thread; seqno's are
     // completely opaque to the IO thread.
     int32 mNextSeqno;
 
     static bool sIsPumpingMessages;
 
     int32 mTimeoutMs;
 
+#ifdef OS_WIN
+    HANDLE mEvent;
+#endif
+
 private:
     bool EventOccurred();
 };
 
 
 } // namespace ipc
 } // namespace mozilla
 #endif  // ifndef ipc_glue_SyncChannel_h
--- a/ipc/glue/WindowsMessageLoop.cpp
+++ b/ipc/glue/WindowsMessageLoop.cpp
@@ -98,19 +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 gEventLoopMessage =
-    RegisterWindowMessage(L"SyncChannel Windows Message Loop Message");
-
 UINT gOOPPSpinNativeLoopEvent =
     RegisterWindowMessage(L"SyncChannel Spin Inner Loop Message");
 
 UINT gOOPPStopNativeLoopEvent =
     RegisterWindowMessage(L"SyncChannel Stop Inner Loop Message");
 
 const wchar_t kOldWndProcProp[] = L"MozillaIPCOldWndProc";
 
@@ -585,90 +582,89 @@ TimeoutHasExpired(const TimeoutData& aDa
 // 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.
-bool
+void
 RPCChannel::SpinInternalEventLoop()
 {
   EnterSpinLoop();
 
-  // Nested windows event loop that's triggered when the child enters into modal
-  // event procedures.
+  // 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 false;
+        return;
       }
     }
 
     if (!RPCChannel::IsSpinLoopActive()) {
       ExitSpinLoop();
-      return false;
+      return;
     }
 
-    // If a modal loop in the child has exited, we want to disable the spin
-    // loop. However, we must continue to wait for a response from the last
-    // rpc call. Returning false here will cause the thread to drop down
-    // into deferred message processing.
+    // 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 false;
-    }
-
-    // At whatever depth we currently sit, a reply to the rpc call we were
-    // waiting for has been received. Exit out of here and respond to it.
-    // Returning true here causes the WaitForNotify() to return.
-    if (PeekMessageW(&msg, (HWND)-1, gEventLoopMessage, gEventLoopMessage,
-                     PM_REMOVE)) {
-      ExitSpinLoop();
-      return true;
+      return;
     }
 
     // Retrieve window or thread messages
     if (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) {
       if (msg.message == gOOPPStopNativeLoopEvent) {
         DecModalLoopCnt();
         ExitSpinLoop();
-        return false;
+        return;
       }
       else if (msg.message == gOOPPSpinNativeLoopEvent) {
         // Keep the spin loop counter accurate, multiple plugins can show ui.
         IncModalLoopCnt();
         continue;
       }
-      else if (msg.message == gEventLoopMessage) {
-        ExitSpinLoop();
-        return true;
-      }
 
       // 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 false;
+          return;
       }
-    } else {
-      // Block and wait for any posted application messages
-      WaitMessage();
+    }
+
+    // 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();
@@ -718,19 +714,24 @@ SyncChannel::WaitForNotify()
       }
 
       // Wait until we have a message in the queue. MSDN docs are a bit unclear
       // but it seems that windows from two different threads (and it should be
       // noted that a thread in another process counts as a "different thread")
       // will implicitly have their message queues attached if they are parented
       // to one another. This wait call, then, will return for a message
       // delivered to *either* thread.
-      DWORD result = MsgWaitForMultipleObjects(0, NULL, FALSE, INFINITE,
+      DWORD result = MsgWaitForMultipleObjects(1, &mEvent, FALSE, INFINITE,
                                                QS_ALLINPUT);
-      if (result != WAIT_OBJECT_0) {
+      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;
@@ -747,23 +748,16 @@ SyncChannel::WaitForNotify()
         (HIWORD(GetQueueStatus(QS_SENDMESSAGE)) & QS_SENDMESSAGE) != 0;
 
       // This PeekMessage call will actually process all "nonqueued" messages
       // that are pending before returning. If we have "nonqueued" messages
       // pending then we should have switched out all the window procedures
       // above. In that case this PeekMessage call won't actually cause any
       // mozilla code (or plugin code) to run.
 
-      // We check first to see if we should break out of the loop by looking for
-      // the special message from the IO thread. We pull it out of the queue.
-      if (PeekMessageW(&msg, (HWND)-1, gEventLoopMessage, gEventLoopMessage,
-                       PM_REMOVE)) {
-        break;
-      }
-
       // If the following PeekMessage call fails to return a message for us (and
       // returns false) and we didn't run any "nonqueued" messages then we must
       // have woken up for a message designated for a window in another thread.
       // If we loop immediately then we could enter a tight loop, so we'll give
       // up our time slice here to let the child process its message.
       if (!PeekMessageW(&msg, NULL, 0, 0, PM_NOREMOVE) &&
           !haveSentMessagesPending) {
         // Message was for child, we should wait a bit.
@@ -814,17 +808,18 @@ RPCChannel::WaitForNotify()
   Init();
 
   MutexAutoUnlock unlock(mMutex);
 
   bool retval = true;
 
   if (WaitNeedsSpinLoop()) {
     SpinInternalEventLoop();
-    return true; // bug 545338
+    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!");
   }
 
@@ -855,19 +850,24 @@ RPCChannel::WaitForNotify()
       // Don't get wrapped up in here if the child connection dies.
       {
         MutexAutoLock lock(mMutex);
         if (!Connected()) {
           break;
         }
       }
 
-      DWORD result = MsgWaitForMultipleObjects(0, NULL, FALSE, INFINITE,
+      DWORD result = MsgWaitForMultipleObjects(1, &mEvent, FALSE, INFINITE,
                                                QS_ALLINPUT);
-      if (result != WAIT_OBJECT_0) {
+      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;
@@ -902,37 +902,31 @@ RPCChannel::WaitForNotify()
         UnhookNeuteredWindows();
 
         // Send all deferred "nonqueued" messages to the intended receiver.
         // We're dropping into SpinInternalEventLoop so we should be fairly
         // certain these will get deliverd soon.
         ScheduleDeferredMessageRun();
         
         // Spin the internal dispatch message loop during calls to WaitForNotify
-        // until the child process tells us the modal loop has closed. A return
-        // of true indicates gEventLoopMessage was received, exit out of
-        // WaitForNotify so we can deal with it in RPCChannel.
+        // until the child process tells us the modal loop has closed.
         IncModalLoopCnt();
         SpinInternalEventLoop();
-        return true; // bug 545338
+        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();
         break;
       }
 
-      if (PeekMessageW(&msg, (HWND)-1, gEventLoopMessage, gEventLoopMessage,
-                       PM_REMOVE)) {
-        break;
-      }
-
       if (!PeekMessageW(&msg, NULL, 0, 0, PM_NOREMOVE) &&
           !haveSentMessagesPending) {
         // Message was for child, we should wait a bit.
         SwitchToThread();
       }
     }
   }
 
@@ -963,19 +957,19 @@ RPCChannel::WaitForNotify()
 
   return retval;
 }
 
 void
 SyncChannel::NotifyWorkerThread()
 {
   mMutex.AssertCurrentThreadOwns();
-  NS_ASSERTION(gUIThreadId, "This should have been set already!");
-  if (!PostThreadMessage(gUIThreadId, gEventLoopMessage, 0, 0)) {
-    NS_WARNING("Failed to post thread message!");
+  NS_ASSERTION(mEvent, "No signal event to set, this is really bad!");
+  if (!SetEvent(mEvent)) {
+    NS_WARNING("Failed to set NotifyWorkerThread event!");
   }
 }
 
 void
 DeferredSendMessage::Run()
 {
   AssertWindowIsNotNeutered(hWnd);
   if (!IsWindow(hWnd)) {