Add virtual keyboard item into opt-in IME menu.

Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle.
Currently the ImeMenuBubble doesn't support update UI. So if there's state change (which leads to UI change) of the VK, just hide the bubble.

BUG=668013
TEST=Verified on local build.

Committed: https://crrev.com/1fb9e7720d5a4ddaef6ce257f4d252a831bea688
Cr-Commit-Position: refs/heads/master@{#434127}

[Azure Wei, 2016-11-03 04:50:32]

Description was changed from

==========
Add virtual keyboard item into opt-in IME menu.

BUG=657146
TEST=Verified on local build.
==========

to

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=657146
TEST=Verified on local build.
==========

[Azure Wei, 2016-11-03 04:50:33]

azurewei@chromium.org changed reviewers:
+ tdanderson@chromium.org

[Azure Wei, 2016-11-03 04:50:47]

Please review this CL. Thanks!

[Azure Wei, 2016-11-03 07:39:35]

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

[commit-bot: I haz the power, 2016-11-03 07:39:53]

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-11-03 09:40:26]

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

[commit-bot: I haz the power, 2016-11-03 09:40:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[tdanderson, 2016-11-03 21:17:20]

https://codereview.chromium.org/2474843002/diff/1/ash/common/system/chromeos/...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2474843002/diff/1/ash/common/system/chromeos/...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:411: return
keyboard_suppressed_ &&
I left the following comment in
https://chromiumcodereview.appspot.com/2469663002/:

"I went into non-MD mode on canary, plugged in an external keyboard, and entered
touchview (maximize mode). When I go into the IME detailed view I see a "disable
on-screen keyboard" button, and when I tap it it turns into an "enable on-screen
keyboard" button. So the toggle row needs to be shown regardless of whether the
toggle state is on or off."

With that said, I don't think you are doing the right thing in this CL. To avoid
any confusion, let's iterate on and land
https://chromiumcodereview.appspot.com/2469663002/ first and then we can come
back to this CL.

[tdanderson, 2016-11-08 19:00:02]

On 2016/11/03 21:17:20, tdanderson wrote:
>
https://codereview.chromium.org/2474843002/diff/1/ash/common/system/chromeos/...
> File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):
> 
>
https://codereview.chromium.org/2474843002/diff/1/ash/common/system/chromeos/...
> ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:411: return
> keyboard_suppressed_ &&
> I left the following comment in
> https://chromiumcodereview.appspot.com/2469663002/:
> 
> "I went into non-MD mode on canary, plugged in an external keyboard, and
entered
> touchview (maximize mode). When I go into the IME detailed view I see a
"disable
> on-screen keyboard" button, and when I tap it it turns into an "enable
on-screen
> keyboard" button. So the toggle row needs to be shown regardless of whether
the
> toggle state is on or off."
> 
> With that said, I don't think you are doing the right thing in this CL. To
avoid
> any confusion, let's iterate on and land
> https://chromiumcodereview.appspot.com/2469663002/ first and then we can come
> back to this CL.

Hi, just a friendly ping on this - let me know when this is ready for me to look
at again.

[Azure Wei, 2016-11-20 07:46:39]

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

[commit-bot: I haz the power, 2016-11-20 07:46:47]

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-11-20 08:12:17]

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

[commit-bot: I haz the power, 2016-11-20 08:12:18]

Dry run: This issue passed the CQ dry run.

[Azure Wei, 2016-11-20 08:56:24]

Uploaded a new patch synced with the latest codes. Please take another look.
Thanks!

https://codereview.chromium.org/2474843002/diff/1/ash/common/system/chromeos/...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2474843002/diff/1/ash/common/system/chromeos/...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:411: return
keyboard_suppressed_ &&
On 2016/11/03 21:17:20, tdanderson wrote:
> I left the following comment in
> https://chromiumcodereview.appspot.com/2469663002/:
> 
> "I went into non-MD mode on canary, plugged in an external keyboard, and
entered
> touchview (maximize mode). When I go into the IME detailed view I see a
"disable
> on-screen keyboard" button, and when I tap it it turns into an "enable
on-screen
> keyboard" button. So the toggle row needs to be shown regardless of whether
the
> toggle state is on or off."
> 
> With that said, I don't think you are doing the right thing in this CL. To
avoid
> any confusion, let's iterate on and land
> https://chromiumcodereview.appspot.com/2469663002/ first and then we can come
> back to this CL.

The method return true when the 'smart deploy toggle' should be shown:
If the user is in TouchView (virtual keyboard should be automatically deployed
if IsVirtualKeyboardEnabled() is false) and an external keyboard attached
(keyboard_suppressed_ is true). 
If the keyboard toggle is shown, turn on or off the toggle should not change the
returned value of this method.

[tdanderson, 2016-11-23 03:08:41]

LGTM

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:555: if (suppressed !=
keyboard_suppressed_ && bubble_)
Is this something the IME detailed view OnKeyboardSuppressionChanged() should be
doing too? (Just curious, no need to address it in this CL.)

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.h (right):

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.h:2: // Use of this source
code is governed by a BSD-style license that can be
nit: can you file a new bug for this and use it as the BUG= number in the cl
description? I already requested a merge for another CL in 657146 and if we want
to merge this back to 56 it should have a separate bug number.

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.h:59: // Returns whether the
virtual keyboard toggle should be shown in the detailed
nit: "shown in the opt-in IME menu" rather than 'detailed view'

[Azure Wei, 2016-11-23 05:21:14]

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right):

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:555: if (suppressed !=
keyboard_suppressed_ && bubble_)
On 2016/11/23 03:08:40, tdanderson wrote:
> Is this something the IME detailed view OnKeyboardSuppressionChanged() should
be
> doing too? (Just curious, no need to address it in this CL.)

Yes, IME detailed view shares the same logic. I used to try to move this to
ImeListView so ImeMenuTray and TrayIME don't need to care the refreshing logic
for VK toggle. But it's hard to implement on ImeMenuTray since the height of
ImeMenuTray should be changed when the number of items changed. So I just put
the OnKeyboardSuppressionChanged() logic here instead of in ImeListView.

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.h (right):

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.h:59: // Returns whether the
virtual keyboard toggle should be shown in the detailed
On 2016/11/23 03:08:40, tdanderson wrote:
> nit: "shown in the opt-in IME menu" rather than 'detailed view'

Done.

[Azure Wei, 2016-11-23 05:21:20]

The CQ bit was checked by azurewei@chromium.org

[commit-bot: I haz the power, 2016-11-23 05:21:29]

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

[Azure Wei, 2016-11-23 05:27:07]

Description was changed from

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=657146
TEST=Verified on local build.
==========

to

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=657146, 668013
TEST=Verified on local build.
==========

[Azure Wei, 2016-11-23 05:27:33]

Description was changed from

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=657146, 668013
TEST=Verified on local build.
==========

to

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=668013
TEST=Verified on local build.
==========

[Azure Wei, 2016-11-23 05:28:28]

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/ime_menu/ime_menu_tray.h (right):

https://codereview.chromium.org/2474843002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/ime_menu/ime_menu_tray.h:2: // Use of this source
code is governed by a BSD-style license that can be
On 2016/11/23 03:08:40, tdanderson wrote:
> nit: can you file a new bug for this and use it as the BUG= number in the cl
> description? I already requested a merge for another CL in 657146 and if we
want
> to merge this back to 56 it should have a separate bug number.

Done. Created bug:668013 and updated the CL description. Thanks for the advice.

[Azure Wei, 2016-11-23 05:28:37]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-11-23 05:28:38]

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

[commit-bot: I haz the power, 2016-11-23 05:28:55]

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-11-23 06:29:14]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1479878917724950,
"parent_rev": "30edfe31a76250dc38a3bbd615ef417f7f823f49", "commit_rev":
"b5545a50489440495899184a58d057a4b6eff736"}

[commit-bot: I haz the power, 2016-11-23 06:29:53]

Description was changed from

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=668013
TEST=Verified on local build.
==========

to

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=668013
TEST=Verified on local build.
==========

[commit-bot: I haz the power, 2016-11-23 06:29:54]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-23 06:31:25]

Description was changed from

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle. 
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=668013
TEST=Verified on local build.
==========

to

==========
Add virtual keyboard item into opt-in IME menu.

The 'Smart deploy toggle' is always hidden in opt-in IME menu. We should
show/hide it based on the vr and devices state.
Make ImeMenuTray listens on the virtual keyboard state to show the toggle.
Currently the ImeMenuBubble doesn't support update UI. So if there's state
change (which leads to UI change) of the VK, just hide the bubble.

BUG=668013
TEST=Verified on local build.

Committed: https://crrev.com/1fb9e7720d5a4ddaef6ce257f4d252a831bea688
Cr-Commit-Position: refs/heads/master@{#434127}
==========

[commit-bot: I haz the power, 2016-11-23 06:31:26]

Patchset 4 (id:??) landed as
https://crrev.com/1fb9e7720d5a4ddaef6ce257f4d252a831bea688
Cr-Commit-Position: refs/heads/master@{#434127}