Bug 1369419 GetMessage() and PeekMessage() shouldn't be used directly as far as possible r=jimm

In TSF mode, application should retrieve messages with ITfMessagePump::GetMessage() or ITfMessagePump::PeekMessage() since TSF/TIP may handle the message before or after the host application handles it.

This patch rewrites the API users with WinUtils::(Get|Peek)Message() which use ITfMessagePump if it's available.

MozReview-Commit-ID: LwHIgp7SxLH

--HG--
extra : rebase_source : aa5750af9812f9b107c29546cbee6f9eede6ebfa

dom/gamepad/windows/WindowsGamepad.cpp

@@ -19,6 +19,7 @@
 #include "nsITimer.h"
 #include "nsTArray.h"
 #include "nsThreadUtils.h"
+#include "WinUtils.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Services.h"
@@ -336,7 +337,7 @@ DirectInputMessageLoopOnceCallback(nsITimer *aTimer, void* aClosure)
 {
   MOZ_ASSERT(NS_GetCurrentThread() == gMonitorThread);
   MSG msg;
-  while (PeekMessageW(&msg, sHWnd, 0, 0, PM_REMOVE) > 0) {
+  while (widget::WinUtils::PeekMessage(&msg, sHWnd, 0, 0, PM_REMOVE) > 0) {
     TranslateMessage(&msg);
     DispatchMessage(&msg);
   }

ipc/glue/WindowsMessageLoop.cpp

@@ -23,6 +23,7 @@
 using namespace mozilla;
 using namespace mozilla::ipc;
 using namespace mozilla::ipc::windows;
+using namespace mozilla::widget;
 
 /**
  * The Windows-only code below exists to solve a general problem with deadlocks
@@ -819,7 +820,7 @@ MessageChannel::SpinInternalEventLoop()
     }
 
     // Retrieve window or thread messages
-    if (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) {
+    if (WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) {
       // 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) {
@@ -909,12 +910,12 @@ NeuteredWindowRegion::PumpOnce()
 
   MSG msg = {0};
   // Pump any COM messages so that we don't hang due to STA marshaling.
-  if (gCOMWindow && ::PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+  if (gCOMWindow && WinUtils::PeekMessage(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
       ::TranslateMessage(&msg);
       ::DispatchMessageW(&msg);
   }
   // Expunge any nonqueued messages on the current thread.
-  ::PeekMessageW(&msg, nullptr, 0, 0, PM_NOREMOVE);
+  WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_NOREMOVE);
 }
 
 DeneuteredWindowRegion::DeneuteredWindowRegion(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL)
@@ -1135,7 +1136,7 @@ MessageChannel::WaitForSyncNotify(bool aHandleWindowsMessages)
       // We have to manually pump all COM messages *after* looking at the queue
       // queue status but before yielding our thread below.
       if (gCOMWindow) {
-        if (PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+        if (WinUtils::PeekMessage(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
           TranslateMessage(&msg);
           ::DispatchMessageW(&msg);
         }
@@ -1146,7 +1147,7 @@ MessageChannel::WaitForSyncNotify(bool aHandleWindowsMessages)
       // 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, nullptr, 0, 0, PM_NOREMOVE) &&
+      if (!WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_NOREMOVE) &&
           !haveSentMessagesPending) {
         // Message was for child, we should wait a bit.
         SwitchToThread();
@@ -1262,7 +1263,7 @@ MessageChannel::WaitForInterruptNotify()
 
     // Run all COM messages *after* looking at the queue status.
     if (gCOMWindow) {
-        if (PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+        if (WinUtils::PeekMessage(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
             TranslateMessage(&msg);
             ::DispatchMessageW(&msg);
         }
@@ -1270,7 +1271,7 @@ MessageChannel::WaitForInterruptNotify()
 
     // PeekMessage markes the messages as "old" so that they don't wake up
     // MsgWaitForMultipleObjects every time.
-    if (!PeekMessageW(&msg, nullptr, 0, 0, PM_NOREMOVE) &&
+    if (!WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_NOREMOVE) &&
         !haveSentMessagesPending) {
       // Message was for child, we should wait a bit.
       SwitchToThread();

mozglue/misc/StackWalk.cpp

@@ -609,6 +609,9 @@ WalkStackThread(void* aData)
   BOOL msgRet;
   MSG msg;
 
+  // XXX This is external linkage code.  Therefore, we cannot use WinUtils.
+  //     So, calls ::PeekMessage() and ::GetMessage() directly here.
+
   // Call PeekMessage to force creation of a message queue so that
   // other threads can safely post events to us.
   ::PeekMessage(&msg, nullptr, WM_USER, WM_USER, PM_NOREMOVE);

widget/windows/WinUtils.cpp

@@ -684,7 +684,7 @@ WinUtils::SetAPCPending()
 #endif // ACCESSIBILITY
 
 /* static */
-bool
+BOOL
 WinUtils::PeekMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
                       UINT aLastMessage, UINT aOption)
 {
@@ -699,7 +699,9 @@ WinUtils::PeekMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
     BOOL ret = FALSE;
     HRESULT hr = msgPump->PeekMessageW(aMsg, aWnd, aFirstMessage, aLastMessage,
                                        aOption, &ret);
-    NS_ENSURE_TRUE(SUCCEEDED(hr), false);
+    if (NS_WARN_IF(FAILED(hr))) {
+      return FALSE; // When PeekMessage() fails, returns FALSE.
+    }
     return ret;
   }
 #endif // #ifdef NS_ENABLE_TSF
@@ -707,7 +709,7 @@ WinUtils::PeekMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
 }
 
 /* static */
-bool
+BOOL
 WinUtils::GetMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
                      UINT aLastMessage)
 {
@@ -717,7 +719,9 @@ WinUtils::GetMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
     BOOL ret = FALSE;
     HRESULT hr = msgPump->GetMessageW(aMsg, aWnd, aFirstMessage, aLastMessage,
                                       &ret);
-    NS_ENSURE_TRUE(SUCCEEDED(hr), false);
+    if (NS_WARN_IF(FAILED(hr))) {
+      return -1; // When GetMessage() fails, returns -1.
+    }
     return ret;
   }
 #endif // #ifdef NS_ENABLE_TSF

widget/windows/WinUtils.h

@@ -233,9 +233,9 @@ public:
    * ITfMessageMgr::GetMessageW().
    * Don't call the native APIs directly.  You MUST use these methods instead.
    */
-  static bool PeekMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
+  static BOOL PeekMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
                           UINT aLastMessage, UINT aOption);
-  static bool GetMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
+  static BOOL GetMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
                          UINT aLastMessage);
 
   /**

widget/windows/nsWindow.cpp

@@ -3375,7 +3375,7 @@ FullscreenTransitionThreadProc(LPVOID lpParam)
   data = nullptr;
 
   MSG msg;
-  while (::GetMessageW(&msg, nullptr, 0, 0)) {
+  while (WinUtils::GetMessage(&msg, nullptr, 0, 0)) {
     ::TranslateMessage(&msg);
     ::DispatchMessage(&msg);
   }

xpcom/threads/HangMonitor.cpp

@@ -27,6 +27,7 @@
 
 #ifdef XP_WIN
 #include <windows.h>
+#include "WinUtils.h"
 #endif
 
 #if defined(MOZ_GECKO_PROFILER) && defined(MOZ_PROFILING) && defined(XP_WIN)
@@ -349,12 +350,15 @@ IsUIMessageWaiting()
 #define NS_WM_IMELAST  WM_IME_KEYUP
   BOOL haveUIMessageWaiting = FALSE;
   MSG msg;
-  haveUIMessageWaiting |= ::PeekMessageW(&msg, nullptr, WM_KEYFIRST,
-                                         WM_IME_KEYLAST, PM_NOREMOVE);
-  haveUIMessageWaiting |= ::PeekMessageW(&msg, nullptr, NS_WM_IMEFIRST,
-                                         NS_WM_IMELAST, PM_NOREMOVE);
-  haveUIMessageWaiting |= ::PeekMessageW(&msg, nullptr, WM_MOUSEFIRST,
-                                         WM_MOUSELAST, PM_NOREMOVE);
+  haveUIMessageWaiting |=
+    widget::WinUtils::PeekMessage(&msg, nullptr, WM_KEYFIRST,
+                                  WM_IME_KEYLAST, PM_NOREMOVE);
+  haveUIMessageWaiting |=
+    widget::WinUtils::PeekMessage(&msg, nullptr, NS_WM_IMEFIRST,
+                                  NS_WM_IMELAST, PM_NOREMOVE);
+  haveUIMessageWaiting |=
+    widget::WinUtils::PeekMessage(&msg, nullptr, WM_MOUSEFIRST,
+                                  WM_MOUSELAST, PM_NOREMOVE);
   return haveUIMessageWaiting;
 #endif
 }