Touch to Mouse conversion validation of views::GestureManager

Touch to Mouse conversion validation of views::GestureManager
 Added TouchToMouse Test in view_unittest to varify Touch to Mouse conversion
 and  event progation in view hierarchy.


BUG=none
TEST=manualy test views_unitests, all test should pass.

[Gajen, 2011-11-10 13:44:31]

Hi, Robert.
As current views_unittest doesn't test actual touch to mouse conversion and
event propagation flow of views::GestureManager::PTEFG() in view hierarchy,
hence this patch.

This patch would help in validation of GestureEvent to mouse conversion and
propagation in GestureRecognizer(GR) patch
http://codereview.chromium.org/8364039/ too.

-Gajen

[rjkroege, 2011-11-10 16:34:40]

Thanks for doing this. Looking reasonable. Please update per suggestions and
we'll see about running it through the try bots.

http://codereview.chromium.org/8521001/diff/1/ui/base/events.h
File ui/base/events.h (right):

http://codereview.chromium.org/8521001/diff/1/ui/base/events.h#newcode106
ui/base/events.h:106: #if defined(USE_X11)
can you move this code to your test? They do not seem to have to be here.

http://codereview.chromium.org/8521001/diff/1/ui/base/x/events_x.cc
File ui/base/x/events_x.cc (right):

http://codereview.chromium.org/8521001/diff/1/ui/base/x/events_x.cc#newcode357
ui/base/x/events_x.cc:357: base::NativeEvent GetNativeEventForTesting() {
see comment in .h file -- move this to the test.

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

http://codereview.chromium.org/8521001/diff/1/views/events/event.h#newcode112
views/events/event.h:112: void set_native_event(base::NativeEvent event) {
native_event_= event; }
Please wrap in a #if defined(TESTING)

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc
File views/view_unittest.cc (right):

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode277
views/view_unittest.cc:277: #if defined(TOUCH_UI)
add a TOOO about making this test work on non-X platforms. And add a #ifdef for
the native event construction to be X only/#error.

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode426
views/view_unittest.cc:426: 
remove

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode583
views/view_unittest.cc:583: #if defined(TOUCH_UI)
there is lot of code in this file that assumes touch support. it would be
desirable to exclude this #if if possible.

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode613
views/view_unittest.cc:613: // Set XEvent and its internal fields.
this code could be completely portable with different implementations of
GetNativeEventForTesting. So you can change the XEvent comment here.

[Gajen, 2011-11-11 13:39:07]

http://codereview.chromium.org/8521001/diff/1/ui/base/events.h
File ui/base/events.h (right):

http://codereview.chromium.org/8521001/diff/1/ui/base/events.h#newcode106
ui/base/events.h:106: #if defined(USE_X11)
On 2011/11/10 16:34:40, rjkroege wrote:
> can you move this code to your test? They do not seem to have to be here.

Done.

http://codereview.chromium.org/8521001/diff/1/ui/base/x/events_x.cc
File ui/base/x/events_x.cc (right):

http://codereview.chromium.org/8521001/diff/1/ui/base/x/events_x.cc#newcode357
ui/base/x/events_x.cc:357: base::NativeEvent GetNativeEventForTesting() {
On 2011/11/10 16:34:40, rjkroege wrote:
> see comment in .h file -- move this to the test.

Done.

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

http://codereview.chromium.org/8521001/diff/1/views/events/event.h#newcode112
views/events/event.h:112: void set_native_event(base::NativeEvent event) {
native_event_= event; }
On 2011/11/10 16:34:40, rjkroege wrote:
> Please wrap in a #if defined(TESTING)

Done.

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc
File views/view_unittest.cc (right):

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode277
views/view_unittest.cc:277: #if defined(TOUCH_UI)
On 2011/11/10 16:34:40, rjkroege wrote:
> add a TOOO about making this test work on non-X platforms. And add a #ifdef
for
> the native event construction to be X only/#error.

Done.

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode426
views/view_unittest.cc:426: 
On 2011/11/10 16:34:40, rjkroege wrote:
> remove

Done.

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode583
views/view_unittest.cc:583: #if defined(TOUCH_UI)
On 2011/11/10 16:34:40, rjkroege wrote:
> there is lot of code in this file that assumes touch support. it would be
> desirable to exclude this #if if possible.

Now placed, X11 based code and actual test "TouchToMouse" under #if

http://codereview.chromium.org/8521001/diff/1/views/view_unittest.cc#newcode613
views/view_unittest.cc:613: // Set XEvent and its internal fields.
On 2011/11/10 16:34:40, rjkroege wrote:
> this code could be completely portable with different implementations of
> GetNativeEventForTesting. So you can change the XEvent comment here.

Added new files test_event_utils/_x/.h/.cc under views/test, later other
platforms can provide their own implementation.

[rjkroege, 2011-11-12 16:11:35]

This looks largely good to me except that you need to adjust the .gyp pattern to
be "list everything" and then exclude. 

I'll run it through the try servers some time later this weekend.

http://codereview.chromium.org/8521001/diff/7001/views/test/test_event_utils_...
File views/test/test_event_utils_x.cc (right):

http://codereview.chromium.org/8521001/diff/7001/views/test/test_event_utils_...
views/test/test_event_utils_x.cc:8: #include <X11/extensions/XInput2.h>
FYI: It has been the case that not all the builders have recent XInput2
available so you might need to adjust this.

http://codereview.chromium.org/8521001/diff/7001/views/view_unittest.cc
File views/view_unittest.cc (right):

http://codereview.chromium.org/8521001/diff/7001/views/view_unittest.cc#newcode6
views/view_unittest.cc:6: #define TESTING 1
I thought that something like this was defined for building tests automatically.
Perhaps it was deprecated.

http://codereview.chromium.org/8521001/diff/7001/views/views.gyp
File views/views.gyp (right):

http://codereview.chromium.org/8521001/diff/7001/views/views.gyp#newcode596
views/views.gyp:596: 'test/test_event_utils.h',
this is not the conventional pattern. You need to list all the code here and
then use excludes to remove your test.

It would also be nice if you could figure out how to make the test platform
independent -- but Linux test only is still better than no test.

[Gajen, 2011-11-14 12:10:17]

Robert.
Incorporated changes,works fine on linux setup, I don't have other
setups(Mac/Win..) so can this be sent to Try bots..to check the validity..

http://codereview.chromium.org/8521001/diff/7001/views/test/test_event_utils_...
File views/test/test_event_utils_x.cc (right):

http://codereview.chromium.org/8521001/diff/7001/views/test/test_event_utils_...
views/test/test_event_utils_x.cc:8: #include <X11/extensions/XInput2.h>
On 2011/11/12 16:11:35, rjkroege wrote:
> FYI: It has been the case that not all the builders have recent XInput2
> available so you might need to adjust this.

AFAIK, USE_XI2_MT flag switch on X2.1 and X2.0 is assumed by default in TouchUI
build, and code in this file is compliant with X2.0, hence I guess there is no
need to adjust..Plz let me know If I quoted wrong things...

This header is also being used in ui/base/x/events_x.cc, without any especial
flag.

Though XInput.h is redundant..

http://codereview.chromium.org/8521001/diff/7001/views/view_unittest.cc
File views/view_unittest.cc (right):

http://codereview.chromium.org/8521001/diff/7001/views/view_unittest.cc#newcode6
views/view_unittest.cc:6: #define TESTING 1
On 2011/11/12 16:11:35, rjkroege wrote:
> I thought that something like this was defined for building tests
automatically.
> Perhaps it was deprecated.

Got idea form (player_wtl.h), So, do I need to remove this or continue with
this...?

http://codereview.chromium.org/8521001/diff/7001/views/views.gyp
File views/views.gyp (right):

http://codereview.chromium.org/8521001/diff/7001/views/views.gyp#newcode596
views/views.gyp:596: 'test/test_event_utils.h',
On 2011/11/12 16:11:35, rjkroege wrote:
> this is not the conventional pattern. You need to list all the code here and
> then use excludes to remove your test.

Done.
> 
> It would also be nice if you could figure out how to make the test platform
> independent -- but Linux test only is still better than no test.

Can we elaborate more on this please..??

[Gajen, 2011-11-16 15:10:01]

Rob and Sadrul,
Waiting for your comments.!!!