Update rich touch information to conform to standard

Update rich touch information to conform to standard

W3C Touch Event standard (draft)

https://dvcs.w3.org/hg/webevents/raw-file/tip/touchevents.html

introduces RadiusX, RadiusY field for touch events. This patch makes views::TouchEvent to have corresonding
fields and proper default value extracted from XEvent, and pass them from RWHVV to WebKit side.

BUG=None
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88830

[rjkroege, 2011-06-08 17:17:33]

LGTM with the mentioned nit.

http://codereview.chromium.org/7129008/diff/1/chrome/browser/renderer_host/re...
File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):

http://codereview.chromium.org/7129008/diff/1/chrome/browser/renderer_host/re...
chrome/browser/renderer_host/render_widget_host_view_views.cc:573:
UpdateTouchParams(&event, point);
why did you make this a separate function?  Seems like the code could just be
inline?

[Yufeng Shen (Slow to review), 2011-06-08 17:30:13]

http://codereview.chromium.org/7129008/diff/1/chrome/browser/renderer_host/re...
File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):

http://codereview.chromium.org/7129008/diff/1/chrome/browser/renderer_host/re...
chrome/browser/renderer_host/render_widget_host_view_views.cc:573:
UpdateTouchParams(&event, point);
On 2011/06/08 17:17:33, rjkroege wrote:
> why did you make this a separate function?  Seems like the code could just be
> inline?

I thought in the future there might be more params get added so put them in a
function we get better readability and know where to update all those params.

[sadrul, 2011-06-08 17:30:14]

I think I remember hearing 'None' is preferred over empty BUG and TEST.

http://codereview.chromium.org/7129008/diff/1/views/events/event.h
File views/events/event.h (right):

http://codereview.chromium.org/7129008/diff/1/views/events/event.h#newcode264
views/events/event.h:264: float radiusX,
In chrome code, 'radius_x' etc. instead of camelCase.

http://codereview.chromium.org/7129008/diff/1/views/events/event_x.cc
File views/events/event_x.cc (right):

http://codereview.chromium.org/7129008/diff/1/views/events/event_x.cc#newcode254
views/events/event_x.cc:254: #endif
Let's have:

float GetTouchParamFromXEvent(XEvent* xev,
    TouchParam tp,
    float default_value) {
  float value = default_value;
#if defined(HAVE_XINPUT2)
  TouchFactory::GetInstance()->ExtractTouchParam(*xev, tp, &value);
#endif
  return value;
}

and use it in the constructor (doing ' / 2.0' for radius_x_ and radius_y_) and
remove GetTouchAngleFromXEvent too

[Yufeng Shen (Slow to review), 2011-06-08 19:39:23]

http://codereview.chromium.org/7129008/diff/1/views/events/event.h
File views/events/event.h (right):

http://codereview.chromium.org/7129008/diff/1/views/events/event.h#newcode264
views/events/event.h:264: float radiusX,
On 2011/06/08 17:30:14, sadrul wrote:
> In chrome code, 'radius_x' etc. instead of camelCase.

Done.

http://codereview.chromium.org/7129008/diff/1/views/events/event_x.cc
File views/events/event_x.cc (right):

http://codereview.chromium.org/7129008/diff/1/views/events/event_x.cc#newcode254
views/events/event_x.cc:254: #endif
On 2011/06/08 17:30:14, sadrul wrote:
> Let's have:
> 
> float GetTouchParamFromXEvent(XEvent* xev,
>     TouchParam tp,
>     float default_value) {
>   float value = default_value;
> #if defined(HAVE_XINPUT2)
>   TouchFactory::GetInstance()->ExtractTouchParam(*xev, tp, &value);
> #endif
>   return value;
> }
> 
> and use it in the constructor (doing ' / 2.0' for radius_x_ and radius_y_) and
> remove GetTouchAngleFromXEvent too

Done.

[sadrul, 2011-06-08 19:52:32]

LGTM on the parts I commented on.

[Yufeng Shen (Slow to review), 2011-06-08 23:40:15]

@rjkroege: any more thoughts on this ?

[rjkroege, 2011-06-10 17:56:36]

LGTM.

BTW: you might need owners approval.

http://codereview.chromium.org/7129008/diff/6001/chrome/browser/renderer_host...
File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):

http://codereview.chromium.org/7129008/diff/6001/chrome/browser/renderer_host...
chrome/browser/renderer_host/render_widget_host_view_views.cc:110: void
UpdateTouchParams(const views::TouchEvent* event,
I still think you should put this code inline.

[syf1984, 2011-06-10 18:11:05]

http://codereview.chromium.org/7129008/diff/6001/chrome/browser/renderer_host...
File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):

http://codereview.chromium.org/7129008/diff/6001/chrome/browser/renderer_host...
chrome/browser/renderer_host/render_widget_host_view_views.cc:110: void
UpdateTouchParams(const views::TouchEvent* event,
On 2011/06/10 17:56:36, rjkroege wrote:
> I still think you should put this code inline.

Done.

[Yufeng Shen (Slow to review), 2011-06-10 18:17:46]

I don't know how my personal Gmail account got logged in ...
Removed that from the reviewers/CC list.

[Yufeng Shen (Slow to review), 2011-06-10 19:30:32]

add sky for owners approval

[sky, 2011-06-10 19:47:38]

LGTM

http://codereview.chromium.org/7129008/diff/4002/chrome/browser/renderer_host...
File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):

http://codereview.chromium.org/7129008/diff/4002/chrome/browser/renderer_host...
chrome/browser/renderer_host/render_widget_host_view_views.cc:110: inline void
UpdateTouchParams(const views::TouchEvent* event,
Make event be by ref as you don't need it to be NULL.

[Yufeng Shen (Slow to review), 2011-06-10 20:47:04]

On 2011/06/10 19:47:38, sky wrote:
> LGTM
> 
>
http://codereview.chromium.org/7129008/diff/4002/chrome/browser/renderer_host...
> File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):
> 
>
http://codereview.chromium.org/7129008/diff/4002/chrome/browser/renderer_host...
> chrome/browser/renderer_host/render_widget_host_view_views.cc:110: inline void
> UpdateTouchParams(const views::TouchEvent* event,
> Make event be by ref as you don't need it to be NULL.

done

[commit-bot: I haz the power, 2011-06-11 21:33:24]

Try job failure for 7129008-7005 on win for step compile:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2011-06-13 16:28:22]

Try job failure for 7129008-7005 on win for step pyauto_functional_tests:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[M-A Ruel, 2011-06-13 18:47:33]

On 2011/06/13 16:28:22, I haz the power (commit-bot) wrote:
> Try job failure for 7129008-7005 on win for step pyauto_functional_tests:
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

Sorry for the pyauto failure, I've removed it from the try jobs.