Bug 102541 – Add hasTouchEventhandlersAt to WebWidget API

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

Add WebInputEventAck.h to expose EventDisposition in WebCompositorInputHandlerImpl

Created attachment 174724 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[James Robinson]

Comment on attachment 174724 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174724&action=review

> Source/WebKit/chromium/public/WebInputEventAck.h:36
> +enum EventDisposition { DidHandle, DidNotHandle, DropEvent };

We don't need this.  Look at WebCompositorInputHandlerClient - it has didHandleInputEvent() and didNotHandleInputEvent(bool) already

What I meant was exposing the distinction to classes outside WebCompositorInputHandler and Client. The client makes that distinction, but when the ACK IPC is send to render_widget_host, we only have the boolean. My plan was to start sending this enum with the IPC(ViewHostMsg_HandleInputEvent_ACK) instead of a bool, so we can make decisions based on whether the event was not handled or dropped on the browser side. 

(In reply to comment #3)
> (From update of attachment 174724 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174724&action=review
> 
> > Source/WebKit/chromium/public/WebInputEventAck.h:36
> > +enum EventDisposition { DidHandle, DidNotHandle, DropEvent };
> 
> We don't need this.  Look at WebCompositorInputHandlerClient - it has didHandleInputEvent() and didNotHandleInputEvent(bool) already

[James Robinson]

(In reply to comment #4)
> What I meant was exposing the distinction to classes outside WebCompositorInputHandler and Client. The client makes that distinction, but when the ACK IPC is send to render_widget_host, we only have the boolean. My plan was to start sending this enum with the IPC(ViewHostMsg_HandleInputEvent_ACK) instead of a bool, so we can make decisions based on whether the event was not handled or dropped on the browser side. 
> 

WebCompositorInputHandlerClient is implemented in chromium (specifically in content/renderer/gpu/compositor_thread.cc), so you have access to all the information you need there and can do whatever you like (such as do something different than just sending the ACK).  You don't need any changes on the WebKit side.

[Alexandre Elias]

I chatted with leviw@ and we agreed that we need to send a didHandle/didNotHandle/dropped tri-state from the WebKit thread as well, to have a fallback for the case where the compositor-thread rects are wrong.  So this will need some kind of API change, though maybe instead of exposing a tri-state like this we could add a new WebView method to hit test for touch handler existence:

bool hasTouchHandlerAt(const WebPoint&);

Created attachment 175250 [details]
Patch

Renamed bug, uploaded new patch that adds hasTouchEventHandlersAt to WebView API. PTAL.

[James Robinson]

Comment on attachment 175250 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175250&action=review

The interface should move I think, but otherwise looks good.

> Source/WebKit/chromium/public/WebView.h:275
> +    // Check whether the given point hits any registered touch event handlers.
> +    virtual bool hasTouchEventHandlersAt(const WebPoint&) = 0;

This should be on WebWidget - that's where handleInputEvent() is

Created attachment 176069 [details]
Patch

Renamed bug to reflect changes and uploaded new patch. PTAL.

[WebKit Review Bot]

Comment on attachment 176069 [details]
Patch

Clearing flags on attachment: 176069

Committed r135787: <http://trac.webkit.org/changeset/135787>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.