Make the native select menu work on Linux touch screen

Make the native select menus work on Linux touch screen

The master device now sends ButtonPress event when touching a native
select menu, and it is converted to another TouchPress, so that the
gesture recognizer cannot generate the right gesture tap event, which
causes the drop-down menu does not response for the touches.

We set masks for the touch events in the GrabPointer, so that master device
will send touch events not button events any more for touch screen.

BUG=458773
Committed: https://crrev.com/9054218f820a9d0502c24db35837c81a8f4ffa73
Cr-Commit-Position: refs/heads/master@{#320127}

[lanwei, 2015-03-03 19:31:16]

Patchset #1 (id:1) has been deleted

[lanwei, 2015-03-03 19:36:56]

lanwei@chromium.org changed reviewers:
+ sadrul@chromium.org

[sadrul, 2015-03-04 06:02:30]

https://codereview.chromium.org/968343002/diff/20001/ui/events/devices/x11/to...
File ui/events/devices/x11/touch_factory_x11.cc (right):

https://codereview.chromium.org/968343002/diff/20001/ui/events/devices/x11/to...
ui/events/devices/x11/touch_factory_x11.cc:164: IsTouchDevice(xiev->deviceid) &&
(xiev->sourceid == xiev->deviceid);
Add a comment here explaining the sourceid == deviceid change.

[lanwei, 2015-03-04 15:46:06]

https://codereview.chromium.org/968343002/diff/20001/ui/events/devices/x11/to...
File ui/events/devices/x11/touch_factory_x11.cc (right):

https://codereview.chromium.org/968343002/diff/20001/ui/events/devices/x11/to...
ui/events/devices/x11/touch_factory_x11.cc:164: IsTouchDevice(xiev->deviceid) &&
(xiev->sourceid == xiev->deviceid);
On 2015/03/04 06:02:29, sadrul wrote:
> Add a comment here explaining the sourceid == deviceid change.

Done.

[lanwei, 2015-03-04 15:48:46]

Patchset #2 (id:40001) has been deleted

[sadrul, 2015-03-05 09:38:53]

https://codereview.chromium.org/968343002/diff/60001/ui/events/devices/x11/to...
File ui/events/devices/x11/touch_factory_x11.cc (right):

https://codereview.chromium.org/968343002/diff/60001/ui/events/devices/x11/to...
ui/events/devices/x11/touch_factory_x11.cc:167: (xiev->sourceid ==
xiev->deviceid);
We actually want to process only the event that is coming from the master device
though, right (that's when sourceid == deviceid)?

Comment above the whole line:

  // Since SetupXI2ForXWindow() selects events from all devices, for a
  // touchscreen attached to a master pointer device, X11 sends two
  // events for each touch: one from the slave (deviceid == the id of
  // the touchscreen device), and one from the master (deviceid == the
  // id of the master pointer device). Instead of processing both
  // events, discard the event that comes from the slave, and only
  // allow processing the event coming from the master.
  // For a 'floating' touchscreen device, X11 sends only one event for
  // each touch, with both deviceid and sourceid set to the id of the
  // touchscreen device.
  bool is_from_attached_slave = xiev->sourceid != xiev->deviceid;
  return !touch_events_disabled && !is_from_attached_slave && ....;

Does that make sense?

[lanwei, 2015-03-07 03:23:44]

Patchset #3 (id:80001) has been deleted

[lanwei, 2015-03-07 04:22:51]

https://codereview.chromium.org/968343002/diff/60001/ui/events/devices/x11/to...
File ui/events/devices/x11/touch_factory_x11.cc (right):

https://codereview.chromium.org/968343002/diff/60001/ui/events/devices/x11/to...
ui/events/devices/x11/touch_factory_x11.cc:167: (xiev->sourceid ==
xiev->deviceid);
check if the device is attached-slave by looking at the device_info->use bit.

On 2015/03/05 09:38:52, sadrul wrote:
> We actually want to process only the event that is coming from the master
device
> though, right (that's when sourceid == deviceid)?
> 
> Comment above the whole line:
> 
>   // Since SetupXI2ForXWindow() selects events from all devices, for a
>   // touchscreen attached to a master pointer device, X11 sends two
>   // events for each touch: one from the slave (deviceid == the id of
>   // the touchscreen device), and one from the master (deviceid == the
>   // id of the master pointer device). Instead of processing both
>   // events, discard the event that comes from the slave, and only
>   // allow processing the event coming from the master.
>   // For a 'floating' touchscreen device, X11 sends only one event for
>   // each touch, with both deviceid and sourceid set to the id of the
>   // touchscreen device.
>   bool is_from_attached_slave = xiev->sourceid != xiev->deviceid;
>   return !touch_events_disabled && !is_from_attached_slave && ....;
> 
> Does that make sense?

Done.

[sadrul, 2015-03-08 04:01:44]

https://codereview.chromium.org/968343002/diff/100001/ui/events/devices/x11/t...
File ui/events/devices/x11/touch_factory_x11.cc (right):

https://codereview.chromium.org/968343002/diff/100001/ui/events/devices/x11/t...
ui/events/devices/x11/touch_factory_x11.cc:178: return !touch_events_disabled_
&&
device_info is leaking here.

XIQueryDevice is expensive. We must not do this for every touch event we
receive. Check sourceid vs deviceid like before instead.

[lanwei, 2015-03-09 20:06:06]

Patchset #5 (id:140001) has been deleted

[lanwei, 2015-03-09 20:08:10]

Patchset #5 (id:160001) has been deleted

[lanwei, 2015-03-09 20:08:13]

Patchset #5 (id:160001) has been deleted

[sadrul, 2015-03-11 07:43:49]

LGTM. Thanks!

https://codereview.chromium.org/968343002/diff/180001/ui/events/devices/x11/t...
File ui/events/devices/x11/touch_factory_x11.cc (right):

https://codereview.chromium.org/968343002/diff/180001/ui/events/devices/x11/t...
ui/events/devices/x11/touch_factory_x11.cc:174: (xiev->sourceid ==
xiev->deviceid);
Just one more nit:

  bool is_from_master_or_float = touch_device_list_[xiev->deviceid];
  bool is_from_slave_device = !is_from_master_or_float &&
      xiev->sourceid == xiev->deviceid;

[lanwei, 2015-03-11 18:44:41]

The CQ bit was checked by lanwei@chromium.org

[lanwei, 2015-03-11 18:44:41]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/968343002/#ps200001
(title: " ")

[commit-bot: I haz the power, 2015-03-11 18:45:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/968343002/200001

[commit-bot: I haz the power, 2015-03-11 19:49:36]

Committed patchset #6 (id:200001)

[commit-bot: I haz the power, 2015-03-11 19:50:23]

Patchset 6 (id:??) landed as
https://crrev.com/9054218f820a9d0502c24db35837c81a8f4ffa73
Cr-Commit-Position: refs/heads/master@{#320127}