Moving IME manifests to chrome resources.

Moving IME manifests to chrome resources.

BUG=378348, 391778
TEST=Verified on sandbox, Pixel device. And made sure no test is broken.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285905

[Shu Chen, 2014-07-14 04:48:31]

Nona/Nikita/Nico, can you please help to review this cl? Thanks

[Nikita (slow), 2014-07-14 05:29:29]

+Dmitry to review chrome/browser/resources/chromeos/*

+Alexander to review
chrome/browser/ui/webui/options/chromeos/*
chrome/browser/ui/webui/chromeos/login/*

[Shu Chen, 2014-07-14 05:33:07]

nona@ for IMF related changes.

nkostylev@/dpolukhin@ for other changes under:
 - chrome/browser/chromeos
 - chrome/browser/resources/chromeos
 - chrome/browser/ui/webui

thakis@ for changes in chrome/browser/browser_resources.grd.

Thanks!
Shu

[nona, 2014-07-14 06:03:49]

Cool refactoring!

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:34:
struct WhitelistedComponentExtensionIME {
Do we still needs this whitelist?
We can choose the correct manifest with resource gyp rule.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/input_method/cangjie_manifest.json
(right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/cangjie_manifest.json:2: "name":
"Chrome OS IME (Cangjie)",
Is this json only for Chromium OS, right?
Could you add the comment into "description"?

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/cangjie_manifest.json:6:
"http://*/",
Can we narrow down the URL instead of using wild card.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/input_method/mozc_manifest.json (right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/mozc_manifest.json:3: "version":
"@MAJOR@.@MINOR@.@BUILD@.@REVISION@",
Is this value is expected? Probably this is not a problem, but just in case.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/input_method/pinyin_manifest.json
(right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/pinyin_manifest.json:6:
"http://*/",
Can we narrow down this URL?

[nona, 2014-07-14 06:46:08]

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(left):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:32:
const char* path;
Can we move this entry into corresponding manifest.json?

[Shu Chen, 2014-07-14 08:53:45]

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(left):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:32:
const char* path;
On 2014/07/14 06:46:07, nona wrote:
> Can we move this entry into corresponding manifest.json?

Done.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/chromeos/inpu...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:34:
struct WhitelistedComponentExtensionIME {
On 2014/07/14 06:03:48, nona wrote:
> Do we still needs this whitelist?
> We can choose the correct manifest with resource gyp rule.

Removed #ifdef.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/input_method/cangjie_manifest.json
(right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/cangjie_manifest.json:2: "name":
"Chrome OS IME (Cangjie)",
On 2014/07/14 06:03:48, nona wrote:
> Is this json only for Chromium OS, right?
> Could you add the comment into "description"?

Done.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/cangjie_manifest.json:6:
"http://*/",
On 2014/07/14 06:03:48, nona wrote:
> Can we narrow down the URL instead of using wild card.

Done.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/input_method/mozc_manifest.json (right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/mozc_manifest.json:3: "version":
"@MAJOR@.@MINOR@.@BUILD@.@REVISION@",
On 2014/07/14 06:03:48, nona wrote:
> Is this value is expected? Probably this is not a problem, but just in case.

Done.

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/input_method/pinyin_manifest.json
(right):

https://codereview.chromium.org/389913002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/input_method/pinyin_manifest.json:6:
"http://*/",
On 2014/07/14 06:03:48, nona wrote:
> Can we narrow down this URL?

Done.

[nona, 2014-07-14 10:02:47]

almost lgtm, please address my few comments :)

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:324:
out->display_name = l10n_util::GetStringUTF8(p->resource_id);
nit: How about adding DCHECK for preventing shipping un-translated strings?

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:780:
OnComponentExtensionInitialized(delegate.Pass());
You no longer need to split functions. How about removing
OnComponentExtensionInitialized?

[Alexander Alekseev, 2014-07-14 14:07:29]

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h
(right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h:43:
// file thread.
Is this comment still valid: "must be called on file thread" ?

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.h (right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.h:206:
std::string pending_input_method_;
Unused?

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.h (right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.h:115: bool
should_reinitialize_language_keyboard_list_;
Unused?

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:503:
if (composition_extension_appended_)
It seems that IME is now accessed in GetLocalizedValues(), so
composition_extension_appended_ should be always true here.

It seems that "composition_extension_appended_" can also be dropped here. (And
JS "options.LanguageOptions.onComponentManagerInitialized").

[Shu Chen, 2014-07-14 15:07:21]

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:324:
out->display_name = l10n_util::GetStringUTF8(p->resource_id);
On 2014/07/14 10:02:47, nona wrote:
> nit: How about adding DCHECK for preventing shipping un-translated strings?

Done.

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h
(right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h:43:
// file thread.
On 2014/07/14 14:07:29, alemate wrote:
> Is this comment still valid: "must be called on file thread" ?

Done.

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:780:
OnComponentExtensionInitialized(delegate.Pass());
On 2014/07/14 10:02:47, nona wrote:
> You no longer need to split functions. How about removing
> OnComponentExtensionInitialized?

Done.

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.h (right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.h:206:
std::string pending_input_method_;
On 2014/07/14 14:07:29, alemate wrote:
> Unused?

Done.

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.h (right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.h:115: bool
should_reinitialize_language_keyboard_list_;
On 2014/07/14 14:07:29, alemate wrote:
> Unused?

Done.

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/389913002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:503:
if (composition_extension_appended_)
On 2014/07/14 14:07:29, alemate wrote:
> It seems that IME is now accessed in GetLocalizedValues(), so
> composition_extension_appended_ should be always true here.
> 
> It seems that "composition_extension_appended_" can also be dropped here. (And
> JS "options.LanguageOptions.onComponentManagerInitialized").

Done.

[Alexander Alekseev, 2014-07-14 15:21:11]

lgtm

Thank you for this refactoring!

[Dmitry Polukhin, 2014-07-15 15:13:31]

https://codereview.chromium.org/389913002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/389913002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:635:
input_method::InputMethodManager::Get()->InitializeComponentExtension();
As far as I understand it is the only place where component extension is loaded.
It should work inside user session but how it works on login screen? Perhaps it
should be called from ComponentLoader to make sure that it is loaded always when
needed.

[Shu Chen, 2014-07-15 15:52:32]

https://codereview.chromium.org/389913002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/389913002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:635:
input_method::InputMethodManager::Get()->InitializeComponentExtension();
On 2014/07/15 15:13:31, Dmitry Polukhin wrote:
> As far as I understand it is the only place where component extension is
loaded.
> It should work inside user session but how it works on login screen? Perhaps
it
> should be called from ComponentLoader to make sure that it is loaded always
when
> needed.

Done.

[Shu Chen, 2014-07-16 01:06:00]

Pinging thakis@, changes in browser_resources.grd and component_loader.cc
requires your lgtm. Thanks

[Shu Chen, 2014-07-16 01:25:41]

+xiyuan@ to see whether the cl is safe for Kiosk.

[Dmitry Polukhin, 2014-07-16 07:11:35]

lgtm

https://codereview.chromium.org/389913002/diff/140001/chrome/browser/extensio...
File chrome/browser/extensions/component_loader.cc (right):

https://codereview.chromium.org/389913002/diff/140001/chrome/browser/extensio...
chrome/browser/extensions/component_loader.cc:432:
input_method_manager->InitializeComponentExtension();
FYI, Per comment above if IME component extensions have background page they
should be registered in different place to don't interfere with tests.

[Shu Chen, 2014-07-16 14:25:41]

https://codereview.chromium.org/389913002/diff/140001/chrome/browser/extensio...
File chrome/browser/extensions/component_loader.cc (right):

https://codereview.chromium.org/389913002/diff/140001/chrome/browser/extensio...
chrome/browser/extensions/component_loader.cc:432:
input_method_manager->InitializeComponentExtension();
On 2014/07/16 07:11:34, Dmitry Polukhin wrote:
> FYI, Per comment above if IME component extensions have background page they
> should be registered in different place to don't interfere with tests.

For test or linux_chromeos, there is no IME extension installed on disk. So IMF
will only parse the manifest for input method list, and it won't load any
extensions (thus, no background JS will run).
But on real device, there will be IME extensions installed on disk.
Should I move this to AddDefaultComponentExtensionsWithBackgroundPages()?

thakis@, since you're ONWER for component_loader, what's your opinion about it?

[xiyuan, 2014-07-16 17:33:46]

On 2014/07/16 01:25:41, Shu Chen wrote:
> +xiyuan@ to see whether the cl is safe for Kiosk

Keyboard works under kiosk mode. LGTM.

[Alexander Alekseev, 2014-07-22 16:49:09]

FYI: This CL will likely have a merge conflict with
https://codereview.chromium.org/397723002/ .

[Nico, 2014-07-22 17:05:08]

I don't understand the big picture here, I trust that other reviewers looked at
this. The new code looks much simpler to me, so that's nice.

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:706:
input_method::Shutdown();
(nit: as a verb, I think it's two words: "shut down", so "ShutDown" is
technically more correct)

(also, is this shutdown work necessary? does it do anything interesting? If it
just frees some pointers, that'll happen on process exit anyway…)

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:74:
const InputMethodNameMap& m2) const {
Is this for lower bound? Shouldn't this be operator< instead? Then you wouldn't
have to pass the 4th argument to lower_bound

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:229:
const char* kImePathKeyName = "ime_path";
Make this `const char kImePathKeyName[]`, else the pointer itself isn't const
and this might need initialization work

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/extensio...
File chrome/browser/extensions/component_loader.cc (right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/extensio...
chrome/browser/extensions/component_loader.cc:475:
chromeos::input_method::InputMethodManager* input_method_manager =
Does this need an #if defined(OS_CHROMEOS) block?

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/resource...
File chrome/browser/resources/chromeos/input_method/OWNERS (right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/resource...
chrome/browser/resources/chromeos/input_method/OWNERS:5: 
nit: no empty last line

https://codereview.chromium.org/389913002/diff/180001/chromeos/ime/ime_keyboa...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/389913002/diff/180001/chromeos/ime/ime_keyboa...
chromeos/ime/ime_keyboard_x11.cc:198: DisableNumLock();
This looks like it's either part of a different CL or an unintentional change.

[Shu Chen, 2014-07-23 00:56:45]

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:706:
input_method::Shutdown();
For the naming, all the shutdown methods around are named as "Shutdown" instead
of "ShutDown". I guess it'd be better to just follow it.

For the necessity, originally it's in ~DBusServices(), so I think it'd be better
to not change the behavior.

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:74:
const InputMethodNameMap& m2) const {
On 2014/07/22 17:05:08, Nico (away) wrote:
> Is this for lower bound? Shouldn't this be operator< instead? Then you
wouldn't
> have to pass the 4th argument to lower_bound

Done.

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:229:
const char* kImePathKeyName = "ime_path";
On 2014/07/22 17:05:08, Nico (away) wrote:
> Make this `const char kImePathKeyName[]`, else the pointer itself isn't const
> and this might need initialization work

Done.

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/extensio...
File chrome/browser/extensions/component_loader.cc (right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/extensio...
chrome/browser/extensions/component_loader.cc:475:
chromeos::input_method::InputMethodManager* input_method_manager =
AddDefaultComponentExtensionsForKioskMode is called only when #if
defined(OS_CHROMEOS).

But to make it safer, I've added the #if defined(OS_CHROMEOS) block. Thanks

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/resource...
File chrome/browser/resources/chromeos/input_method/OWNERS (right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/resource...
chrome/browser/resources/chromeos/input_method/OWNERS:5: 
On 2014/07/22 17:05:08, Nico (away) wrote:
> nit: no empty last line

Done.

https://codereview.chromium.org/389913002/diff/180001/chromeos/ime/ime_keyboa...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/389913002/diff/180001/chromeos/ime/ime_keyboa...
chromeos/ime/ime_keyboard_x11.cc:198: DisableNumLock();
On 2014/07/22 17:05:08, Nico (away) wrote:
> This looks like it's either part of a different CL or an unintentional change.

This is moved from line 191 of original chrome_browser_main_chromeos.cc.

[Nico, 2014-07-23 01:03:11]

lgtm

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/extensio...
File chrome/browser/extensions/component_loader.cc (right):

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/extensio...
chrome/browser/extensions/component_loader.cc:475:
chromeos::input_method::InputMethodManager* input_method_manager =
On 2014/07/23 00:56:45, Shu Chen wrote:
> AddDefaultComponentExtensionsForKioskMode is called only when #if
> defined(OS_CHROMEOS).
> 
> But to make it safer, I've added the #if defined(OS_CHROMEOS) block. Thanks

Oh ok, if this is only called on cros either, then as it was before is fine too.
Up to you.

https://codereview.chromium.org/389913002/diff/180001/chrome/browser/extensio...
chrome/browser/extensions/component_loader.cc:475:
chromeos::input_method::InputMethodManager* input_method_manager =
On 2014/07/23 00:56:45, Shu Chen wrote:
> AddDefaultComponentExtensionsForKioskMode is called only when #if
> defined(OS_CHROMEOS).
> 
> But to make it safer, I've added the #if defined(OS_CHROMEOS) block. Thanks

Oh ok, if this is only called on cros either, then as it was before is fine too.
Up to you.

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/chromeos...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:316:
if (p != map + map_size && !(*p < map_key) && !(map_key < *p))
nit:
  if (p != map + map_size && name == p->message_name)
is both shorter and easier to understand i think

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/resource...
File chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json
(right):

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/resource...
chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json:18:
"background" : {
(all the other keys in this file don't have a space in front of the : fwiw)

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/resource...
chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json:18:
"background" : {
(all the other keys in this file don't have a space in front of the : fwiw)

[Shu Chen, 2014-07-23 03:05:55]

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/chromeos...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:316:
if (p != map + map_size && !(*p < map_key) && !(map_key < *p))
On 2014/07/23 01:03:10, Nico (away) wrote:
> nit:
>   if (p != map + map_size && name == p->message_name)
> is both shorter and easier to understand i think

Done. Thanks for the good idea. :)

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/resource...
File chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json
(right):

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/resource...
chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json:18:
"background" : {
On 2014/07/23 01:03:11, Nico (away) wrote:
> (all the other keys in this file don't have a space in front of the : fwiw)

Done.

https://codereview.chromium.org/389913002/diff/200001/chrome/browser/resource...
chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json:18:
"background" : {
On 2014/07/23 01:03:11, Nico (away) wrote:
> (all the other keys in this file don't have a space in front of the : fwiw)

Done.

[Shu Chen, 2014-07-28 08:12:11]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2014-07-28 08:12:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shuchen@chromium.org/389913002/300001

[Shu Chen, 2014-07-28 08:13:10]

The CQ bit was unchecked by shuchen@chromium.org

[Yuki, 2014-07-28 08:37:55]

On 2014/07/14 10:02:47, nona wrote:
> almost lgtm, please address my few comments :)
> 
>
https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
> File
> chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
> (right):
> 
>
https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
>
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:324:
> out->display_name = l10n_util::GetStringUTF8(p->resource_id);
> nit: How about adding DCHECK for preventing shipping un-translated strings?
> 
>
https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
> File chrome/browser/chromeos/input_method/input_method_manager_impl.cc
(right):
> 
>
https://codereview.chromium.org/389913002/diff/40001/chrome/browser/chromeos/...
> chrome/browser/chromeos/input_method/input_method_manager_impl.cc:780:
> OnComponentExtensionInitialized(delegate.Pass());
> You no longer need to split functions. How about removing
> OnComponentExtensionInitialized?

[Yuki, 2014-07-28 08:39:03]

lgtm on behalf of nona.
This is a rubber stamp lgtm.

[Shu Chen, 2014-07-28 08:40:04]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2014-07-28 08:40:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shuchen@chromium.org/389913002/300001

[commit-bot: I haz the power, 2014-07-28 14:39:47]

Change committed as 285905

[mtomasz, 2014-07-31 02:20:18]

https://codereview.chromium.org/389913002/diff/300001/chrome/browser/chromeos...
File
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
(right):

https://codereview.chromium.org/389913002/diff/300001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:258:
if (!base::PathExists(file_path))
This crashes Debug build, because of thread restrictions.

[4092:4092:0731/103038:FATAL:thread_restrictions.cc(38)] Function marked as
IO-only was called from a thread that disallows IO!  If this thread really
should be allowed to make IO calls, adjust the call to
base::ThreadRestrictions::SetIOAllowed() in this thread's startup.

#0  base::debug::(anonymous namespace)::DebugBreak () at
../../base/debug/debugger_posix.cc:228
#1  0x00000000019a85f9 in base::debug::BreakDebugger () at
../../base/debug/debugger_posix.cc:241
#2  0x0000000001a1f772 in logging::LogMessage::~LogMessage (this=0x7fffffff96c8)
at ../../base/logging.cc:644
#3  0x0000000001ad879f in base::ThreadRestrictions::AssertIOAllowed () at
../../base/threading/thread_restrictions.cc:38
#4  0x00000000019e6c01 in base::PathExists (path=...) at
../../base/file_util_posix.cc:350
#5  0x0000000008a9d854 in chromeos::ComponentExtensionIMEManagerImpl::Load
(this=0x3e841b5cd920, 
    extension_id="fgoepimhcoialccpbmpnnblemnepkkao", 
    manifest="{\n  \"key\":
\"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsVpj8qC5VhObWORqQ3odZm+z5Vf58jWtqG2dmWkG87gsS1ZuYm4jA7mFZItOxgMENpYYWla8HCs21W6hEGoydDiIEAEVaWMTzCw19/x0m/LMeNIBtuaZ/IJQvFJuo1KHvmHX0lWxsHZgUfwp"...,
file_path=...)
    at
../../chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:258