Bug 1370198 Back out the patch for bug 1369419 because the patch couldn't fix bug 1361132 and causes new crash bugs r=jimm
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 05 Jun 2017 21:52:16 +0900
changeset 362432 21e1a47c4852e1fd5463e79891a98f81fc2e930d
parent 362431 2f48933c5555569e49b900867de004ae40f57584
child 362433 c93d5b222a0f8bf1b3834071558a26d185139372
push id31977
push userarchaeopteryx@coole-files.de
push dateTue, 06 Jun 2017 09:17:27 +0000
treeherdermozilla-central@d3b8e8571020 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1370198, 1369419, 1361132
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1370198 Back out the patch for bug 1369419 because the patch couldn't fix bug 1361132 and causes new crash bugs r=jimm MozReview-Commit-ID: Eq7Dkk6Ghwc
dom/gamepad/windows/WindowsGamepad.cpp
ipc/glue/WindowsMessageLoop.cpp
mozglue/misc/StackWalk.cpp
widget/windows/WinUtils.cpp
widget/windows/WinUtils.h
widget/windows/nsWindow.cpp
xpcom/threads/HangMonitor.cpp
--- a/dom/gamepad/windows/WindowsGamepad.cpp
+++ b/dom/gamepad/windows/WindowsGamepad.cpp
@@ -14,17 +14,16 @@
 #include <hidsdi.h>
 #include <stdio.h>
 #include <xinput.h>
 
 #include "nsIComponentManager.h"
 #include "nsITimer.h"
 #include "nsTArray.h"
 #include "nsThreadUtils.h"
-#include "WinUtils.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Services.h"
 
 #include "mozilla/ipc/BackgroundParent.h"
 #include "mozilla/dom/GamepadPlatformService.h"
 
 namespace {
@@ -332,17 +331,17 @@ private:
 
 HWND sHWnd = nullptr;
 
 static void
 DirectInputMessageLoopOnceCallback(nsITimer *aTimer, void* aClosure)
 {
   MOZ_ASSERT(NS_GetCurrentThread() == gMonitorThread);
   MSG msg;
-  while (widget::WinUtils::PeekMessage(&msg, sHWnd, 0, 0, PM_REMOVE) > 0) {
+  while (PeekMessageW(&msg, sHWnd, 0, 0, PM_REMOVE) > 0) {
     TranslateMessage(&msg);
     DispatchMessage(&msg);
   }
   aTimer->Cancel();
   if (!sIsShutdown) {
     aTimer->InitWithFuncCallback(DirectInputMessageLoopOnceCallback,
                                  nullptr, kWindowsGamepadPollInterval,
                                  nsITimer::TYPE_ONE_SHOT);
--- a/ipc/glue/WindowsMessageLoop.cpp
+++ b/ipc/glue/WindowsMessageLoop.cpp
@@ -18,17 +18,16 @@
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/PaintTracker.h"
 
 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
  * that we experience when sending synchronous IPC messages to processes that
  * contain native windows (i.e. HWNDs). Windows (the OS) sends synchronous
  * messages between parent and child HWNDs in multiple circumstances (e.g.
  * WM_PARENTNOTIFY, WM_NCACTIVATE, etc.), even when those HWNDs are controlled
  * by different threads or different processes. Thus we can very easily end up
@@ -815,17 +814,17 @@ MessageChannel::SpinInternalEventLoop()
     {
       MonitorAutoLock lock(*mMonitor);
       if (!Connected()) {
         return;
       }
     }
 
     // Retrieve window or thread messages
-    if (WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) {
+    if (PeekMessageW(&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) {
           NS_ERROR("WM_QUIT received in SpinInternalEventLoop!");
       } else {
           TranslateMessage(&msg);
           ::DispatchMessageW(&msg);
           return;
@@ -905,22 +904,22 @@ NeuteredWindowRegion::PumpOnce()
 {
   if (!gWindowHook) {
     // This should be a no-op if nothing has been neutered.
     return;
   }
 
   MSG msg = {0};
   // Pump any COM messages so that we don't hang due to STA marshaling.
-  if (gCOMWindow && WinUtils::PeekMessage(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+  if (gCOMWindow && ::PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
       ::TranslateMessage(&msg);
       ::DispatchMessageW(&msg);
   }
   // Expunge any nonqueued messages on the current thread.
-  WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_NOREMOVE);
+  ::PeekMessageW(&msg, nullptr, 0, 0, PM_NOREMOVE);
 }
 
 DeneuteredWindowRegion::DeneuteredWindowRegion(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL)
   : mReneuter(gWindowHook != NULL)
 {
   MOZ_GUARD_OBJECT_NOTIFIER_INIT;
   if (mReneuter) {
     StopNeutering();
@@ -1131,28 +1130,28 @@ MessageChannel::WaitForSyncNotify(bool a
       // "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 have to manually pump all COM messages *after* looking at the queue
       // queue status but before yielding our thread below.
       if (gCOMWindow) {
-        if (WinUtils::PeekMessage(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+        if (PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
           TranslateMessage(&msg);
           ::DispatchMessageW(&msg);
         }
       }
 
       // 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 (!WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_NOREMOVE) &&
+      if (!PeekMessageW(&msg, nullptr, 0, 0, PM_NOREMOVE) &&
           !haveSentMessagesPending) {
         // Message was for child, we should wait a bit.
         SwitchToThread();
       }
     }
   }
 
   if (timerId) {
@@ -1258,25 +1257,25 @@ MessageChannel::WaitForInterruptNotify()
     }
 
     // See MessageChannel's WaitFor*Notify for details.
     bool haveSentMessagesPending =
       (HIWORD(GetQueueStatus(QS_SENDMESSAGE)) & QS_SENDMESSAGE) != 0;
 
     // Run all COM messages *after* looking at the queue status.
     if (gCOMWindow) {
-        if (WinUtils::PeekMessage(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+        if (PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
             TranslateMessage(&msg);
             ::DispatchMessageW(&msg);
         }
     }
 
     // PeekMessage markes the messages as "old" so that they don't wake up
     // MsgWaitForMultipleObjects every time.
-    if (!WinUtils::PeekMessage(&msg, nullptr, 0, 0, PM_NOREMOVE) &&
+    if (!PeekMessageW(&msg, nullptr, 0, 0, PM_NOREMOVE) &&
         !haveSentMessagesPending) {
       // Message was for child, we should wait a bit.
       SwitchToThread();
     }
   }
 
   if (timerId) {
     KillTimer(nullptr, timerId);
--- a/mozglue/misc/StackWalk.cpp
+++ b/mozglue/misc/StackWalk.cpp
@@ -604,19 +604,16 @@ WalkStackMain64(struct WalkStackData* aD
 }
 
 static unsigned int WINAPI
 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);
 
   // and tell the thread that created us that we're ready.
   HANDLE readyEvent = (HANDLE)aData;
   ::SetEvent(readyEvent);
 
--- a/widget/windows/WinUtils.cpp
+++ b/widget/windows/WinUtils.cpp
@@ -679,54 +679,50 @@ static Atomic<bool> sAPCPending;
 void
 WinUtils::SetAPCPending()
 {
   sAPCPending = true;
 }
 #endif // ACCESSIBILITY
 
 /* static */
-BOOL
+bool
 WinUtils::PeekMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
                       UINT aLastMessage, UINT aOption)
 {
 #ifdef ACCESSIBILITY
   if (NS_IsMainThread() && sAPCPending.exchange(false)) {
     while (sNtTestAlert() != STATUS_SUCCESS) ;
   }
 #endif
 #ifdef NS_ENABLE_TSF
   RefPtr<ITfMessagePump> msgPump = TSFTextStore::GetMessagePump();
   if (msgPump) {
     BOOL ret = FALSE;
     HRESULT hr = msgPump->PeekMessageW(aMsg, aWnd, aFirstMessage, aLastMessage,
                                        aOption, &ret);
-    if (NS_WARN_IF(FAILED(hr))) {
-      return FALSE; // When PeekMessage() fails, returns FALSE.
-    }
+    NS_ENSURE_TRUE(SUCCEEDED(hr), false);
     return ret;
   }
 #endif // #ifdef NS_ENABLE_TSF
   return ::PeekMessageW(aMsg, aWnd, aFirstMessage, aLastMessage, aOption);
 }
 
 /* static */
-BOOL
+bool
 WinUtils::GetMessage(LPMSG aMsg, HWND aWnd, UINT aFirstMessage,
                      UINT aLastMessage)
 {
 #ifdef NS_ENABLE_TSF
   RefPtr<ITfMessagePump> msgPump = TSFTextStore::GetMessagePump();
   if (msgPump) {
     BOOL ret = FALSE;
     HRESULT hr = msgPump->GetMessageW(aMsg, aWnd, aFirstMessage, aLastMessage,
                                       &ret);
-    if (NS_WARN_IF(FAILED(hr))) {
-      return -1; // When GetMessage() fails, returns -1.
-    }
+    NS_ENSURE_TRUE(SUCCEEDED(hr), false);
     return ret;
   }
 #endif // #ifdef NS_ENABLE_TSF
   return ::GetMessageW(aMsg, aWnd, aFirstMessage, aLastMessage);
 }
 
 #if defined(ACCESSIBILITY)
 static DWORD
--- a/widget/windows/WinUtils.h
+++ b/widget/windows/WinUtils.h
@@ -228,19 +228,19 @@ public:
   static void LogW(const wchar_t *fmt, ...);
 
   /**
    * PeekMessage() and GetMessage() are wrapper methods for PeekMessageW(),
    * GetMessageW(), ITfMessageMgr::PeekMessageW() and
    * 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);
 
   /**
    * Wait until a message is ready to be processed.
    * Prefer using this method to directly calling ::WaitMessage since
    * ::WaitMessage will wait if there is an unread message in the queue.
    * That can cause freezes until another message enters the queue if the
    * message is marked read by a call to PeekMessage which the caller is
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -3370,17 +3370,17 @@ FullscreenTransitionThreadProc(LPVOID lp
                  data->mBounds.width, data->mBounds.height, 0);
   data->mWnd = wnd;
   ::ReleaseSemaphore(data->mSemaphore, 1, nullptr);
   // The initialization data may no longer be valid
   // after we release the semaphore.
   data = nullptr;
 
   MSG msg;
-  while (WinUtils::GetMessage(&msg, nullptr, 0, 0)) {
+  while (::GetMessageW(&msg, nullptr, 0, 0)) {
     ::TranslateMessage(&msg);
     ::DispatchMessage(&msg);
   }
   ::ShowCursor(true);
   ::DestroyWindow(wnd);
   return 0;
 }
 
--- a/xpcom/threads/HangMonitor.cpp
+++ b/xpcom/threads/HangMonitor.cpp
@@ -22,17 +22,16 @@
 #include "GeckoProfiler.h"
 
 #ifdef MOZ_CRASHREPORTER
 #include "nsExceptionHandler.h"
 #endif
 
 #ifdef XP_WIN
 #include <windows.h>
-#include "WinUtils.h"
 #endif
 
 #if defined(MOZ_GECKO_PROFILER) && defined(MOZ_PROFILING) && defined(XP_WIN)
   #define REPORT_CHROME_HANGS
 #endif
 
 namespace mozilla {
 namespace HangMonitor {
@@ -345,25 +344,22 @@ IsUIMessageWaiting()
 {
 #ifndef XP_WIN
   return false;
 #else
 #define NS_WM_IMEFIRST WM_IME_SETCONTEXT
 #define NS_WM_IMELAST  WM_IME_KEYUP
   BOOL haveUIMessageWaiting = FALSE;
   MSG msg;
-  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);
+  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);
   return haveUIMessageWaiting;
 #endif
 }
 
 void
 NotifyActivity(ActivityType aActivityType)
 {
   MOZ_ASSERT(NS_IsMainThread(),