Make Interaction Media Features MQ dynamic on Linux.

Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418, 495634
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2827803002
Cr-Commit-Position: refs/heads/master@{#472536}
Committed: https://chromium.googlesource.com/chromium/src/+/83f1b6d5641a294f5b1fb8d52e37e0a1f9ff08d5

[darktears, 2017-04-21 21:39:33]

Patchset #1 (id:1) has been deleted

[darktears, 2017-04-21 21:41:04]

Description was changed from

==========
WIP

BUG=
==========

to

==========
Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418,495634
==========

[darktears, 2017-04-21 21:41:33]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 21:42:06]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 21:54:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 21:54:53]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[darktears, 2017-04-21 22:17:57]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 22:18:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[darktears, 2017-04-21 22:49:49]

alexis.menard@intel.com changed reviewers:
+ mustaq@chromium.org, rbyers@chromium.org

[darktears, 2017-04-21 22:49:50]

First patch. Tested on ChromeOS, I need to install regular Linux desktop on a
convertible/detachable to double check.

Feel free to CC the relevant reviewers/people to give me feedback.

[commit-bot: I haz the power, 2017-04-21 22:56:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 22:56:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[darktears, 2017-04-24 17:49:12]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 17:49:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mustaq, 2017-04-24 17:58:03]

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

[mustaq, 2017-04-24 17:58:04]

Thanks for picking this up. Here are my comments on MQs and the test code.

Adding sadrul@ for Linux specific comments.

https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
File ui/base/touch/touch_device_linux.cc (right):

https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
ui/base/touch/touch_device_linux.cc:48: return isMouseOrTouchpadPresent() ?
HOVER_TYPE_HOVER : HOVER_TYPE_NONE;
For mouse+touch case, the return value should be HOVER|NONE as before.

https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
ui/base/touch/touch_device_linux.cc:73: bool
return_available_pointer_and_hover_types_for_testing = false;
This code duplication with touch_device_win.cc looks odd. Since this testing
code is platform independent, I suggest moving all test specific code (including
a dummy |GetAvailablePointerAndHoverTypes()| into a separate source file
touch_device_unittest.cc, and including the new file in the "ui_base_unittests"
rule in ui/base/BUILD.gn along with appropriate "sources -= touch_device_*"
rules.

[darktears, 2017-04-24 18:23:09]

On 2017/04/24 17:58:04, mustaq wrote:
> Thanks for picking this up. Here are my comments on MQs and the test code.
> 
> Adding sadrul@ for Linux specific comments.
> 
>
https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
> File ui/base/touch/touch_device_linux.cc (right):
> 
>
https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
> ui/base/touch/touch_device_linux.cc:48: return isMouseOrTouchpadPresent() ?
> HOVER_TYPE_HOVER : HOVER_TYPE_NONE;
> For mouse+touch case, the return value should be HOVER|NONE as before.

Ok.

> 
>
https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
> ui/base/touch/touch_device_linux.cc:73: bool
> return_available_pointer_and_hover_types_for_testing = false;
> This code duplication with touch_device_win.cc looks odd. Since this testing
> code is platform independent, I suggest moving all test specific code
(including
> a dummy |GetAvailablePointerAndHoverTypes()| into a separate source file
> touch_device_unittest.cc, and including the new file in the
"ui_base_unittests"
> rule in ui/base/BUILD.gn along with appropriate "sources -= touch_device_*"
> rules.

Yes I was thinking making that better, wanted to get some CQ love and get
something up for discussion.

[commit-bot: I haz the power, 2017-04-24 18:27:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-24 18:27:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[darktears, 2017-04-26 16:59:51]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 17:00:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 17:05:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-26 17:05:07]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[darktears, 2017-04-26 17:05:24]

Patchset #3 (id:60001) has been deleted

[darktears, 2017-04-26 17:05:50]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 17:06:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 17:27:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-26 17:27:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[darktears, 2017-04-26 17:43:19]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 17:43:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 18:23:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-26 18:23:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[darktears, 2017-05-09 16:08:12]

Description was changed from

==========
Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418,495634
==========

to

==========
Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418,495634
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[darktears, 2017-05-09 16:08:22]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 16:08:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[darktears, 2017-05-09 20:35:11]

On 2017/04/24 18:23:09, darktears wrote:
> On 2017/04/24 17:58:04, mustaq wrote:
> > Thanks for picking this up. Here are my comments on MQs and the test code.
> > 
> > Adding sadrul@ for Linux specific comments.
> > 
> >
>
https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
> > File ui/base/touch/touch_device_linux.cc (right):
> > 
> >
>
https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
> > ui/base/touch/touch_device_linux.cc:48: return isMouseOrTouchpadPresent() ?
> > HOVER_TYPE_HOVER : HOVER_TYPE_NONE;
> > For mouse+touch case, the return value should be HOVER|NONE as before.
> 
> Ok.
> 
> > 
> >
>
https://codereview.chromium.org/2827803002/diff/40001/ui/base/touch/touch_dev...
> > ui/base/touch/touch_device_linux.cc:73: bool
> > return_available_pointer_and_hover_types_for_testing = false;
> > This code duplication with touch_device_win.cc looks odd. Since this testing
> > code is platform independent, I suggest moving all test specific code
> (including
> > a dummy |GetAvailablePointerAndHoverTypes()| into a separate source file
> > touch_device_unittest.cc, and including the new file in the
> "ui_base_unittests"
> > rule in ui/base/BUILD.gn along with appropriate "sources -= touch_device_*"
> > rules.
> 
> Yes I was thinking making that better, wanted to get some CQ love and get
> something up for discussion.

@mustaq @sadrul : Please have a look.

Thanks.

[commit-bot: I haz the power, 2017-05-10 00:47:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 00:47:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sadrul, 2017-05-10 01:47:54]

https://codereview.chromium.org/2827803002/diff/120001/content/browser/frame_...
File content/browser/frame_host/frame_tree_browsertest.cc (right):

https://codereview.chromium.org/2827803002/diff/120001/content/browser/frame_...
content/browser/frame_host/frame_tree_browsertest.cc:712: // frame initiating
the navigation. We disable those checks for this test, to
git cl format

https://codereview.chromium.org/2827803002/diff/120001/content/browser/render...
File content/browser/renderer_host/input/input_device_change_observer.cc
(right):

https://codereview.chromium.org/2827803002/diff/120001/content/browser/render...
content/browser/renderer_host/input/input_device_change_observer.cc:22:
ui::DeviceDataManager::GetInstance()->AddObserver(this);
I have left a comment in https://codereview.chromium.org/2690323002/#msg84

This should be:
 
#if defined(OS_WIN) || defined(OS_LINUX)
  ui::InputDeviceManager::GetInstance()->AddObserver(this);
#endif

https://codereview.chromium.org/2827803002/diff/120001/ui/base/BUILD.gn
File ui/base/BUILD.gn (right):

https://codereview.chromium.org/2827803002/diff/120001/ui/base/BUILD.gn#newco...
ui/base/BUILD.gn:648: "test/user_interactive_test_case.h",
Can touch_device_util be here? (and maybe called touch_device_test_support)

https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
File ui/base/touch/touch_device_linux.cc (right):

https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
ui/base/touch/touch_device_linux.cc:18: bool isMouseOrTouchpadPresent() {
IsMouseOrTouchpadPresent()

https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
File ui/base/touch/touch_device_util.cc (right):

https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
ui/base/touch/touch_device_util.cc:25: if
(return_available_pointer_and_hover_types_for_testing)
Indenting is off here

https://codereview.chromium.org/2827803002/diff/120001/ui/events/devices/inpu...
File ui/events/devices/input_device.h (right):

https://codereview.chromium.org/2827803002/diff/120001/ui/events/devices/inpu...
ui/events/devices/input_device.h:48: bool enabled;
bool enabled = true;

https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
File ui/events/ozone/evdev/event_converter_evdev.cc (right):

https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
ui/events/ozone/evdev/event_converter_evdev.cc:65: enabled_ =
input_device_.enabled = enabled;
Can we get rid of |enabled_|?

[spang, 2017-05-10 16:03:40]

spang@chromium.org changed reviewers:
+ spang@chromium.org

[spang, 2017-05-10 16:03:41]

https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
File ui/events/ozone/evdev/input_device_factory_evdev.cc (right):

https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
ui/events/ozone/evdev/input_device_factory_evdev.cc:335:
UpdateDirtyFlags(converter);
Please update dirtiness conservatively - that's the purpose of tracking
dirtiness. If nothing changed, it's not dirty.

As-is, this will cause hotplug notifications to chrome for every device on the
system, every time the configuration changes.

[darktears, 2017-05-12 22:22:47]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 22:23:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 23:04:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 23:04:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[darktears, 2017-05-15 15:48:38]

On 2017/05/10 01:47:54, sadrul wrote:
>
https://codereview.chromium.org/2827803002/diff/120001/content/browser/frame_...
> File content/browser/frame_host/frame_tree_browsertest.cc (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/content/browser/frame_...
> content/browser/frame_host/frame_tree_browsertest.cc:712: // frame initiating
> the navigation. We disable those checks for this test, to
> git cl format

Ran it.

> 
>
https://codereview.chromium.org/2827803002/diff/120001/content/browser/render...
> File content/browser/renderer_host/input/input_device_change_observer.cc
> (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/content/browser/render...
> content/browser/renderer_host/input/input_device_change_observer.cc:22:
> ui::DeviceDataManager::GetInstance()->AddObserver(this);
> I have left a comment in https://codereview.chromium.org/2690323002/#msg84

I've replied there but basically because Windows doesn't have a reliable way to
keep a list of connected devices I believe it doesn't make sense to subclass
InputDeviceManager just to avoid the special ifdef for Windows. The last
platform Android is expected to follow Linux as well. 

> 
> This should be:
>  
> #if defined(OS_WIN) || defined(OS_LINUX)
>   ui::InputDeviceManager::GetInstance()->AddObserver(this);
> #endif
> 
> https://codereview.chromium.org/2827803002/diff/120001/ui/base/BUILD.gn
> File ui/base/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/base/BUILD.gn#newco...
> ui/base/BUILD.gn:648: "test/user_interactive_test_case.h",
> Can touch_device_util be here? (and maybe called touch_device_test_support)

No. Mustaq asked about the same thing above but here is the reason why we can't
have this in the test_support target. Reason is that this feature is verified
(at least the best possible) with a browser test which verifies that the MQ are
updated in JavaScript. Therefore it's not an unittest so I need a hook in
content.so to be able to modify the interactions return values to emulate a
system change of interactions.

> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
> File ui/base/touch/touch_device_linux.cc (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
> ui/base/touch/touch_device_linux.cc:18: bool isMouseOrTouchpadPresent() {
> IsMouseOrTouchpadPresent()
> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
> File ui/base/touch/touch_device_util.cc (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/base/touch/touch_de...
> ui/base/touch/touch_device_util.cc:25: if
> (return_available_pointer_and_hover_types_for_testing)
> Indenting is off here

Fixed.

> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/events/devices/inpu...
> File ui/events/devices/input_device.h (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/events/devices/inpu...
> ui/events/devices/input_device.h:48: bool enabled;
> bool enabled = true;

Fixed.

> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
> File ui/events/ozone/evdev/event_converter_evdev.cc (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
> ui/events/ozone/evdev/event_converter_evdev.cc:65: enabled_ =
> input_device_.enabled = enabled;
> Can we get rid of |enabled_|?

Done.

[darktears, 2017-05-15 15:48:55]

On 2017/05/10 16:03:41, spang wrote:
>
https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
> File ui/events/ozone/evdev/input_device_factory_evdev.cc (right):
> 
>
https://codereview.chromium.org/2827803002/diff/120001/ui/events/ozone/evdev/...
> ui/events/ozone/evdev/input_device_factory_evdev.cc:335:
> UpdateDirtyFlags(converter);
> Please update dirtiness conservatively - that's the purpose of tracking
> dirtiness. If nothing changed, it's not dirty.
> 
> As-is, this will cause hotplug notifications to chrome for every device on the
> system, every time the configuration changes.

Make sense I've updated the patch.

[darktears, 2017-05-15 16:29:15]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 16:30:00]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 18:36:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-15 18:36:21]

Dry run: This issue passed the CQ dry run.

[Rick Byers, 2017-05-16 13:52:26]

Description was changed from

==========
Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418,495634
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418,495634
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Rick Byers, 2017-05-16 13:52:27]

rbyers@chromium.org changed reviewers:
- rbyers@chromium.org

[Rick Byers, 2017-05-16 13:53:14]

rbyers@chromium.org changed reviewers:
+ rbyers@chromium.org

[Rick Byers, 2017-05-16 13:53:15]

FYI: Moved myself to cc as you've already got the real experts in this code on
the review (and I'm not an OWNER in any of these directories).  Thanks for
working on this though - I'm looking forward to this working properly!

[Rick Byers, 2017-05-16 13:53:31]

rbyers@chromium.org changed reviewers:
- rbyers@chromium.org

[darktears, 2017-05-16 15:01:41]

alexis.menard@intel.com changed reviewers:
+ jam@chromium.org

[darktears, 2017-05-16 15:01:42]

jam@chromium.org: Please review changes in content/

[spang, 2017-05-16 15:27:43]

ui/events/ozone lgtm

Is there any plan to support this on X11?

[darktears, 2017-05-16 15:31:16]

On 2017/05/16 15:27:43, spang wrote:
> ui/events/ozone lgtm
> 
> Is there any plan to support this on X11?

Yes, as soon as I have time to setup a convertible/detachable with Linux on it.
I will move onto Android as well.

[mustaq, 2017-05-16 16:33:44]

Pointer/hover media query values lgtm.

[jam, 2017-05-17 01:21:20]

lgtm

[sadrul, 2017-05-17 15:32:17]

lgtm

https://codereview.chromium.org/2827803002/diff/160001/ui/base/touch/touch_de...
File ui/base/touch/touch_device_linux.cc (right):

https://codereview.chromium.org/2827803002/diff/160001/ui/base/touch/touch_de...
ui/base/touch/touch_device_linux.cc:18: bool isMouseOrTouchpadPresent() {
IsMouseOrTouchpadPresent()

https://codereview.chromium.org/2827803002/diff/160001/ui/base/touch/touch_de...
ui/base/touch/touch_device_linux.cc:52: available_hover_types |=
HOVER_TYPE_HOVER;
One sidenote: having both NONE and HOVER bits set is confusing.

https://codereview.chromium.org/2827803002/diff/160001/ui/events/devices/inpu...
File ui/events/devices/input_device.h (right):

https://codereview.chromium.org/2827803002/diff/160001/ui/events/devices/inpu...
ui/events/devices/input_device.h:50: 
Can you please also update InputDevice mojom in
ui/events/devices/mojo/input_devices.mojom (and the corresponding
struct-traits)? You can do that in a follow up CL if you want.

[mustaq, 2017-05-17 16:46:55]

Still lgtm:

https://codereview.chromium.org/2827803002/diff/160001/ui/base/touch/touch_de...
File ui/base/touch/touch_device_linux.cc (right):

https://codereview.chromium.org/2827803002/diff/160001/ui/base/touch/touch_de...
ui/base/touch/touch_device_linux.cc:52: available_hover_types |=
HOVER_TYPE_HOVER;
On 2017/05/17 15:32:17, sadrul wrote:
> One sidenote: having both NONE and HOVER bits set is confusing.

I agree this looks confusing but available_hover_types must reflect the union of
all input devices because the spec requires 'any-hover' to be true in both the
following cases for a device with mouse+touch:
  any-hover: none
  any-hover: hover

It is as if answering "yes" to the following two questions:
  Is there an input that doesn't support hover?
  Is there an input that supports hover?

https://drafts.csswg.org/mediaqueries-4/#any-input

So what we have here looks correct.

[darktears, 2017-05-17 17:07:01]

alexis.menard@intel.com changed reviewers:
+ dcheng@chromium.org

[darktears, 2017-05-17 17:08:53]

alexis.menard@intel.com changed reviewers:
+ kenrb@chromium.org, rsesek@chromium.org
- dcheng@chromium.org

[darktears, 2017-05-17 17:09:34]

On 2017/05/17 16:46:55, mustaq wrote:
> Still lgtm:
> 
>
https://codereview.chromium.org/2827803002/diff/160001/ui/base/touch/touch_de...
> File ui/base/touch/touch_device_linux.cc (right):
> 
>
https://codereview.chromium.org/2827803002/diff/160001/ui/base/touch/touch_de...
> ui/base/touch/touch_device_linux.cc:52: available_hover_types |=
> HOVER_TYPE_HOVER;
> On 2017/05/17 15:32:17, sadrul wrote:
> > One sidenote: having both NONE and HOVER bits set is confusing.
> 
> I agree this looks confusing but available_hover_types must reflect the union
of
> all input devices because the spec requires 'any-hover' to be true in both the
> following cases for a device with mouse+touch:
>   any-hover: none
>   any-hover: hover
> 
> It is as if answering "yes" to the following two questions:
>   Is there an input that doesn't support hover?
>   Is there an input that supports hover?
> 
> https://drafts.csswg.org/mediaqueries-4/#any-input
> 
> So what we have here looks correct.

@rsesek : mind reviewing the mojo/ bits, they require a SECURITY_OWNER review.

[Robert Sesek, 2017-05-17 17:11:15]

mojom lgtm

[darktears, 2017-05-17 17:12:17]

The CQ bit was checked by alexis.menard@intel.com

[darktears, 2017-05-17 17:12:17]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
spang@chromium.org, sadrul@chromium.org, mustaq@chromium.org
Link to the patchset: https://codereview.chromium.org/2827803002/#ps180001
(title: "Patch for landing, added the mojo bits")

[commit-bot: I haz the power, 2017-05-17 17:13:20]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 19:37:39]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1495041137063310,
"parent_rev": "5978e88f58a40606f6f2cc6535394feeb1279854", "commit_rev":
"83f1b6d5641a294f5b1fb8d52e37e0a1f9ff08d5"}

[commit-bot: I haz the power, 2017-05-17 19:37:52]

Description was changed from

==========
Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418,495634
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Make Interaction Media Features MQ dynamic on Linux.
    
With the new convertible and detachable form factors for
laptops it is important to make sure the interaction media
features are updated whenever the keyboard/trackpad combo is
flipped (so inactive) or detached. This will allow content
author to react to media query changes to adapt the user
interface to better suit the new interaction method (often
touch vs trackpad).
    
The patch make sure that the detection works by verifying the
touchpads and mouses from the input manager then make sure to
update the input device change whenever the platforms detect
changes. In order to propagate the changes a new "enabled"
property has been added to InputDevice. The reason for that
is because on ChromeOS even when the laptop is flipped the
trackpad is not removed from the list of devices, it just
doesn't propagate inputs to the UI (ASH turn it off).
    
BUG=442418,495634
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2827803002
Cr-Commit-Position: refs/heads/master@{#472536}
Committed:
https://chromium.googlesource.com/chromium/src/+/83f1b6d5641a294f5b1fb8d52e37...
==========

[commit-bot: I haz the power, 2017-05-17 19:37:55]

Committed patchset #8 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/83f1b6d5641a294f5b1fb8d52e37...