Updated the VK Tray button to properly remove visual activated state.

Updated the VK Tray button to properly remove visual activated state.

The VirtualKeyboardTray button was attaching to a KeyboardController instance
that was being replaced without attaching itself as an observer to the new one.

BUG=726273
TEST=manual

Review-Url: https://codereview.chromium.org/2916653002
Cr-Commit-Position: refs/heads/master@{#476285}
Committed: https://chromium.googlesource.com/chromium/src/+/ce80702d421f8e1626e99d23dd54d83aec979b78

[bruthig, 2017-05-31 17:54:40]

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

[commit-bot: I haz the power, 2017-05-31 17:55:04]

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-31 18:45:58]

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

[commit-bot: I haz the power, 2017-05-31 18:45:59]

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

[bruthig, 2017-05-31 21:03:31]

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

[commit-bot: I haz the power, 2017-05-31 21:04:46]

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-31 21:54:03]

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

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

Dry run: This issue passed the CQ dry run.

[bruthig, 2017-05-31 21:55:41]

bruthig@chromium.org changed reviewers:
+ jamescook@chromium.org

[bruthig, 2017-05-31 21:55:41]

James, can you PTAL?

[James Cook, 2017-05-31 23:44:26]

LGTM.

Do you know who wrote the KeyboardController code? It seems really odd that we
create and destroy controller instances. I think everything might be simpler if
we always created the controller and then built/destroyed the ui views as
needed. But that's a refactor the author/owner should do, not you.

https://codereview.chromium.org/2916653002/diff/20001/ash/system/virtual_keyb...
File ash/system/virtual_keyboard/virtual_keyboard_tray.cc (right):

https://codereview.chromium.org/2916653002/diff/20001/ash/system/virtual_keyb...
ash/system/virtual_keyboard/virtual_keyboard_tray.cc:56:
Shell::Get()->RemoveShellObserver(this);
super-nit: do this one line above so things are removed in the opposite order to
how they were added

[bruthig, 2017-06-01 14:05:15]

On 2017/05/31 23:44:26, James Cook wrote:
> LGTM.
> 
> Do you know who wrote the KeyboardController code? It seems really odd that we
> create and destroy controller instances. I think everything might be simpler
if
> we always created the controller and then built/destroyed the ui views as
> needed. But that's a refactor the author/owner should do, not you.

bshe@ is OOO until Monday and then he was going to take a look at this issue.  I
will make sure he's aware of this point.

https://codereview.chromium.org/2916653002/diff/20001/ash/system/virtual_keyb...
File ash/system/virtual_keyboard/virtual_keyboard_tray.cc (right):

https://codereview.chromium.org/2916653002/diff/20001/ash/system/virtual_keyb...
ash/system/virtual_keyboard/virtual_keyboard_tray.cc:56:
Shell::Get()->RemoveShellObserver(this);
On 2017/05/31 23:44:25, James Cook wrote:
> super-nit: do this one line above so things are removed in the opposite order
to
> how they were added

Done.

[bruthig, 2017-06-01 14:05:19]

The CQ bit was checked by bruthig@chromium.org

[bruthig, 2017-06-01 14:05:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2916653002/#ps40001
(title: "Reordered observer detaching.")

[commit-bot: I haz the power, 2017-06-01 14:05:39]

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-06-01 14:48:04]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496325919328270,
"parent_rev": "1359d99e8b759c08e9dff8e934e002e75ca6f0ac", "commit_rev":
"ce80702d421f8e1626e99d23dd54d83aec979b78"}

[commit-bot: I haz the power, 2017-06-01 14:48:11]

Description was changed from

==========
Updated the VK Tray button to properly remove visual activated state.

The VirtualKeyboardTray button was attaching to a KeyboardController instance
that was being replaced without attaching itself as an observer to the new one.

BUG=726273
TEST=manual
==========

to

==========
Updated the VK Tray button to properly remove visual activated state.

The VirtualKeyboardTray button was attaching to a KeyboardController instance
that was being replaced without attaching itself as an observer to the new one.

BUG=726273
TEST=manual

Review-Url: https://codereview.chromium.org/2916653002
Cr-Commit-Position: refs/heads/master@{#476285}
Committed:
https://chromium.googlesource.com/chromium/src/+/ce80702d421f8e1626e99d23dd54...
==========

[commit-bot: I haz the power, 2017-06-01 14:48:12]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/ce80702d421f8e1626e99d23dd54...

[bshe, 2017-06-08 17:38:32]

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

[bshe, 2017-06-08 17:38:32]

+cc yhanada who is still working on keyboard related code

[yhanada, 2017-06-09 05:56:23]

On 2017/06/08 17:38:32, bshe wrote:
> +cc yhanada who is still working on keyboard related code

Filed crbug.com/731537.