Allow synthesized WM_MOUSEMOVE and WM_MOUSELEAVE messages for touch based input through.

Allow synthesized WM_MOUSEMOVE and WM_MOUSELEAVE messages for touch based input through.

This ensures that focus transitions which occur on mouse moves and leaves works correctly. For e.g. the
bookmarks bar, etc.

The synthesized WM_MOUSEMOVE and WM_MOUSELEAVE messages should not interfere with the regular touch handling based
on my reading of the touch and gesture event code.

Fixes bug https://code.google.com/p/chromium/issues/detail?id=326933

BUG=326933
R=sky,girard

[sky, 2013-12-13 16:58:58]

It seems as though touch input is very fragile. Can you write tests for this?
I'm specifically thinking of an interactive test (I know you hate these) that
generates touch inputs as the OS level like we do with mouse/keyboard. Is that
possible?

[girard, 2013-12-13 17:42:58]

On 2013/12/13 16:58:58, sky wrote:
> It seems as though touch input is very fragile. Can you write tests for this?
> I'm specifically thinking of an interactive test (I know you hate these) that
> generates touch inputs as the OS level like we do with mouse/keyboard. Is that
> possible?

Windows touch uses the GetTouchInputInfo callback to extract extra info from a
message. This uses an os-level handle to manage the touch info. We could create
an alternative message and mechanism (WM_FAKE_TOUCH...) and wire in testing that
way. This wouldn't help in the current situation though, since we would also
need to inject the fake mouse events.

Another alternative is to implement a "touch device driver", but I doubt this
would be easy to test with.

Alternately, Win8 introduced some os-level hooks for injecting touch events. We
might be able to use that for win8-only.

[sky, 2013-12-13 17:54:39]

I am fine with win8-only tests. As long as we get some level of test coverage.

On Fri, Dec 13, 2013 at 9:42 AM,  <girard@chromium.org> wrote:
> On 2013/12/13 16:58:58, sky wrote:
>>
>> It seems as though touch input is very fragile. Can you write tests for
>> this?
>> I'm specifically thinking of an interactive test (I know you hate these)
>> that
>> generates touch inputs as the OS level like we do with mouse/keyboard. Is
>> that
>> possible?
>
>
> Windows touch uses the GetTouchInputInfo callback to extract extra info from
> a
> message. This uses an os-level handle to manage the touch info. We could
> create
> an alternative message and mechanism (WM_FAKE_TOUCH...) and wire in testing
> that
> way. This wouldn't help in the current situation though, since we would also
> need to inject the fake mouse events.
>
> Another alternative is to implement a "touch device driver", but I doubt
> this
> would be easy to test with.
>
> Alternately, Win8 introduced some os-level hooks for injecting touch events.
> We
> might be able to use that for win8-only.
>
> https://codereview.chromium.org/114873002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2013-12-13 20:33:55]

Ananta said he is going to do test separately, so LGTM

[tdanderson, 2013-12-13 21:02:10]

On 2013/12/13 20:33:55, sky wrote:
> Ananta said he is going to do test separately, so LGTM

+mohsen@, who I believe wrote some of the Aura-specific code in
hwnd_message_handler.

As for this patch, I don't like the idea of letting these
synthesized events through just to solve this particular
bug because of the unintended side effects they may have.

IIUC, I suspect this problem will be fixed once we make the cursor state
global for desktop Aura (https://codereview.chromium.org/111043002/)
and then re-enable our internal tracking of cursor visibility
state on touch (https://codereview.chromium.org/82463002/).
RootWindow::OnCursorVisibilityChanged(false) will be invoked on
touch, which will then synthesize our own mouse exit event
to clear the hover state. I strongly prefer this approach
rather than letting all synthetic moves/exits though.

I am in the process of building the code to verify the above.

[ananta, 2013-12-13 21:06:09]

On 2013/12/13 21:02:10, tdanderson wrote:
> On 2013/12/13 20:33:55, sky wrote:
> > Ananta said he is going to do test separately, so LGTM
> 
> +mohsen@, who I believe wrote some of the Aura-specific code in
> hwnd_message_handler.
> 
> As for this patch, I don't like the idea of letting these
> synthesized events through just to solve this particular
> bug because of the unintended side effects they may have.
> 
> IIUC, I suspect this problem will be fixed once we make the cursor state
> global for desktop Aura (https://codereview.chromium.org/111043002/)
> and then re-enable our internal tracking of cursor visibility
> state on touch (https://codereview.chromium.org/82463002/).
> RootWindow::OnCursorVisibilityChanged(false) will be invoked on
> touch, which will then synthesize our own mouse exit event
> to clear the hover state. I strongly prefer this approach
> rather than letting all synthetic moves/exits though.
> 
> I am in the process of building the code to verify the above.

The GetMessageExtraInfo API which is used to check if a mouse message is
generated
via touch does not return correct values for some mouse messages IIRC
WM_LBUTTONUP/WM_RBUTTONUP, etc.
That is a bug in the API

[ananta, 2013-12-13 21:29:00]

On 2013/12/13 21:02:10, tdanderson wrote:
> On 2013/12/13 20:33:55, sky wrote:
> > Ananta said he is going to do test separately, so LGTM
> 
> +mohsen@, who I believe wrote some of the Aura-specific code in
> hwnd_message_handler.
> 
> As for this patch, I don't like the idea of letting these
> synthesized events through just to solve this particular
> bug because of the unintended side effects they may have.
> 
> IIUC, I suspect this problem will be fixed once we make the cursor state
> global for desktop Aura (https://codereview.chromium.org/111043002/)
> and then re-enable our internal tracking of cursor visibility
> state on touch (https://codereview.chromium.org/82463002/).
> RootWindow::OnCursorVisibilityChanged(false) will be invoked on
> touch, which will then synthesize our own mouse exit event
> to clear the hover state. I strongly prefer this approach
> rather than letting all synthetic moves/exits though.
> 
> I am in the process of building the code to verify the above.

Sounds good. I will assign this bug to you and abort this patch.

[tdanderson, 2013-12-13 21:34:04]

On 2013/12/13 21:06:09, ananta wrote:
> On 2013/12/13 21:02:10, tdanderson wrote:
> > On 2013/12/13 20:33:55, sky wrote:
> > > Ananta said he is going to do test separately, so LGTM
> > 
> > +mohsen@, who I believe wrote some of the Aura-specific code in
> > hwnd_message_handler.
> > 
> > As for this patch, I don't like the idea of letting these
> > synthesized events through just to solve this particular
> > bug because of the unintended side effects they may have.
> > 
> > IIUC, I suspect this problem will be fixed once we make the cursor state
> > global for desktop Aura (https://codereview.chromium.org/111043002/)
> > and then re-enable our internal tracking of cursor visibility
> > state on touch (https://codereview.chromium.org/82463002/).
> > RootWindow::OnCursorVisibilityChanged(false) will be invoked on
> > touch, which will then synthesize our own mouse exit event
> > to clear the hover state. I strongly prefer this approach
> > rather than letting all synthetic moves/exits though.
> > 
> > I am in the process of building the code to verify the above.

I have verified that my two patches will (mostly) fix your problem:

- Tap on a bookmark icon. It shows a hovered state.
- Long-press in web contents to invoke a context menu.
- Tap to dismiss the context menu.

With my patches, the hover effect over the bookmark icon disappears
while the longpress timer is active but strangely it re-appears once
the context menu appears. But when you tap to dismiss the context 
menu the hover effect over the bookmark also disappears.

> 
> The GetMessageExtraInfo API which is used to check if a mouse message is
> generated
> via touch does not return correct values for some mouse messages IIRC
> WM_LBUTTONUP/WM_RBUTTONUP, etc.
> That is a bug in the API

Yes, I have noticed that as well. I have even seen a WM_MOUSE_MOVE come
through as a result of a tap, but without being marked as EF_FROM_TOUCH.

Are you suggesting that because some make it through by accident we should
just let all of them through?

[ananta, 2013-12-13 21:38:24]

On 2013/12/13 21:34:04, tdanderson wrote:
> On 2013/12/13 21:06:09, ananta wrote:
> > On 2013/12/13 21:02:10, tdanderson wrote:
> > > On 2013/12/13 20:33:55, sky wrote:
> > > > Ananta said he is going to do test separately, so LGTM
> > > 
> > > +mohsen@, who I believe wrote some of the Aura-specific code in
> > > hwnd_message_handler.
> > > 
> > > As for this patch, I don't like the idea of letting these
> > > synthesized events through just to solve this particular
> > > bug because of the unintended side effects they may have.
> > > 
> > > IIUC, I suspect this problem will be fixed once we make the cursor state
> > > global for desktop Aura (https://codereview.chromium.org/111043002/)
> > > and then re-enable our internal tracking of cursor visibility
> > > state on touch (https://codereview.chromium.org/82463002/).
> > > RootWindow::OnCursorVisibilityChanged(false) will be invoked on
> > > touch, which will then synthesize our own mouse exit event
> > > to clear the hover state. I strongly prefer this approach
> > > rather than letting all synthetic moves/exits though.
> > > 
> > > I am in the process of building the code to verify the above.
> 
> I have verified that my two patches will (mostly) fix your problem:
> 
> - Tap on a bookmark icon. It shows a hovered state.
> - Long-press in web contents to invoke a context menu.
> - Tap to dismiss the context menu.
> 
> With my patches, the hover effect over the bookmark icon disappears
> while the longpress timer is active but strangely it re-appears once
> the context menu appears. But when you tap to dismiss the context 
> menu the hover effect over the bookmark also disappears.
> 
> > 
> > The GetMessageExtraInfo API which is used to check if a mouse message is
> > generated
> > via touch does not return correct values for some mouse messages IIRC
> > WM_LBUTTONUP/WM_RBUTTONUP, etc.
> > That is a bug in the API
> 
> Yes, I have noticed that as well. I have even seen a WM_MOUSE_MOVE come
> through as a result of a tap, but without being marked as EF_FROM_TOUCH.
> 
> Are you suggesting that because some make it through by accident we should
> just let all of them through?

I am not suggesting that. We should make our code robust enough to handle this
case.

[tdanderson, 2013-12-17 21:25:50]

On 2013/12/13 21:38:24, ananta wrote:
> On 2013/12/13 21:34:04, tdanderson wrote:
> > On 2013/12/13 21:06:09, ananta wrote:
> > > On 2013/12/13 21:02:10, tdanderson wrote:
> > > > On 2013/12/13 20:33:55, sky wrote:
> > > > > Ananta said he is going to do test separately, so LGTM
> > > > 
> > > > +mohsen@, who I believe wrote some of the Aura-specific code in
> > > > hwnd_message_handler.
> > > > 
> > > > As for this patch, I don't like the idea of letting these
> > > > synthesized events through just to solve this particular
> > > > bug because of the unintended side effects they may have.
> > > > 
> > > > IIUC, I suspect this problem will be fixed once we make the cursor state
> > > > global for desktop Aura (https://codereview.chromium.org/111043002/)
> > > > and then re-enable our internal tracking of cursor visibility
> > > > state on touch (https://codereview.chromium.org/82463002/).
> > > > RootWindow::OnCursorVisibilityChanged(false) will be invoked on
> > > > touch, which will then synthesize our own mouse exit event
> > > > to clear the hover state. I strongly prefer this approach
> > > > rather than letting all synthetic moves/exits though.
> > > > 
> > > > I am in the process of building the code to verify the above.
> > 
> > I have verified that my two patches will (mostly) fix your problem:
> > 
> > - Tap on a bookmark icon. It shows a hovered state.
> > - Long-press in web contents to invoke a context menu.
> > - Tap to dismiss the context menu.
> > 
> > With my patches, the hover effect over the bookmark icon disappears
> > while the longpress timer is active but strangely it re-appears once
> > the context menu appears. But when you tap to dismiss the context 
> > menu the hover effect over the bookmark also disappears.
> > 
> > > 
> > > The GetMessageExtraInfo API which is used to check if a mouse message is
> > > generated
> > > via touch does not return correct values for some mouse messages IIRC
> > > WM_LBUTTONUP/WM_RBUTTONUP, etc.
> > > That is a bug in the API
> > 
> > Yes, I have noticed that as well. I have even seen a WM_MOUSE_MOVE come
> > through as a result of a tap, but without being marked as EF_FROM_TOUCH.
> > 
> > Are you suggesting that because some make it through by accident we should
> > just let all of them through?
> 
> I am not suggesting that. We should make our code robust enough to handle this
> case.

@ananta, it looks like my patches fixed the problem, so you can go ahead
and close this.