Automatic deployment of the virtual keyboard.

Automatic deployment of the virtual keyboard. For now we hide this under the flag: auto-virtual-keyboard.

This patch makes the controller a input device observer, instead of a shell observer. It records the presence of internal keyboards, external keyboard, and touchscreens. When the flag is enabled, the keyboard will not be shown if there does not exist a touchscreen device. The keyboard will be shown if the user is in maximized mode, or does not have an internal keyboard present.

BUG=373402
TEST=VirtualKeyboardControllerAutoTest.DisabledIfInternalKeyboardPresent, VirtualKeyboardControllerAutoTest.DisabledIfNoTouchScreen

Committed: https://crrev.com/9172bc8ad1c0514a1f0041a791dfbe9ef4549f36
Cr-Commit-Position: refs/heads/master@{#301977}

[rsadam, 2014-09-30 20:51:51]

rsadam@chromium.org changed reviewers:
+ flackr@chromium.org

[rsadam, 2014-09-30 20:51:52]

Hi Flackr, PTAL!

[rsadam, 2014-10-02 22:57:16]

Hi flackr, PTAL! This assumes that cl 618283003 lands.

[flackr, 2014-10-04 01:02:54]

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:47: // Traverse active devices and record the
presence of a touchscreen,
nit: Suggest something that implies the subsequent update. i.e.
Determines the active input devices and triggers an update of the virtual
keyboard state based on this.

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:60: if ((*screen_iter).type ==
ui::InputDeviceType::INTERNAL)
Why not deploy for an external touchscreen? Isn't this exactly the scenario we
want to support for chromeboxes?

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:89:
keyboard::switches::kAutoVirtualKeyboard)) {
SetTouchKeyboardEnabled(maximized_) and early return. Avoid changing members to
things they're not to affect logic.

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:98: keyboard::SetTouchKeyboardEnabled(false);
Can you move these two commands to enable/disable the keyboard to a private
function like SetKeyboardEnabled(bool enabled)?

Then this entire block can change to:
SetTouchKeyboardEnabled(!has_internal_keyboard_ && has_touchscreen);

And later to:
SetTouchKeyboardEnabled(!(has_internal_keyboard_ || has_external_keyboard_) &&
has_touchscreen);

See my comment in https://codereview.chromium.org/618283003/ about making it so
that maximize mode isn't a special case.

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.h:28: virtual void
OnKeyboardDeviceConfigurationChanged() OVERRIDE;
nit: empty line before private:

https://codereview.chromium.org/613343005/diff/40001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/613343005/diff/40001/chrome/app/generated_res...
chrome/app/generated_resources.grd:13830: +      <message
name="IDS_FLAGS_AUTO_VIRTUAL_KEYBOARD_DESCRIPTION" desc="Description of
about:flags option to turn on the virtual keyboard">
to turn on automatic deployment of the virtual keyboard.

[rsadam, 2014-10-23 20:10:52]

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:47: // Traverse active devices and record the
presence of a touchscreen,
On 2014/10/04 01:02:54, flackr wrote:
> nit: Suggest something that implies the subsequent update. i.e.
> Determines the active input devices and triggers an update of the virtual
> keyboard state based on this.

Done.

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:60: if ((*screen_iter).type ==
ui::InputDeviceType::INTERNAL)
On 2014/10/04 01:02:54, flackr wrote:
> Why not deploy for an external touchscreen? Isn't this exactly the scenario we
> want to support for chromeboxes?

Done.

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:89:
keyboard::switches::kAutoVirtualKeyboard)) {
On 2014/10/04 01:02:54, flackr wrote:
> SetTouchKeyboardEnabled(maximized_) and early return. Avoid changing members
to
> things they're not to affect logic.

Done.

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.cc:98: keyboard::SetTouchKeyboardEnabled(false);
On 2014/10/04 01:02:54, flackr wrote:
> Can you move these two commands to enable/disable the keyboard to a private
> function like SetKeyboardEnabled(bool enabled)?
> 
> Then this entire block can change to:
> SetTouchKeyboardEnabled(!has_internal_keyboard_ && has_touchscreen);
> 
> And later to:
> SetTouchKeyboardEnabled(!(has_internal_keyboard_ || has_external_keyboard_) &&
> has_touchscreen);
> 
> See my comment in https://codereview.chromium.org/618283003/ about making it
so
> that maximize mode isn't a special case.

Done.

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/40001/ash/virtual_keyboard_con...
ash/virtual_keyboard_controller.h:28: virtual void
OnKeyboardDeviceConfigurationChanged() OVERRIDE;
On 2014/10/04 01:02:54, flackr wrote:
> nit: empty line before private:

Done.

https://codereview.chromium.org/613343005/diff/40001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/613343005/diff/40001/chrome/app/generated_res...
chrome/app/generated_resources.grd:13830: +      <message
name="IDS_FLAGS_AUTO_VIRTUAL_KEYBOARD_DESCRIPTION" desc="Description of
about:flags option to turn on the virtual keyboard">
On 2014/10/04 01:02:54, flackr wrote:
> to turn on automatic deployment of the virtual keyboard.

Done.

[rsadam, 2014-10-23 21:24:06]

Add check so that the keyboard is not displayed while testing on Desktop.

[flackr, 2014-10-24 14:54:24]

Looks good, can't wait to see it in action.

Add TEST= line to description listing tests.

https://codereview.chromium.org/613343005/diff/100001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/100001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:14: // This class observes enter/leaving
maximized mode for virtual keyboard.
Update comment.

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:81: if (!controller ||
!controller->CanEnterMaximizeMode())
I think controller should always exist. Also, keep in mind that
CanEnterMaximizeMode is not correct at startup. It's initialized false and
requires an asynchronous response from the accelerometer before it updates to
true. We could consider adding a notification if you need to know this as soon
as possible at startup.

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:88: return !has_internal_keyboard_ &&
has_touchscreen_;
Since this is behind a flag, can we try adding && !has_external_keyboard_ and
add the UI for re-enabling it later? This should get us most of the feature.

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller_unittest.cc (right):

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller_unittest.cc:29:
keyboard::switches::kAutoVirtualKeyboard);
It's probably worth keeping a test of the current behavior until we can switch
it.

[rsadam, 2014-10-24 15:32:32]

Updated TESTS=.

https://codereview.chromium.org/613343005/diff/100001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/100001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:14: // This class observes enter/leaving
maximized mode for virtual keyboard.
On 2014/10/24 14:54:23, flackr wrote:
> Update comment.

Done.

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:81: if (!controller ||
!controller->CanEnterMaximizeMode())
On 2014/10/24 14:54:23, flackr wrote:
> I think controller should always exist. Also, keep in mind that
> CanEnterMaximizeMode is not correct at startup. It's initialized false and
> requires an asynchronous response from the accelerometer before it updates to
> true. We could consider adding a notification if you need to know this as soon
> as possible at startup.

I was getting segfaults that the controller was not ready without the check. My
thought process was that if it's ready we're fine, if it isn't, that means it'll
disable the internal keyboard some point in the future when it is ready and
we'll get a KeyboardDeviceConfigurationEvent at that point.

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:88: return !has_internal_keyboard_ &&
has_touchscreen_;
On 2014/10/24 14:54:23, flackr wrote:
> Since this is behind a flag, can we try adding && !has_external_keyboard_ and
> add the UI for re-enabling it later? This should get us most of the feature.

Done.

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller_unittest.cc (right):

https://codereview.chromium.org/613343005/diff/140001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller_unittest.cc:29:
keyboard::switches::kAutoVirtualKeyboard);
On 2014/10/24 14:54:23, flackr wrote:
> It's probably worth keeping a test of the current behavior until we can switch
> it.

My intention was for DisabledIfInternalKeyboardPresent to do that - the first
sets up the previous maximized mode environment and tests that
enabling/disabling the keyboard device has the expected result. 

An issue with directly calling the EnableMaximizeModeWIndowManager is that in
test environments you don't have an internal keyboard to disable, and I don't
see an easy way via the unittest to do this since
scoped_disable_internal_mouse_and_keyboard_x11.cc uses the raw input from X11
rather than the keyboard_devices() and touchscreen_devices() methods that we
added. Perhaps it makes sense to refactor that class? 

Even then that seems to be more of a
scoped_disable_internal_mouse_and_keyboard_x11 test than vkcontroller test,
since you're essentially testing that disable_internal does actually disable the
internal keyboard.

[rsadam, 2014-10-24 15:33:44]

I think part of the issue was that we needed this class to be created before the
accelerometer because we were a shell observer. Though since this is no longer
the case, perhaps it makes sense to delay when this class is created to after?

[rsadam, 2014-10-24 17:16:47]

Hey Rob,

I took a different crack at it - I think this approach is cleaner. Essentially
we delay construction so we can create/destroy the keyboard as we like, and are
guaranteed that maximized mode controller is already ready (preventing the
issues we saw before). I also changed the check to
IsMaximizeModeWindowManagerEnabled, since entering maximized mode does not cause
the can enter to be set to true.

In terms of tests I reintroduced the enabled during maximized mode test and now
mock the expected disable of keyboard - we can remove this once scoped_disable
is refactored to not use the raw XI2 data.

[rsadam, 2014-10-24 18:15:52]

Changed it so we preserve the old behavior during testing.

[flackr, 2014-10-27 15:20:38]

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:96: ->IsMaximizeModeWindowManagerEnabled());
This would have already been done by OnMaximizeModeStarted though right? Perhaps
this function should just be a no-op if the flag is not enabled?

[flackr, 2014-10-27 15:33:13]

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:90: // Updates the keyboard state.
Move function comments to .h file.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:91: void VirtualKeyboardController::Update()
{
Maybe call this UpdateKeyboardEnabled to differentiate from UpdateDevices.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:26: // InputDeviceObserver:
nit: ui::InputDeviceObserver

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:33: virtual void SetKeyboardEnabled(bool
enabled);
Comment above methods.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:37: bool has_touchscreen_;
Short description comments. i.e. True if ...

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller_unittest.cc (right):

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller_unittest.cc:30:
keyboard::switches::kAutoVirtualKeyboard);
Since flags often affect initialization logic it would be preferable to create a
separate suite for testing with the flag. I.e. extend
VirtualKeyboardControllerTest with VirtualKeyboardControllerAutoTest which adds
the flag before calling AshTestBase::SetUp.

[rsadam, 2014-10-27 15:51:19]

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:90: // Updates the keyboard state.
On 2014/10/27 15:33:12, flackr wrote:
> Move function comments to .h file.

Done.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:91: void VirtualKeyboardController::Update()
{
On 2014/10/27 15:33:12, flackr wrote:
> Maybe call this UpdateKeyboardEnabled to differentiate from UpdateDevices.

Done.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:96: ->IsMaximizeModeWindowManagerEnabled());
On 2014/10/27 15:20:38, flackr wrote:
> This would have already been done by OnMaximizeModeStarted though right?
Perhaps
> this function should just be a no-op if the flag is not enabled?

Since we delayed when this is created, we might miss the first call.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:26: // InputDeviceObserver:
On 2014/10/27 15:33:13, flackr wrote:
> nit: ui::InputDeviceObserver

Done.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:33: virtual void SetKeyboardEnabled(bool
enabled);
On 2014/10/27 15:33:12, flackr wrote:
> Comment above methods.

Done.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:37: bool has_touchscreen_;
On 2014/10/27 15:33:13, flackr wrote:
> Short description comments. i.e. True if ...

Done.

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller_unittest.cc (right):

https://codereview.chromium.org/613343005/diff/220001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller_unittest.cc:30:
keyboard::switches::kAutoVirtualKeyboard);
On 2014/10/27 15:33:13, flackr wrote:
> Since flags often affect initialization logic it would be preferable to create
a
> separate suite for testing with the flag. I.e. extend
> VirtualKeyboardControllerTest with VirtualKeyboardControllerAutoTest which
adds
> the flag before calling AshTestBase::SetUp.

Done.

[flackr, 2014-10-27 19:05:02]

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:32: // Determines the active input devices.
nit: Descriptive comments
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Comm...)
i.e. Updates the list of active input devices.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:33: virtual void UpdateDevices();
No virtual on any of these unless they are going to be overridden.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:36: // Creates/Destroys the virtual keyboard
as necessary.
Creates the virtual keyboard if |enabled|, destroys it otherwise.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:39: // True if there exists an external
keyboard enabled.
grammar? True if an external keyboard is connected.

You probably don't have to be explicit about the "enabled" part. That's more of
an implementation detail that while the physical keyboard still exists we
disable it in touchview.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller_unittest.cc (right):

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller_unittest.cc:51: class
VirtualKeyboardControllerAutoTest : public VirtualKeyboardControllerTest {
nit: Move this to just above the test using it.

[rsadam, 2014-10-27 19:51:42]

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:32: // Determines the active input devices.
On 2014/10/27 19:05:02, flackr wrote:
> nit: Descriptive comments
>
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Comm...)
> i.e. Updates the list of active input devices.

Done.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:33: virtual void UpdateDevices();
On 2014/10/27 19:05:01, flackr wrote:
> No virtual on any of these unless they are going to be overridden.

Done.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:36: // Creates/Destroys the virtual keyboard
as necessary.
On 2014/10/27 19:05:01, flackr wrote:
> Creates the virtual keyboard if |enabled|, destroys it otherwise.

Done.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:39: // True if there exists an external
keyboard enabled.
On 2014/10/27 19:05:02, flackr wrote:
> grammar? True if an external keyboard is connected.
> 
> You probably don't have to be explicit about the "enabled" part. That's more
of
> an implementation detail that while the physical keyboard still exists we
> disable it in touchview.

Done.

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller_unittest.cc (right):

https://codereview.chromium.org/613343005/diff/240001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller_unittest.cc:51: class
VirtualKeyboardControllerAutoTest : public VirtualKeyboardControllerTest {
On 2014/10/27 19:05:02, flackr wrote:
> nit: Move this to just above the test using it.

Done.

[flackr, 2014-10-27 19:58:09]

LGTM

nit: Update TEST= to match new test names

[rsadam, 2014-10-27 20:00:03]

On 2014/10/27 19:58:09, flackr wrote:
> LGTM
> 
> nit: Update TEST= to match new test names

Done!

[rsadam, 2014-10-27 20:01:31]

rsadam@chromium.org changed reviewers:
+ sky@chromium.org

[rsadam, 2014-10-27 20:01:31]

+sky for OWNERS

[sky, 2014-10-27 20:28:33]

LGTM

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:56: std::vector<ui::TouchscreenDevice>
screens =
nit: remove this local and move to has_touchscreen_, eg:
has_touchscreen_ = device_data_manager->touchscreen_devices() > 0.

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:65:
std::vector<ui::KeyboardDevice>::const_iterator iter;
nit: move into for loop.
nit2: use auto to make more readable.
nit3: stop looping if has_internal/has_external is true.

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:34: // Updates the keyboard state.
nit: add newlines between functions for improved readability.

[rsadam, 2014-10-28 15:07:29]

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.cc (right):

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:56: std::vector<ui::TouchscreenDevice>
screens =
On 2014/10/27 20:28:33, sky wrote:
> nit: remove this local and move to has_touchscreen_, eg:
> has_touchscreen_ = device_data_manager->touchscreen_devices() > 0.

Done.

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.cc:65:
std::vector<ui::KeyboardDevice>::const_iterator iter;
On 2014/10/27 20:28:33, sky wrote:
> nit: move into for loop.
> nit2: use auto to make more readable.
> nit3: stop looping if has_internal/has_external is true.

Done.

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
File ash/virtual_keyboard_controller.h (right):

https://codereview.chromium.org/613343005/diff/260001/ash/virtual_keyboard_co...
ash/virtual_keyboard_controller.h:34: // Updates the keyboard state.
On 2014/10/27 20:28:33, sky wrote:
> nit: add newlines between functions for improved readability.

Done.

[rsadam, 2014-10-28 15:07:34]

The CQ bit was checked by rsadam@chromium.org

[commit-bot: I haz the power, 2014-10-28 15:08:35]

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

[commit-bot: I haz the power, 2014-10-28 15:11:47]

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

[commit-bot: I haz the power, 2014-10-28 15:11:48]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/74595)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_32 on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[rsadam, 2014-10-28 15:54:12]

Fixed merge conflicts.

[rsadam, 2014-10-28 16:51:31]

rsadam@chromium.org changed reviewers:
+ mpearson@google.com

[rsadam, 2014-10-28 16:51:32]

+mpearson for histogram OWNERS

[rsadam, 2014-10-28 16:51:44]

rsadam@chromium.org changed reviewers:
+ mpearson@chromium.org
- mpearson@google.com

[rsadam, 2014-10-28 16:52:16]

Accidentally added mpearson@google as reviewer. Changed to mpearson@chromium.

[Mark P, 2014-10-28 16:56:03]

histograms.xml lgtm

[rsadam, 2014-10-28 17:37:18]

Add EVENTS export to support win compiler.

[rsadam, 2014-10-28 17:37:23]

The CQ bit was checked by rsadam@chromium.org

[commit-bot: I haz the power, 2014-10-28 17:38:40]

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

[commit-bot: I haz the power, 2014-10-28 18:49:53]

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

[commit-bot: I haz the power, 2014-10-28 18:49:53]

Try jobs failed on following builders:
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[rsadam, 2014-10-28 19:19:02]

Patchset #19 (id:360001) has been deleted

[rsadam, 2014-10-28 19:41:27]

Use EVENTS_BASE_EXPORT instead of EVENTS_EXPORT.

[rsadam, 2014-10-28 19:41:37]

The CQ bit was checked by rsadam@chromium.org

[commit-bot: I haz the power, 2014-10-28 19:43:05]

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

[commit-bot: I haz the power, 2014-10-29 00:58:25]

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

[commit-bot: I haz the power, 2014-10-29 00:58:27]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[rsadam, 2014-10-29 02:54:36]

The CQ bit was checked by rsadam@chromium.org

[commit-bot: I haz the power, 2014-10-29 02:54:50]

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

[commit-bot: I haz the power, 2014-10-29 06:13:58]

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

[commit-bot: I haz the power, 2014-10-29 06:14:00]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[rsadam, 2014-10-29 17:20:30]

Made the VirtualKeyboardController ChromeOS only. The DeviceDataManager that
this class calls is not created on Windows which was causing the windows bots to
crash. Given that we never use our virtual keyboard on Windows, made this class
chromeos only.

@Sky, PTANL!

[sky, 2014-10-29 19:16:50]

There is nothing really chromeos specific about this code, right? It's just you
don't want on windows? If that's the case leave files where they were and
exclude from necessary build files.

[rsadam, 2014-10-29 19:48:51]

Done!

[rsadam, 2014-10-29 20:42:22]

Fix typo. s/virtual_keyboard/virtual_keyboard_controller

[sky, 2014-10-29 21:12:48]

LGTM

[rsadam, 2014-10-29 22:05:59]

The CQ bit was checked by rsadam@chromium.org

[commit-bot: I haz the power, 2014-10-29 22:07:15]

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

[commit-bot: I haz the power, 2014-10-29 23:37:48]

Committed patchset #23 (id:460001)

[commit-bot: I haz the power, 2014-10-29 23:38:51]

Patchset 23 (id:??) landed as
https://crrev.com/9172bc8ad1c0514a1f0041a791dfbe9ef4549f36
Cr-Commit-Position: refs/heads/master@{#301977}