pepper: Make sure gesture events are mapped to touch events correctly in fullscreen.

pepper: Make sure gesture events are mapped to touch events correctly in fullscreen.

If a plugin accepts raw touch events, then the scroll gesture should be
converted into mouse-wheel events. Otherwise, a scroll gesture should be
converted into a mouse-drag event.

BUG=chrome-os-partner:17433, chrome-os-partner:17428
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180248

[sadrul, 2013-02-01 19:06:38]

I tested this with both fullscreen videos and PDF documents. The patch fixes the
bug for the fullscreen video. I noticed that the events for a fullscreen PDF
document does not follow this codepath, and so it does not break the
touch-scrolling (or any other touch-event related behaviour) in PDF documents.

piman@ Please review.

rjkroege@/jhorwich@: FYI.

[piman, 2013-02-01 19:19:51]

https://codereview.chromium.org/12093110/diff/4001/content/renderer/render_wi...
File content/renderer/render_widget_fullscreen_pepper.cc (right):

https://codereview.chromium.org/12093110/diff/4001/content/renderer/render_wi...
content/renderer/render_widget_fullscreen_pepper.cc:82: break;
I don't understand this.
A FlingStart should be a Mouse Up (fallthrough) but only on a touch screen? 

Definitely needs a comment because that is far from obvious. Also, I'd prefer
not to rely on conditional fallthrough, it's really hard to read, so prefer
something like:

if (gesture.sourceDevice == WebGestureEvent::Touchscreen) {
  // A Fling Start on a touch screen should be a mouse up because yaddi yadda...
  mouse.type = WebInputEvent::MouseUp;
  break;
} else {
  // Otherwise ignore the event.
  return mouse;
}

https://codereview.chromium.org/12093110/diff/4001/content/renderer/render_wi...
content/renderer/render_widget_fullscreen_pepper.cc:235: // fullscreen PDF. So,
we should continue to generate wheel events for
fullscreen PDF doesn't use this codepath (only used for Flash), I don't think
this comment is necessary.

[sadrul, 2013-02-01 19:56:23]

Thanks for the quick review!

[sadrul, 2013-02-01 19:58:04]

rietveld ate my comments!

https://codereview.chromium.org/12093110/diff/4001/content/renderer/render_wi...
File content/renderer/render_widget_fullscreen_pepper.cc (right):

https://codereview.chromium.org/12093110/diff/4001/content/renderer/render_wi...
content/renderer/render_widget_fullscreen_pepper.cc:82: break;
On 2013/02/01 19:19:52, piman wrote:
> I don't understand this.
> A FlingStart should be a Mouse Up (fallthrough) but only on a touch screen? 
> 
> Definitely needs a comment because that is far from obvious. Also, I'd prefer
> not to rely on conditional fallthrough, it's really hard to read, so prefer
> something like:
> 
> if (gesture.sourceDevice == WebGestureEvent::Touchscreen) {
>   // A Fling Start on a touch screen should be a mouse up because yaddi
yadda...
>   mouse.type = WebInputEvent::MouseUp;
>   break;
> } else {
>   // Otherwise ignore the event.
>   return mouse;
> }

Done.

https://codereview.chromium.org/12093110/diff/4001/content/renderer/render_wi...
content/renderer/render_widget_fullscreen_pepper.cc:235: // fullscreen PDF. So,
we should continue to generate wheel events for
On 2013/02/01 19:19:52, piman wrote:
> fullscreen PDF doesn't use this codepath (only used for Flash), I don't think
> this comment is necessary.

Whoops. This comment was for myself to check the PDF scrolling behaviour. I
forgot to remove it before uploading for review. Removed.

[piman, 2013-02-01 20:18:19]

lgtm

[rjkroege, 2013-02-04 20:54:55]

This does look better than my wheel scheme.

I have a concern though: won't this change make touch dragging on flash content
work differently in full vs non-fullscreen? 

See: Source/WebKit/chromium/src/WebPluginContainerImpl.cpp -- we scroll plugins
that don't know how to handle touches.