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
--- a/dom/gamepad/windows/WindowsGamepad.cpp
+++ b/dom/gamepad/windows/WindowsGamepad.cpp
@@ -14,16 +14,17 @@
#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 {
@@ -331,17 +332,17 @@ private:
HWND sHWnd = nullptr;
static void
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);
}
aTimer->Cancel();
if (!sIsShutdown) {
aTimer->InitWithFuncCallback(DirectInputMessageLoopOnceCallback,
nullptr, kWindowsGamepadPollInterval,
nsITimer::TYPE_ONE_SHOT);
--- a/ipc/glue/WindowsMessageLoop.cpp
+++ b/ipc/glue/WindowsMessageLoop.cpp
@@ -18,16 +18,17 @@
#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
@@ -814,17 +815,17 @@ MessageChannel::SpinInternalEventLoop()
{
MonitorAutoLock lock(*mMonitor);
if (!Connected()) {
return;
}
}
// 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) {
NS_ERROR("WM_QUIT received in SpinInternalEventLoop!");
} else {
TranslateMessage(&msg);
::DispatchMessageW(&msg);
return;
@@ -904,22 +905,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 && ::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)
: mReneuter(gWindowHook != NULL)
{
MOZ_GUARD_OBJECT_NOTIFIER_INIT;
if (mReneuter) {
StopNeutering();
@@ -1130,28 +1131,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 (PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+ if (WinUtils::PeekMessage(&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 (!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();
}
}
}
if (timerId) {
@@ -1257,25 +1258,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 (PeekMessageW(&msg, gCOMWindow, 0, 0, PM_REMOVE)) {
+ if (WinUtils::PeekMessage(&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 (!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();
}
}
if (timerId) {
KillTimer(nullptr, timerId);
--- a/mozglue/misc/StackWalk.cpp
+++ b/mozglue/misc/StackWalk.cpp
@@ -604,16 +604,19 @@ 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,50 +679,54 @@ 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);
- 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
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);
- 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
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 (::GetMessageW(&msg, nullptr, 0, 0)) {
+ while (WinUtils::GetMessage(&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,16 +22,17 @@
#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 {
@@ -344,22 +345,25 @@ 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 |= ::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
}
void
NotifyActivity(ActivityType aActivityType)
{
MOZ_ASSERT(NS_IsMainThread(),