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

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold, working slower)]

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().

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold, working slower)]

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

Comment 2

[Ioana (away)]

According to Socorro, no builds post February 18th crashed with this signature:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Awidget%3A%3ANativeKey%3A%3ARemoveFollowingCharMessage%28%29&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&version=Firefox%3A30.0a1&version=Firefox%3A29.0a2&version=Firefox%3A28.0b&hang_type=any&date=2014-03-11+07%3A00%3A00&range_value=4#tab-reports