Change system settings based on when mouse/touchpad is connected/disconnected.

Change system settings based on when mouse/touchpad is connected/disconnected.

BUG=chromium-os:22305
TEST=None yet.

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

[achuithb, 2011-12-21 01:14:30]

In this CL, I have the systems options page monitor xinput hierarchy looking for
XIDeviceEnabled/XIDeviceDisabled signals. When I receive these signals, I
refresh my view of the touchpad/mouse settings (it's mostly useful for mouse
since pluggable touchpads are probably rare).

The mouse_ctrl script has yet to be written, but when I simulate its input, this
works as expected.

[xiyuan, 2011-12-21 03:38:24]

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
File chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.cc (right):

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.cc:110:
XIHierarchyEvent* event = static_cast<XIHierarchyEvent*>(cookie->data);
Should this cast only valid when cookie->evtype == XI_HierarchyChanged?

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.cc:115: if
(event->flags & XIDeviceEnabled || event->flags & XIDeviceDisabled)
this is also only valid when cookie->evtype == XI_HierarchyChanged, right?

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
File chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h (right):

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h:32: class
Observer {
Think the new preferred way is to have the observer/listener interface in its
own header file.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h:34: virtual
~Observer() {}
protected?

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h:35: virtual
void DeviceChanged() = 0;
Think DeviceTreeChanged or DeviceHierarchyChanged is closer to what this really
means.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/system_options_handler.cc (right):

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/system_options_handler.cc:157:
base::FundamentalValue(*exists));
Think clang would bark on this and ask you to to declare the value.
Try linux_chromeos_clang.

[achuithb, 2011-12-21 22:15:01]

Thanks for the feedback. PTAL, Xiyuan.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
File chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.cc (right):

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.cc:110:
XIHierarchyEvent* event = static_cast<XIHierarchyEvent*>(cookie->data);
On 2011/12/21 03:38:24, xiyuan wrote:
> Should this cast only valid when cookie->evtype == XI_HierarchyChanged?

Yup, I did not look at the surrounding code carefully...

Done.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.cc:115: if
(event->flags & XIDeviceEnabled || event->flags & XIDeviceDisabled)
On 2011/12/21 03:38:24, xiyuan wrote:
> this is also only valid when cookie->evtype == XI_HierarchyChanged, right?

Done.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
File chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h (right):

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h:32: class
Observer {
On 2011/12/21 03:38:24, xiyuan wrote:
> Think the new preferred way is to have the observer/listener interface in its
> own header file.

Done.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h:34: virtual
~Observer() {}
On 2011/12/21 03:38:24, xiyuan wrote:
> protected?

Done.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/chromeos/xin...
chrome/browser/chromeos/xinput_hierarchy_changed_event_listener.h:35: virtual
void DeviceChanged() = 0;
On 2011/12/21 03:38:24, xiyuan wrote:
> Think DeviceTreeChanged or DeviceHierarchyChanged is closer to what this
really
> means. 

Done.

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/system_options_handler.cc (right):

http://codereview.chromium.org/8965070/diff/14002/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/system_options_handler.cc:157:
base::FundamentalValue(*exists));
On 2011/12/21 03:38:24, xiyuan wrote:
> Think clang would bark on this and ask you to to declare the value.
> Try linux_chromeos_clang.

That's unfortunate. Looks like this is part of the C++98 standard.
Done.

[achuithb, 2011-12-21 22:16:11]

James: Owner lgtm needed for ui/webui/options*

[James Hawkins, 2011-12-21 22:18:17]

lgtm

[xiyuan, 2011-12-21 22:27:36]

lgtm

[commit-bot: I haz the power, 2011-12-21 22:35:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/8965070/10019

[commit-bot: I haz the power, 2011-12-21 22:35:41]

Can't apply patch for file
chrome/browser/ui/webui/options2/chromeos/system_options_handler.cc.
While running patch -p0 --forward --force;
patching file
chrome/browser/ui/webui/options2/chromeos/system_options_handler.cc
Hunk #1 FAILED at 22.
Hunk #2 FAILED at 44.
Hunk #3 FAILED at 126.
Hunk #4 FAILED at 153.
4 out of 4 hunks FAILED -- saving rejects to file
chrome/browser/ui/webui/options2/chromeos/system_options_handler.cc.rej

[achuithb, 2011-12-21 23:34:33]

James: Had to rebase because options2/chromeos/system_options_handler.* got
renamed to options2/chromeos/system_options_handler2.*.

Could you please take another look? system_options_handler2.* are the only files
that have changed.

[James Hawkins, 2011-12-21 23:37:13]

SLGTM

[commit-bot: I haz the power, 2011-12-21 23:40:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/8965070/12008

[commit-bot: I haz the power, 2011-12-22 01:25:07]

Try job failure for 8965070-12008 (retry) on mac_rel for step
"test_shell_tests".
It's a second try, previously, step "test_shell_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2011-12-22 01:50:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/8965070/12008

[commit-bot: I haz the power, 2011-12-22 05:14:26]

Try job failure for 8965070-12008 (retry) (previous was lost) on win_rel for
step "content_unittests".
It's a second try, previously, steps "browser_tests, ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2011-12-22 05:37:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/8965070/12008

[commit-bot: I haz the power, 2011-12-22 13:22:19]

Change committed as 115525