Supports setting non-login input methods in VPD.

Supports setting non-login input methods in VPD.

BUG=chrome-os-partner:48623
TEST=Set the "keyboard_layout" value as "ime:ko:hangul" in VPD; Goes to
OOBE screen, set the locale as Korean; Goes to Guest mode, verify the
Korean input method is the default (and the only one) input method.

Committed: https://crrev.com/2fc63c2c015a7a4f0842e5dd4603791b0413f9e3
Cr-Commit-Position: refs/heads/master@{#369358}

[Shu Chen, 2016-01-12 14:04:22]

shuchen@chromium.org changed reviewers:
+ alemate@chromium.org

[Shu Chen, 2016-01-12 14:04:22]

Alexender, can you please review this cl? Thanks

[Alexander Alekseev, 2016-01-12 14:37:40]

https://codereview.chromium.org/1578133003/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/1578133003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:177: descriptor =
manager->GetInputMethodUtil()->GetInputMethodDescriptorFromId(
Nit: Could this be a local variable?
We don't need this |descriptor| outside of this block.
(This way we don't need new line #160.)

https://codereview.chromium.org/1578133003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:181:
->GetCurrentInputMethod();
This alters existing behavior for new users: new user gets input method matching
the one that was used on GaiaScreen when adding this user.
(I.e. a person selects some input method on Gaia screen, creates new Chrome OS
user and continues to use the same IM in the session).

But after this change, new user will get hardware default input method.

Could you support existing logic here?

[Shu Chen, 2016-01-13 00:36:56]

https://codereview.chromium.org/1578133003/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/1578133003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:177: descriptor =
manager->GetInputMethodUtil()->GetInputMethodDescriptorFromId(
On 2016/01/12 14:37:40, Alexander Alekseev wrote:
> Nit: Could this be a local variable?
> We don't need this |descriptor| outside of this block.
> (This way we don't need new line #160.)

Done.

https://codereview.chromium.org/1578133003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:181:
->GetCurrentInputMethod();
On 2016/01/12 14:37:40, Alexander Alekseev wrote:
> This alters existing behavior for new users: new user gets input method
matching
> the one that was used on GaiaScreen when adding this user.
> (I.e. a person selects some input method on Gaia screen, creates new Chrome OS
> user and continues to use the same IM in the session).
> 
> But after this change, new user will get hardware default input method.
> 
> Could you support existing logic here?

That is a problem. GetHardwareInputMethodIds() can return non-login input
methods, but GaiaScreen only contains login-able input methods.
How about a logic like this:

  if (hardware_input_method.GetPreferredKeyboardLayout() ==
      default_input_method.GetPreferredKeyboardLayout()) {
    <use hardware_input_method>;
  } else {
    <use default_input_method>;
  }

For now, only Korean will return the Korean input method as the first of the
hardware input methods.
And there would be edge cases like: choose "xkb:us::fil" at GaiaScreen, but the
first of the hardware input methods is "xkb:us::ind".
I think we can ignore that case because "xkb:us::fil" and "xkb:us::ind" are
basically the same input method and will cause duplicates in the input method
tray menu.
So the VPD cannot have both "xkb:us::fil" and "xkb:us::ind" at the same time.

[Shu Chen, 2016-01-13 02:16:51]

I've upload a new patch to reflect my idea.

Please take another look. Thanks

[Alexander Alekseev, 2016-01-14 01:33:29]

On 2016/01/13 02:16:51, Shu Chen wrote:
> I've upload a new patch to reflect my idea.
> 
> Please take another look. Thanks

Hmmm. I probably on't understand your idea.
1) For OOBE screen this CL should not enable displaying Korean keyboard in
dropdown "select" element with a list of available keyboards.
2) Login screen(s) are doing exactly the same filtering as OOBE, so it still
cannot be used there.
3) For user sessions, because of 1) and 2), you'll get:

preferred_input_method =
  session_manager->GetDefaultIMEState(profile)->GetCurrentInputMethod();

which actually means "US default keyboard" because of 2).
Do you mean that preferred_input_method.GetPreferredKeyboardLayout() for "US
default keyboard" will return the same  string as
descriptor->GetPreferredKeyboardLayout() for "Korean default keyboard" ?

[Shu Chen, 2016-01-14 01:44:44]

On 2016/01/14 01:33:29, Alexander Alekseev wrote:
> On 2016/01/13 02:16:51, Shu Chen wrote:
> > I've upload a new patch to reflect my idea.
> > 
> > Please take another look. Thanks
> 
> Hmmm. I probably on't understand your idea.
> 1) For OOBE screen this CL should not enable displaying Korean keyboard in
> dropdown "select" element with a list of available keyboards.
> 2) Login screen(s) are doing exactly the same filtering as OOBE, so it still
> cannot be used there.
> 3) For user sessions, because of 1) and 2), you'll get:
> 
> preferred_input_method =
>   session_manager->GetDefaultIMEState(profile)->GetCurrentInputMethod();
> 
> which actually means "US default keyboard" because of 2).
> Do you mean that preferred_input_method.GetPreferredKeyboardLayout() for "US
> default keyboard" will return the same  string as
> descriptor->GetPreferredKeyboardLayout() for "Korean default keyboard" ?

Yes, "Korean default keyboard"'s keyboard layout is "us" which is the same as
"US default keyboard".
Most of the non-login input methods are based on "us" layout.
The only one special case is one of Japanese input methods, which is based on
"jp" layout.
For that case, Japanese input method "nacl_mozc_jp" and Japanese keyboard
"xkb:jp::jpn" share the same "jp" layout.
So, if the VPD set as "ime:jp:mozc_jp" (maps to "nacl_mozc_jp"), in OOBE, user
can choose "xkb:jp::jpn".
But after the initial login (or guest login), the user will get "ime:jp:mozc_jp"
as default input method.

[Alexander Alekseev, 2016-01-14 02:27:13]

lgtm

[Alexander Alekseev, 2016-01-14 02:28:07]

On 2016/01/14 01:44:44, Shu Chen wrote:
> On 2016/01/14 01:33:29, Alexander Alekseev wrote:
> > On 2016/01/13 02:16:51, Shu Chen wrote:
> > > I've upload a new patch to reflect my idea.
> > > 
> > > Please take another look. Thanks
> > 
> > Hmmm. I probably on't understand your idea.
> > 1) For OOBE screen this CL should not enable displaying Korean keyboard in
> > dropdown "select" element with a list of available keyboards.
> > 2) Login screen(s) are doing exactly the same filtering as OOBE, so it still
> > cannot be used there.
> > 3) For user sessions, because of 1) and 2), you'll get:
> > 
> > preferred_input_method =
> >   session_manager->GetDefaultIMEState(profile)->GetCurrentInputMethod();
> > 
> > which actually means "US default keyboard" because of 2).
> > Do you mean that preferred_input_method.GetPreferredKeyboardLayout() for "US
> > default keyboard" will return the same  string as
> > descriptor->GetPreferredKeyboardLayout() for "Korean default keyboard" ?
> 
> Yes, "Korean default keyboard"'s keyboard layout is "us" which is the same as
> "US default keyboard".
> Most of the non-login input methods are based on "us" layout.
> The only one special case is one of Japanese input methods, which is based on
> "jp" layout.
> For that case, Japanese input method "nacl_mozc_jp" and Japanese keyboard
> "xkb:jp::jpn" share the same "jp" layout.
> So, if the VPD set as "ime:jp:mozc_jp" (maps to "nacl_mozc_jp"), in OOBE, user
> can choose "xkb:jp::jpn".
> But after the initial login (or guest login), the user will get
"ime:jp:mozc_jp"
> as default input method.

Thank you for explanation!

[Shu Chen, 2016-01-14 02:35:39]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2016-01-14 02:40:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1578133003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1578133003/40001

[commit-bot: I haz the power, 2016-01-14 03:55:14]

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

[commit-bot: I haz the power, 2016-01-14 03:55:14]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Shu Chen, 2016-01-14 04:08:44]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2016-01-14 04:09:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1578133003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1578133003/40001

[commit-bot: I haz the power, 2016-01-14 04:46:42]

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

[commit-bot: I haz the power, 2016-01-14 04:46:42]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_android on tryserver.chromium.android (JOB_TIMED_OUT, no build URL)

[Shu Chen, 2016-01-14 05:55:48]

The CQ bit was checked by shuchen@chromium.org

[Shu Chen, 2016-01-14 05:55:48]

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

[commit-bot: I haz the power, 2016-01-14 05:56:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1578133003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1578133003/60001

[commit-bot: I haz the power, 2016-01-14 07:15:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-14 07:16:54]

Description was changed from

==========
Supports setting non-login input methods in VPD.

BUG=chrome-os-partner:48623
TEST=Set the "keyboard_layout" value as "ime:ko:hangul" in VPD; Goes to
OOBE screen, set the locale as Korean; Goes to Guest mode, verify the
Korean input method is the default (and the only one) input method.
==========

to

==========
Supports setting non-login input methods in VPD.

BUG=chrome-os-partner:48623
TEST=Set the "keyboard_layout" value as "ime:ko:hangul" in VPD; Goes to
OOBE screen, set the locale as Korean; Goes to Guest mode, verify the
Korean input method is the default (and the only one) input method.

Committed: https://crrev.com/2fc63c2c015a7a4f0842e5dd4603791b0413f9e3
Cr-Commit-Position: refs/heads/master@{#369358}
==========

[commit-bot: I haz the power, 2016-01-14 07:16:55]

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