Closed Bug 981947 Opened 7 years ago Closed 7 years ago

Use PeekMessage() instead of GetMessage() when we try removing found message with PeekMessage(PM_NOREMOVE)

Categories

(Core :: Widget: Win32, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla30
Tracking Status
firefox28 --- verified
firefox29 --- verified
firefox30 --- verified
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: crash, topcrash-metro)

Crash Data

We're not sure the crash reason why there is such cases.

Crash reports indicate that we failed to get found message which is found by PeekMessage(PM_NOREMOVE) with using GetMessage().

According to MSDN, PeekMessage(PM_REMOVE) and GetMessage() have some differences. Additionally, GetMessage() may return -1 even though it fails to get expected message. So, we should use PeekMessage(PM_REMOVE) for removing found message for avoiding the different behavior between PeekMessage(PM_NOREMOVE) and GetMessage().
This was fixed by attachment 8368509 [details] [diff] [review] and fixed bug 900802.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #18)
> Comment on attachment 8368509 [details] [diff] [review]
> Use PeekMessage() with RM_REMOVE instead of GetMessage() (landed on m-c,
> aurora and beta)
> 
> Let's try this.
> 
> We should fix at least two things after this.
> 
> 1. The result type of WinUtils::GetMessage() should be BOOL, not bool. bool
> cannot represent -1.
> 
> 2. Use GetMessage() in the message loop in nsAppShell.cpp. WM_QUIT should be
> preferred than the other messages.

(In reply to Jim Mathies [:jimm] (away 4/1-4/21) from comment #21)
> Comment on attachment 8368509 [details] [diff] [review]
> Use PeekMessage() with RM_REMOVE instead of GetMessage() (landed on m-c,
> aurora and beta)
> 
> Ok sounds good. This will be an interesting experiment.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #22)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8ded1c0760cc
> 
> Thanks! I can go to bed!!
> 
> Let's watch the result for a couple of days.

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #23)
> https://hg.mozilla.org/mozilla-central/rev/8ded1c0760cc
Blocks: 900802
Status: ASSIGNED → RESOLVED
Crash Signature: [@ mozilla::widget::NativeKey::RemoveFollowingCharMessage()]
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.