Attempt at once-and-for-all fix for bug #2993 (Windows)

The patch keeps track of the mouse buttons that are currently pressed. Now, when a mouse drag occurs, it checks the state of the mouse buttons again explicitly to see whether perchance a button was released, but without the corresponding release event.

If so, it processes a release event out of order, and remembers this fact, so that the same event isn't fired twice.

The code explicitly tracks all 3 buttons, even though 
currently, just checking a general "button released" would be sufficient (the current code generally isn't concerned WHICH button was released). 
Tracking the buttons explicitly should be safer for potential future changes, though.

Don't know about any reliable way to test this, though.
Tested it inside the debugger by setting breakpoints that gave me the chance to release the mouse button out of order.

Implemented under Windows only atm.

BUG=2993
TEST=none

[Roland, 2009-07-10 05:39:25]

Hi Ben,

You seemed to be the best person to review this, based on svn blame.

Cheers,

Roland

[Roland, 2009-07-14 05:16:49]

Oops, just found a bug in my implementation, and another spot where I could make
it a bit safer. Will upload updated patch soon.

[Roland, 2009-07-15 07:26:27]

Uploaded a modified patch set that removes aforementioned bug, and should be
better in corner cases.

[Roland, 2009-07-15 07:28:19]

Updated patch for 2993, added reviewers (taken from 'svn blame').

[Peter Kasting, 2009-07-15 18:55:05]

At first glance this fix seems really worrisome.  I think we ought to understand
how we can get a drag without getting a release first, and fix that instead. 
This is a bandaid around the problem.

For example, perhaps the message loop is somehow processing events out of order.
 Perhaps the release happens off the window and we need to be capturing the
mouse in order to see it.  Perhaps Windows sometimes drops mouse messages under
load. etc.  Without understanding the root cause we don't know the right fix. 
(My money would be on the mouse capture issue, without knowing more.)

[Ben Goodger (Google), 2009-07-16 03:09:35]

Hi Roland,

Sorry for the delay. I'll look at this tomorrow.

-Ben

On 2009/07/10 05:39:25, Roland wrote:
> Hi Ben,
> 
> You seemed to be the best person to review this, based on svn blame.
> 
> Cheers,
> 
> Roland

[Roland, 2009-07-17 07:01:16]

I realize this is a hackish fix, but I don't think there is anything wrong
with the code per se - it does mouse-capturing and -tracking, and under
normal circumstances works fine. It's only when the CPU is pegged that
sometimes mouse release events seem to get lost. This is also what makes it
hard to test for, as it happens only very infrequently.

FWIW, I have seen the same problem with other software, too, not just
Chromium, usu. when switching to another program while the CPU is churning.
Another interesting tidbit is that one can rather reliably get the
out-of-order mouse release when releasing the mouse button while doing a
throwing motion. Under normal circumstances this makes no big difference,
though (the mouse release event gets fired shortly thereafter).

Cheers,

Roland

On Thu, Jul 16, 2009 at 3:55 AM, <pkasting@chromium.org> wrote:

> At first glance this fix seems really worrisome.  I think we ought to
> understand how we can get a drag without getting a release first, and
> fix that instead.  This is a bandaid around the problem.
>
> For example, perhaps the message loop is somehow processing events out
> of order.  Perhaps the release happens off the window and we need to be
> capturing the mouse in order to see it.  Perhaps Windows sometimes drops
> mouse messages under load. etc.  Without understanding the root cause we
> don't know the right fix.  (My money would be on the mouse capture
> issue, without knowing more.)
>
>
> http://codereview.chromium.org/155343
>

[omattos, 2009-07-24 03:18:31]

>  At first glance this fix seems really worrisome
Agreed.  We need to know if these messages are delivered out of order by windows
to the application (or lost entirely), or if the order is lost inside chromium
as the messages and events are queued and manipulated.

For that I suggest a tool like this:
http://www.windows-spy.com/

that way you can see all the messages received, and check who is getting them
out of order.  While I have no say in the matter, it would be disappointing if
this patch is accepted as-is without a detailed investigation to the cause of
the issue, and a fix of the root cause if the bug is anywhere within chromium.

[omattos, 2009-07-24 04:18:11]

Not a directly code related issue, but following on from my last message:

Having used that tool, I see the following messages in the "bad" case:

WM_LBUTTONDOWN  (x,y)
WM_MOUSEMOVE (same x and y)
WM_LBUTTONUP

And I see the following in the "good" case:
WM_LBUTTONDOWN  (x,y)
WM_MOUSEMOVE (same x and y)
WM_LBUTTONUP
WM_LBUTTONUP

No idea what causes the double-button-up messages, but they are clearly
significant...

Debugging tip:  Open 30 tabs per gig of ram of google wave in firefox, chrome,
and IE, and the issue reproduces nicely. (so for 4 gigs of ram, open ~120 tabs
in each browser, expect to need to reboot your PC after testing)