Bug 102541

Summary: Add hasTouchEventhandlersAt to WebWidget API
Product: WebKit Reporter: yusufo
Component: New BugsAssignee: yusufo
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aelias, dglazkov, fishd, jamesr, leviw, rjkroege, tkent+wkapi, webkit.review.bot, yusufo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description yusufo 2012-11-16 11:13:10 PST
Add WebInputEventAck.h to expose EventDisposition in WebCompositorInputHandlerImpl
Comment 1 yusufo 2012-11-16 11:14:48 PST
Created attachment 174724 [details]
Patch
Comment 2 WebKit Review Bot 2012-11-16 11:18:14 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 James Robinson 2012-11-16 12:52:07 PST
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
Comment 4 yusufo 2012-11-16 13:31:40 PST
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
Comment 5 James Robinson 2012-11-16 13:36:38 PST
(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.
Comment 6 Alexandre Elias 2012-11-16 18:19:28 PST
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&);
Comment 7 yusufo 2012-11-20 11:21:07 PST
Created attachment 175250 [details]
Patch
Comment 8 yusufo 2012-11-20 11:21:55 PST
Renamed bug, uploaded new patch that adds hasTouchEventHandlersAt to WebView API. PTAL.
Comment 9 James Robinson 2012-11-25 22:01:19 PST
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
Comment 10 yusufo 2012-11-26 14:33:44 PST
Created attachment 176069 [details]
Patch
Comment 11 yusufo 2012-11-26 14:34:21 PST
Renamed bug to reflect changes and uploaded new patch. PTAL.
Comment 12 WebKit Review Bot 2012-11-26 16:40:40 PST
Comment on attachment 176069 [details]
Patch

Clearing flags on attachment: 176069

Committed r135787: <http://trac.webkit.org/changeset/135787>
Comment 13 WebKit Review Bot 2012-11-26 16:40:44 PST
All reviewed patches have been landed.  Closing bug.