MD Settings: Add mouse settings, update pointer settings

MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers subpage when both are visible.

If the subpage is open and all mice/touchpads are disconnected, the subpage
closes.

Also address some flakiness I found in the tests.

Screenshots in the bug.

BUG=612986, 598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/fe807f4f43fc317d93df7a8e48a7c62caf42fb4b
Cr-Commit-Position: refs/heads/master@{#406396}

[michaelpg, 2016-06-29 16:56:00]

Description was changed from

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers sub-page when both are visible.

The fake adds a mouse and keyboard when running on non-Chrome OS. On
Debug builds these can be disabled via chrome://device-emulator.

BUG=612986
R=stevenjb@chromium.org
==========

to

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers sub-page when both are visible.

The fake adds a mouse and keyboard when running on non-Chrome OS. On
Debug builds these can be disabled via chrome://device-emulator.

BUG=612986
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-06-29 17:38:54]

Description was changed from

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers sub-page when both are visible.

The fake adds a mouse and keyboard when running on non-Chrome OS. On
Debug builds these can be disabled via chrome://device-emulator.

BUG=612986
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers sub-page when both are visible.

The fake adds a mouse and keyboard when running on non-Chrome OS. On
Debug builds these can be disabled via chrome://device-emulator.

Screenshots in the bug.

BUG=612986,598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-06-29 17:44:22]

PTAL! Not sure how this got so big, there's so much boilerplate to adding a new
page and handler. :-\

The logic and UI itself are remarkably small...

[stevenjb, 2016-06-30 00:07:49]

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/device_page.js (right):

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/device_page.js:67: },
nit: Any reason not to just inline these two fairly trivial functions?

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/pointers.html (right):

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/pointers.html:47:
<settings-checkbox pref="{{prefs.settings.touchpad.enable_tap_to_click}}"
nit: pref= on next line here and on line 50

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/pointers.html:75:
on-keydown="onLearnMoreLinkActivated_">
I don't see on-keydown used elsewhere, any particular reason to use it just
here?

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc
(right):

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc:35:
system::InputDeviceSettings::GetForTesting();
We should move this logic to system::InputDeviceSettings:

InputDeviceSettings* InputDeviceSettings::Get() {
  if (g_test_instance)
    return g_test_instance;
  if (!g_instance) {
    if (base::SysInfo::IsRunningOnChromeOS() {
#if USE_OZONE
      g_instance = new InputDeviceSettingsImplOzone;
#else
      g_instance = new InputDeviceSettingsImplX11;
#endif
    } else {
      g_instance = new FakeInputDeviceSettings;
    }
  }
  return g_instance;
}

Then we can eliminate ystem::InputDeviceSettings::GetForTesting() (which has a
confusing name anyway)

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc:52:
has_touchpad_ = exists;
Where are these members actually used? i.e. why not just pass |exists|?

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc:55:
base::FundamentalValue(has_touchpad_));
Sigh. At some point we're going to need to move this to an extension API, but in
the interest of moving forward this seems fine for now.

[michaelpg, 2016-07-09 00:11:31]

Patchset #3 (id:40001) has been deleted

[michaelpg, 2016-07-09 04:11:30]

PTAL. Since the upstream changed a lot I couldn't rebase in a separate patch,
but the meaningful changes I've made from patchset 2 are in:
* device_page.js
* pointers.html
* pointers.js
* device_pointer_handler.h
* device_pointer_handler.cc
* device_page_tests.js

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/device_page.js (right):

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/device_page.js:67: },
On 2016/06/30 00:07:49, stevenjb wrote:
> nit: Any reason not to just inline these two fairly trivial functions?

Done.

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/pointers.html (right):

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/pointers.html:47:
<settings-checkbox pref="{{prefs.settings.touchpad.enable_tap_to_click}}"
On 2016/06/30 00:07:49, stevenjb wrote:
> nit: pref= on next line here and on line 50

Done.

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/pointers.html:75:
on-keydown="onLearnMoreLinkActivated_">
On 2016/06/30 00:07:49, stevenjb wrote:
> I don't see on-keydown used elsewhere, any particular reason to use it just
> here?

this is moved from touchpad.html which was reviewed a while ago by dbeam. it
prevents the link from activating the radio button:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/device...

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc
(right):

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc:35:
system::InputDeviceSettings::GetForTesting();
On 2016/06/30 00:07:49, stevenjb wrote:
> We should move this logic to system::InputDeviceSettings:
> 
> InputDeviceSettings* InputDeviceSettings::Get() {
>   if (g_test_instance)
>     return g_test_instance;
>   if (!g_instance) {
>     if (base::SysInfo::IsRunningOnChromeOS() {
> #if USE_OZONE
>       g_instance = new InputDeviceSettingsImplOzone;
> #else
>       g_instance = new InputDeviceSettingsImplX11;
> #endif
>     } else {
>       g_instance = new FakeInputDeviceSettings;
>     }
>   }
>   return g_instance;
> }
> 
> Then we can eliminate ystem::InputDeviceSettings::GetForTesting() (which has a
> confusing name anyway)
> 

Done.

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc:52:
has_touchpad_ = exists;
On 2016/06/30 00:07:49, stevenjb wrote:
> Where are these members actually used? i.e. why not just pass |exists|?

Vestigial from older patches, removed, thanks!

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc:55:
base::FundamentalValue(has_touchpad_));
On 2016/06/30 00:07:49, stevenjb wrote:
> Sigh. At some point we're going to need to move this to an extension API, but
in
> the interest of moving forward this seems fine for now.

cr.sendWithPromise is supposed to be the new way of doing things because it's
mojo-migration-friendly, right?

[michaelpg, 2016-07-09 04:14:02]

Description was changed from

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers sub-page when both are visible.

The fake adds a mouse and keyboard when running on non-Chrome OS. On
Debug builds these can be disabled via chrome://device-emulator.

Screenshots in the bug.

BUG=612986,598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers subpage when both are visible.

If the subpage is open and all mice/touchpads are disconnected, the subpage
closes.

Also address some flakiness I found in the tests.

Screenshots in the bug.

BUG=612986,598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-07-09 04:17:54]

Patch Set 4 adds a minor feature: the subpage closes if you remove the mouse
and/or touchpad (otherwise the page would just be blank, even the title)

My updated tests would fail about 1/30 times, which I spent half a day
debugging. The key is to wait for 'neon-animation-finish' when transitioning
between subpages even with transitions/animations set to 0 duration.

[stevenjb, 2016-07-11 17:43:35]

lgtm

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc
(right):

https://codereview.chromium.org/2110833003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/device_pointer_handler.cc:55:
base::FundamentalValue(has_touchpad_));
On 2016/07/09 04:11:30, michaelpg wrote:
> On 2016/06/30 00:07:49, stevenjb wrote:
> > Sigh. At some point we're going to need to move this to an extension API,
but
> in
> > the interest of moving forward this seems fine for now.
> 
> cr.sendWithPromise is supposed to be the new way of doing things because it's
> mojo-migration-friendly, right?

Maybe, but some day we will will want this API available from a settings app in
kiosk mode. Ideally that will be a straight mojo API, so I guess we will cross
that bridge when we get there.

[stevenjb, 2016-07-11 17:46:21]

https://codereview.chromium.org/2110833003/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/device_page_tests.js (right):

https://codereview.chromium.org/2110833003/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/device_page_tests.js:239:
devicePage.$.pages.addEventListener('neon-animation-finish', resolve);
BTW, this seems like something we should implement more generally at some point.

[michaelpg, 2016-07-19 18:14:47]

Description was changed from

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers subpage when both are visible.

If the subpage is open and all mice/touchpads are disconnected, the subpage
closes.

Also address some flakiness I found in the tests.

Screenshots in the bug.

BUG=612986,598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers subpage when both are visible.

If the subpage is open and all mice/touchpads are disconnected, the subpage
closes.

Also address some flakiness I found in the tests.

Screenshots in the bug.

BUG=612986,598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-07-19 18:15:27]

The CQ bit was checked by michaelpg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 18:16:09]

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, 2016-07-19 18:16:10]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-19 19:26:31]

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

[commit-bot: I haz the power, 2016-07-19 19:26:32]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2016-07-19 22:04:07]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-07-19 22:04:08]

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

[commit-bot: I haz the power, 2016-07-19 22:05:05]

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, 2016-07-19 22:05:07]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-19 22:17:05]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-07-19 22:17:15]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-19 22:19:37]

Description was changed from

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers subpage when both are visible.

If the subpage is open and all mice/touchpads are disconnected, the subpage
closes.

Also address some flakiness I found in the tests.

Screenshots in the bug.

BUG=612986,598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Add mouse settings, update pointer settings

Adds mouse settings and touchpad sensitivity settings.

Updates the pointer icon and title in the device page based on whether a
mouse, touchpad, or both are connected. Shows headers separating the
mouse and touchpad sections on the pointers subpage when both are visible.

If the subpage is open and all mice/touchpads are disconnected, the subpage
closes.

Also address some flakiness I found in the tests.

Screenshots in the bug.

BUG=612986,598901
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/fe807f4f43fc317d93df7a8e48a7c62caf42fb4b
Cr-Commit-Position: refs/heads/master@{#406396}
==========

[commit-bot: I haz the power, 2016-07-19 22:19:38]

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