Bug 103278

Summary: [chromium] Touchscreen fling handling
Product: WebKit Reporter: Alexandre Elias <aelias>
Component: New BugsAssignee: Alexandre Elias <aelias>
Status: RESOLVED FIXED    
Severity: Normal CC: aelias, jamesr, rjkroege, skyostil, tomhudson, wangxianzhu, webkit.review.bot, yusufo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alexandre Elias 2012-11-26 12:13:23 PST
[chromium] Simplify fling handling
Comment 1 Alexandre Elias 2012-11-26 12:24:07 PST
Created attachment 176045 [details]
Patch
Comment 2 James Robinson 2012-11-26 12:39:25 PST
On ChromeOS we want the synthetic wheel behavior. On Android you do not, but that doesn't mean you can just delete the other path.
Comment 3 James Robinson 2012-11-26 12:41:20 PST
Comment on attachment 176045 [details]
Patch

I think you want to pick between wheel-like and touch-like flings based on the input event's SourceDevice.  If it's touchpad, do the wheel fling.  If it's touchscreen, do a touch fling.
Comment 4 Alexandre Elias 2012-11-26 12:44:20 PST
Robert, could you clarify what parts of this behavior you actually want to keep on ChromeOS and why?  I don't know what's intended behavior and what's historical cruft, so I erred on the side of deletion.
Comment 5 James Robinson 2012-11-26 12:46:25 PST
(In reply to comment #4)
> Robert, could you clarify what parts of this behavior you actually want to keep on ChromeOS and why?  I don't know what's intended behavior and what's historical cruft, so I erred on the side of deletion.

The code currently here implements touchpad-style fling (for lack of a better term).  It's what you get on a mac - a sequence of wheel events targeting a specific point on the page (not an element).  The sort of fling you want in android (which I'll call touchscreen fling) is different - there you get a sequence of scrolls (not events) targeting a specific scrollable layer (not a point).
Comment 6 Alexandre Elias 2012-11-26 15:32:24 PST
Created attachment 176093 [details]
Patch
Comment 7 Alexandre Elias 2012-11-26 15:32:59 PST
PTAL, Imade the patch a no-op for touchpad flings.
Comment 8 James Robinson 2012-11-26 15:50:39 PST
Comment on attachment 176093 [details]
Patch

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

Could you update WebCompositorInputHandlerImplTest.cpp?  It has several tests for the touchpad behavior - it would be good to have some for the touchscreen behavior as well.

> Source/WebKit/chromium/src/WebCompositorInputHandlerImpl.cpp:320
> +    TRACE_EVENT2("cc", "WebCompositorInputHandlerImpl::scrollBy", "x", increment.x, "y", increment.y);

i think this should be in the "webkit" trace category, not "cc" (the other ones in this file are wrong too, methinks)
Comment 9 Alexandre Elias 2012-11-26 17:23:08 PST
Created attachment 176123 [details]
Patch
Comment 10 Alexandre Elias 2012-11-26 17:23:35 PST
Added tests for touchscreen fling.
Comment 11 James Robinson 2012-11-26 17:25:09 PST
Comment on attachment 176123 [details]
Patch

R=me
Comment 12 WebKit Review Bot 2012-11-26 19:24:43 PST
Comment on attachment 176123 [details]
Patch

Rejecting attachment 176123 [details] from commit-queue.

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Full output: http://queues.webkit.org/results/15003371
Comment 13 WebKit Review Bot 2012-11-26 20:09:38 PST
Comment on attachment 176123 [details]
Patch

Clearing flags on attachment: 176123

Committed r135808: <http://trac.webkit.org/changeset/135808>
Comment 14 WebKit Review Bot 2012-11-26 20:09:42 PST
All reviewed patches have been landed.  Closing bug.