Add chrome.inputMethodPrivate.onImeMenuActivationChanged API.

Add chrome.inputMethodPrivate.onImeMenuActivationChanged API, which will be fired when the checkbox in language settings about IME menu is checked or unchecked.

Modify chrome.inputMethodPrivate.getInputMethodConfig() API to get the initial state of the activation of IME menu.

These APIs will be used for IME extensions to opt-in the IME menu on shelf.

BUG=570761
TEST=Verified the API is available on Chrome OS.

Committed: https://crrev.com/f7a97e05428cb68d7b77e6d04f925772f252f4be
Cr-Commit-Position: refs/heads/master@{#373479}

[Azure Wei, 2016-01-04 07:06:52]

Description was changed from

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API.

BUG=570761
TEST=Verified the API is available on Chrome OS.
==========

to

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API.
This API will be used for IME extensions to opt-in the IME menu on shelf. 

BUG=570761
TEST=Verified the API is available on Chrome OS.
==========

[Azure Wei, 2016-01-04 07:06:52]

azurewei@chromium.org changed reviewers:
+ isherman@chromium.org, michaelpg@chromium.org, shuchen@chromium.org,
stevenjb@chromium.org

[Azure Wei, 2016-01-04 07:10:00]

Hi stevenjb, Ilya Sherman, Shu Chen and michaelpg, 
this cl used to be part of: https://codereview.chromium.org/1523593002/, can you
please review it? Thank you!

[stevenjb, 2016-01-04 18:21:54]

How will the JS get the initial state for IME menu activation?

You will need an extensions/ owner to review this, e.g. rdevlin.cronin@

[Azure Wei, 2016-01-05 07:40:27]

azurewei@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Azure Wei, 2016-01-05 10:35:50]

On 2016/01/04 18:21:54, stevenjb wrote:
> How will the JS get the initial state for IME menu activation?

This event will be fired when the preference pref:kLangugaeImeMenuActivated
changes. pref:kLangugaeImeMenuActivated is registered as false, and then will be
initialized with user prefs. So the JS will get the event
onImeMenuActivationChanged(true) if the initial state is true, and won't get any
event if it's false. Since the IME menu will only be activated on event
onImeMenuActivationChanged(true), I think it's OK that the JS doesn't get the
initial false state. Does this make sense? I haven't implemented this mechanism
yet, I hope I have not got it wrong.

> You will need an extensions/ owner to review this, e.g. rdevlin.cronin@

+rdevlin.cronin@ as reviewer.
Hi Devlin, can you help review this cl? Thank you!

[stevenjb, 2016-01-05 17:26:36]

On 2016/01/05 10:35:50, Azure Wei wrote:
> On 2016/01/04 18:21:54, stevenjb wrote:
> > How will the JS get the initial state for IME menu activation?
> 
> This event will be fired when the preference pref:kLangugaeImeMenuActivated
> changes. pref:kLangugaeImeMenuActivated is registered as false, and then will
be
> initialized with user prefs. So the JS will get the event
> onImeMenuActivationChanged(true) if the initial state is true, and won't get
any
> event if it's false. Since the IME menu will only be activated on event
> onImeMenuActivationChanged(true), I think it's OK that the JS doesn't get the
> initial false state. Does this make sense? I haven't implemented this
mechanism
> yet, I hope I have not got it wrong.
> 
> > You will need an extensions/ owner to review this, e.g. rdevlin.cronin@
> 
> +rdevlin.cronin@ as reviewer.
> Hi Devlin, can you help review this cl? Thank you!

An extension may (will likely) get loaded after the pref has been registered, so
the extension may not receive an event for the initial state. i.e. after the
user has set the pref then restarts the browser, the extension will likely get
loaded after the pref (now true) is loaded.

[Devlin, 2016-01-05 18:50:02]

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/ime_menu_event_router.cc (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.cc:1: // Copyright 2015
The Chromium Authors. All rights reserved.
year

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/ime_menu_event_router.h (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:1: // Copyright 2015
The Chromium Authors. All rights reserved.
not 2015

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:8: #include
"base/compiler_specific.h"
needed?

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:24: // Implements
input_method::InputMethodManager::ImeMenuObserver:
Lately, the pattern has been
// InheritedClass:
void InheritedMethod() override;

So, here:
// input_method::InputMethodManager::ImeMenuObserver:
void ImeMenuActivationChanged(bool activation) override;

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:30:
DISALLOW_COPY_AND_ASSIGN(ExtensionImeMenuEventRouter);
base/macros

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:33: }  // namespace
chromeos
\n

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:192:
"inputMethodPrivate.onImeMenuActivationChanged";
... Why doesn't inputMethodPrivate use the auto-generated files?

[Azure Wei, 2016-01-06 11:44:16]

On 2016/01/05 17:26:36, stevenjb wrote:
> On 2016/01/05 10:35:50, Azure Wei wrote:
> > On 2016/01/04 18:21:54, stevenjb wrote:
> > > How will the JS get the initial state for IME menu activation?
> > 
> > This event will be fired when the preference pref:kLangugaeImeMenuActivated
> > changes. pref:kLangugaeImeMenuActivated is registered as false, and then
will
> be
> > initialized with user prefs. So the JS will get the event
> > onImeMenuActivationChanged(true) if the initial state is true, and won't get
> any
> > event if it's false. Since the IME menu will only be activated on event
> > onImeMenuActivationChanged(true), I think it's OK that the JS doesn't get
the
> > initial false state. Does this make sense? I haven't implemented this
> mechanism
> > yet, I hope I have not got it wrong.
> > 
> > > You will need an extensions/ owner to review this, e.g. rdevlin.cronin@
> > 
> > +rdevlin.cronin@ as reviewer.
> > Hi Devlin, can you help review this cl? Thank you!
> 
> An extension may (will likely) get loaded after the pref has been registered,
so
> the extension may not receive an event for the initial state. i.e. after the
> user has set the pref then restarts the browser, the extension will likely get
> loaded after the pref (now true) is loaded.

How about put the  initial state in inputMethodPrivate.getInputMethodConfig()
function, or add a new API?

[Azure Wei, 2016-01-06 11:50:44]

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/ime_menu_event_router.cc (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.cc:1: // Copyright 2015
The Chromium Authors. All rights reserved.
On 2016/01/05 18:50:02, Devlin wrote:
> year

Done.

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/ime_menu_event_router.h (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:1: // Copyright 2015
The Chromium Authors. All rights reserved.
On 2016/01/05 18:50:02, Devlin wrote:
> not 2015

Done.

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:8: #include
"base/compiler_specific.h"
On 2016/01/05 18:50:02, Devlin wrote:
> needed?

Deleted.

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:24: // Implements
input_method::InputMethodManager::ImeMenuObserver:
On 2016/01/05 18:50:02, Devlin wrote:
> Lately, the pattern has been
> // InheritedClass:
> void InheritedMethod() override;
> 
> So, here:
> // input_method::InputMethodManager::ImeMenuObserver:
> void ImeMenuActivationChanged(bool activation) override;

Done.

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:30:
DISALLOW_COPY_AND_ASSIGN(ExtensionImeMenuEventRouter);
On 2016/01/05 18:50:02, Devlin wrote:
> base/macros

Done.

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:33: }  // namespace
chromeos
On 2016/01/05 18:50:02, Devlin wrote:
> \n

Done.

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:192:
"inputMethodPrivate.onImeMenuActivationChanged";
On 2016/01/05 18:50:02, Devlin wrote:
> ... Why doesn't inputMethodPrivate use the auto-generated files?

There's no public API in chrome.inputMethodPrivate.*. input_method_private.json
is not listed in schemas.gypi, so there's no generated input_method_private.h
file.

[Ilya Sherman, 2016-01-06 18:44:56]

histograms.xml lgtm

[Shu Chen, 2016-01-06 21:15:44]

https://codereview.chromium.org/1552743003/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/ime_menu_event_router.cc (right):

https://codereview.chromium.org/1552743003/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.cc:28:
extensions::InputMethodAPI::kOnImeMenuActivationChanged)) {
indent.

https://codereview.chromium.org/1552743003/diff/60001/ui/base/ime/chromeos/in...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/60001/ui/base/ime/chromeos/in...
ui/base/ime/chromeos/input_method_manager.h:72: // ImeMenuObserver is notified
of events related to the IME menu on the system
s/system tray/shelf bar

https://codereview.chromium.org/1552743003/diff/60001/ui/base/ime/chromeos/in...
ui/base/ime/chromeos/input_method_manager.h:78: // indicates whether the user
should be notified of this change.
wrong comment here.

[stevenjb, 2016-01-06 21:31:34]

On 2016/01/06 11:44:16, Azure Wei wrote:
> On 2016/01/05 17:26:36, stevenjb wrote:
> > On 2016/01/05 10:35:50, Azure Wei wrote:
> > > On 2016/01/04 18:21:54, stevenjb wrote:
> > > > How will the JS get the initial state for IME menu activation?
> > > 
> > > This event will be fired when the preference
pref:kLangugaeImeMenuActivated
> > > changes. pref:kLangugaeImeMenuActivated is registered as false, and then
> will
> > be
> > > initialized with user prefs. So the JS will get the event
> > > onImeMenuActivationChanged(true) if the initial state is true, and won't
get
> > any
> > > event if it's false. Since the IME menu will only be activated on event
> > > onImeMenuActivationChanged(true), I think it's OK that the JS doesn't get
> the
> > > initial false state. Does this make sense? I haven't implemented this
> > mechanism
> > > yet, I hope I have not got it wrong.
> > > 
> > > > You will need an extensions/ owner to review this, e.g. rdevlin.cronin@
> > > 
> > > +rdevlin.cronin@ as reviewer.
> > > Hi Devlin, can you help review this cl? Thank you!
> > 
> > An extension may (will likely) get loaded after the pref has been
registered,
> so
> > the extension may not receive an event for the initial state. i.e. after the
> > user has set the pref then restarts the browser, the extension will likely
get
> > loaded after the pref (now true) is loaded.
> 
> How about put the  initial state in inputMethodPrivate.getInputMethodConfig()
> function, or add a new API?

That seems reasonable to me.

[Azure Wei, 2016-01-07 01:59:31]

https://codereview.chromium.org/1552743003/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/ime_menu_event_router.cc (right):

https://codereview.chromium.org/1552743003/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/ime_menu_event_router.cc:28:
extensions::InputMethodAPI::kOnImeMenuActivationChanged)) {
On 2016/01/06 21:15:44, Shu Chen wrote:
> indent.

Done.

https://codereview.chromium.org/1552743003/diff/60001/ui/base/ime/chromeos/in...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/60001/ui/base/ime/chromeos/in...
ui/base/ime/chromeos/input_method_manager.h:72: // ImeMenuObserver is notified
of events related to the IME menu on the system
On 2016/01/06 21:15:44, Shu Chen wrote:
> s/system tray/shelf bar

Done.

https://codereview.chromium.org/1552743003/diff/60001/ui/base/ime/chromeos/in...
ui/base/ime/chromeos/input_method_manager.h:78: // indicates whether the user
should be notified of this change.
On 2016/01/06 21:15:44, Shu Chen wrote:
> wrong comment here.

Done.

[Shu Chen, 2016-01-07 02:26:41]

lgtm

[Devlin, 2016-01-07 18:35:31]

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:192:
"inputMethodPrivate.onImeMenuActivationChanged";
On 2016/01/06 11:50:44, Azure Wei wrote:
> On 2016/01/05 18:50:02, Devlin wrote:
> > ... Why doesn't inputMethodPrivate use the auto-generated files?
> 
> There's no public API in chrome.inputMethodPrivate.*.
input_method_private.json
> is not listed in schemas.gypi, so there's no generated input_method_private.h
> file.

I know it's not listed in schemas.gypi, but my question was why.  It's very
unorthodox and rather fragile to be hand-coding all of these events here.

[Shu Chen, 2016-01-08 01:15:57]

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:192:
"inputMethodPrivate.onImeMenuActivationChanged";
On 2016/01/07 18:35:31, Devlin wrote:
> On 2016/01/06 11:50:44, Azure Wei wrote:
> > On 2016/01/05 18:50:02, Devlin wrote:
> > > ... Why doesn't inputMethodPrivate use the auto-generated files?
> > 
> > There's no public API in chrome.inputMethodPrivate.*.
> input_method_private.json
> > is not listed in schemas.gypi, so there's no generated
input_method_private.h
> > file.
> 
> I know it's not listed in schemas.gypi, but my question was why.  It's very
> unorthodox and rather fragile to be hand-coding all of these events here.

That may be because of the historical reasons. The code was made years ago
before we took over the ownership.

I think using the auto-generated files should be out of scope for this cl.

And it can be a separated cl. WDYT?

[michaelpg, 2016-01-08 03:08:54]

https://codereview.chromium.org/1552743003/diff/120001/ui/base/ime/chromeos/i...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/120001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:77: // Called when the IME menu is
activated/deactivated. Notices to activate
"Notices to activate"? Can you make this clearer?

If you just mean that |activation| specifies whether activation was set to true
or false, the comment is redundant. Or, are you trying to describe what the
behavior of the virtual function should be?

[Azure Wei, 2016-01-08 03:26:05]

https://codereview.chromium.org/1552743003/diff/120001/ui/base/ime/chromeos/i...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/120001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:77: // Called when the IME menu is
activated/deactivated. Notices to activate
On 2016/01/08 03:08:54, michaelpg wrote:
> "Notices to activate"? Can you make this clearer?
> 
> If you just mean that |activation| specifies whether activation was set to
true
> or false, the comment is redundant. Or, are you trying to describe what the
> behavior of the virtual function should be?

Yeah, I simply mean that |activation| specifies whether activate or deactivate
the IME menu. I've deleted this redundant comment.

[stevenjb, 2016-01-12 21:59:19]

On 2016/01/06 21:31:34, stevenjb wrote:
> On 2016/01/06 11:44:16, Azure Wei wrote:
> > On 2016/01/05 17:26:36, stevenjb wrote:
> > > On 2016/01/05 10:35:50, Azure Wei wrote:
> > > > On 2016/01/04 18:21:54, stevenjb wrote:
> > > > > How will the JS get the initial state for IME menu activation?
> > > > 
> > > > This event will be fired when the preference
> pref:kLangugaeImeMenuActivated
> > > > changes. pref:kLangugaeImeMenuActivated is registered as false, and then
> > will
> > > be
> > > > initialized with user prefs. So the JS will get the event
> > > > onImeMenuActivationChanged(true) if the initial state is true, and won't
> get
> > > any
> > > > event if it's false. Since the IME menu will only be activated on event
> > > > onImeMenuActivationChanged(true), I think it's OK that the JS doesn't
get
> > the
> > > > initial false state. Does this make sense? I haven't implemented this
> > > mechanism
> > > > yet, I hope I have not got it wrong.
> > > > 
> > > > > You will need an extensions/ owner to review this, e.g.
rdevlin.cronin@
> > > > 
> > > > +rdevlin.cronin@ as reviewer.
> > > > Hi Devlin, can you help review this cl? Thank you!
> > > 
> > > An extension may (will likely) get loaded after the pref has been
> registered,
> > so
> > > the extension may not receive an event for the initial state. i.e. after
the
> > > user has set the pref then restarts the browser, the extension will likely
> get
> > > loaded after the pref (now true) is loaded.
> > 
> > How about put the  initial state in
inputMethodPrivate.getInputMethodConfig()
> > function, or add a new API?
> 
> That seems reasonable to me.

I would like to see the menu activation state be added to
inputMethodPrivate.getInputMethodConfig either as part of this CL, or as a
separate CL added prior to this one landing. Otherwise we will be providing an
API that is incorrect or misleading since an initial event may not trigger.

We should also add a test to input_method_apitest_chromeos.cc /
chrome/test/data/extensions/api_test/input_method/background.js (which will be
easier / more robust with the initial state available).

[Devlin, 2016-01-13 18:11:27]

lgtm with minor nits

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:192:
"inputMethodPrivate.onImeMenuActivationChanged";
On 2016/01/08 01:15:56, Shu Chen wrote:
> On 2016/01/07 18:35:31, Devlin wrote:
> > On 2016/01/06 11:50:44, Azure Wei wrote:
> > > On 2016/01/05 18:50:02, Devlin wrote:
> > > > ... Why doesn't inputMethodPrivate use the auto-generated files?
> > > 
> > > There's no public API in chrome.inputMethodPrivate.*.
> > input_method_private.json
> > > is not listed in schemas.gypi, so there's no generated
> input_method_private.h
> > > file.
> > 
> > I know it's not listed in schemas.gypi, but my question was why.  It's very
> > unorthodox and rather fragile to be hand-coding all of these events here.
> 
> That may be because of the historical reasons. The code was made years ago
> before we took over the ownership.
> 
> I think using the auto-generated files should be out of scope for this cl.
> 
> And it can be a separated cl. WDYT?

Yeah, a separate CL is fine.  But I'd like it to be a priority - not using a
common generated file is fragile/inefficient/unorthodox/etc.

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/ime_menu_event_router.h (right):

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:17: // Event router
class for the IME Menu activated/deactivate events.
Can you document the lifetime of this object, since it is required that it's
restricted to the lifetime of the InputMethodManager and the EventRouter?

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_api.cc:245: if
(!ime_menu_event_router_.get()) {
Is this different than
else if (details.event_name == kOnImeMenuActivationChanged &&
!ime_menu_event_router_)
?

https://codereview.chromium.org/1552743003/diff/140001/chrome/common/extensio...
File chrome/common/extensions/api/input_method_private.json (right):

https://codereview.chromium.org/1552743003/diff/140001/chrome/common/extensio...
chrome/common/extensions/api/input_method_private.json:231: "description":
"Fired when the system IME Menu is activated",
nit: lowercase "menu"
Is this only fired when the menu is activated?  Or also when it is deactivated? 
(I think the latter, in which case this should say "Fired when the system IME
menu is activated or deactivated.")

https://codereview.chromium.org/1552743003/diff/140001/chrome/common/extensio...
chrome/common/extensions/api/input_method_private.json:236: "description":
"Whether the IME menu is activated or deactivated"
nit: the phrasing here is a bit weird, because it makes it difficult to
understand whether this refers to the current state, or the state it was
transitioned out of.  Assuming it's the current state, say something like
"Whether the IME menu is currently active."

https://codereview.chromium.org/1552743003/diff/140001/ui/base/ime/chromeos/i...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/140001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:78: virtual void
ImeMenuActivationChanged(bool activation) = 0;
nit: |activation| is also a weird name here because it doesn't clearly state
what the value will be in each scenario.  If this is whether or not the menu is
active when this method is called, call it "is_active".

https://codereview.chromium.org/1552743003/diff/140001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:261: virtual void
ImeMenuActivationChanged(bool activation) = 0;
ditto

[Azure Wei, 2016-01-14 09:38:26]

>
> Yeah, a separate CL is fine.  But I'd like it to be a priority - not using a
> common generated file is fragile/inefficient/unorthodox/etc.
> 
Thank you for all your detailed comments. This CL has been rebased on the
following Cl:

https://codereview.chromium.org/1587913002/

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/ime_menu_event_router.h (right):

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/ime_menu_event_router.h:17: // Event router
class for the IME Menu activated/deactivate events.
On 2016/01/13 18:11:27, Devlin wrote:
> Can you document the lifetime of this object, since it is required that it's
> restricted to the lifetime of the InputMethodManager and the EventRouter?

Done.

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_api.cc:245: if
(!ime_menu_event_router_.get()) {
On 2016/01/13 18:11:27, Devlin wrote:
> Is this different than
> else if (details.event_name == kOnImeMenuActivationChanged &&
> !ime_menu_event_router_)
> ?

Done.

https://codereview.chromium.org/1552743003/diff/140001/chrome/common/extensio...
File chrome/common/extensions/api/input_method_private.json (right):

https://codereview.chromium.org/1552743003/diff/140001/chrome/common/extensio...
chrome/common/extensions/api/input_method_private.json:231: "description":
"Fired when the system IME Menu is activated",
On 2016/01/13 18:11:27, Devlin wrote:
> nit: lowercase "menu"
> Is this only fired when the menu is activated?  Or also when it is
deactivated? 
> (I think the latter, in which case this should say "Fired when the system IME
> menu is activated or deactivated.")

Done.

https://codereview.chromium.org/1552743003/diff/140001/chrome/common/extensio...
chrome/common/extensions/api/input_method_private.json:236: "description":
"Whether the IME menu is activated or deactivated"
On 2016/01/13 18:11:27, Devlin wrote:
> nit: the phrasing here is a bit weird, because it makes it difficult to
> understand whether this refers to the current state, or the state it was
> transitioned out of.  Assuming it's the current state, say something like
> "Whether the IME menu is currently active."

Done.

https://codereview.chromium.org/1552743003/diff/140001/ui/base/ime/chromeos/i...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/140001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:78: virtual void
ImeMenuActivationChanged(bool activation) = 0;
On 2016/01/13 18:11:27, Devlin wrote:
> nit: |activation| is also a weird name here because it doesn't clearly state
> what the value will be in each scenario.  If this is whether or not the menu
is
> active when this method is called, call it "is_active".

Done.

https://codereview.chromium.org/1552743003/diff/140001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:261: virtual void
ImeMenuActivationChanged(bool activation) = 0;
On 2016/01/13 18:11:27, Devlin wrote:
> ditto

Done.

[Azure Wei, 2016-01-14 11:28:34]

On 2016/01/12 21:59:19, stevenjb wrote:
> On 2016/01/06 21:31:34, stevenjb wrote:
> > On 2016/01/06 11:44:16, Azure Wei wrote:
> > > On 2016/01/05 17:26:36, stevenjb wrote:
> > > > On 2016/01/05 10:35:50, Azure Wei wrote:
> > > > > On 2016/01/04 18:21:54, stevenjb wrote:
> > > > > > How will the JS get the initial state for IME menu activation?
> > > > > 
> > > > > This event will be fired when the preference
> > pref:kLangugaeImeMenuActivated
> > > > > changes. pref:kLangugaeImeMenuActivated is registered as false, and
then
> > > will
> > > > be
> > > > > initialized with user prefs. So the JS will get the event
> > > > > onImeMenuActivationChanged(true) if the initial state is true, and
won't
> > get
> > > > any
> > > > > event if it's false. Since the IME menu will only be activated on
event
> > > > > onImeMenuActivationChanged(true), I think it's OK that the JS doesn't
> get
> > > the
> > > > > initial false state. Does this make sense? I haven't implemented this
> > > > mechanism
> > > > > yet, I hope I have not got it wrong.
> > > > > 
> > > > > > You will need an extensions/ owner to review this, e.g.
> rdevlin.cronin@
> > > > > 
> > > > > +rdevlin.cronin@ as reviewer.
> > > > > Hi Devlin, can you help review this cl? Thank you!
> > > > 
> > > > An extension may (will likely) get loaded after the pref has been
> > registered,
> > > so
> > > > the extension may not receive an event for the initial state. i.e. after
> the
> > > > user has set the pref then restarts the browser, the extension will
likely
> > get
> > > > loaded after the pref (now true) is loaded.
> > > 
> > > How about put the  initial state in
> inputMethodPrivate.getInputMethodConfig()
> > > function, or add a new API?
> > 
> > That seems reasonable to me.
> 
> I would like to see the menu activation state be added to
> inputMethodPrivate.getInputMethodConfig either as part of this CL, or as a
> separate CL added prior to this one landing. Otherwise we will be providing an
> API that is incorrect or misleading since an initial event may not trigger.
> 
> We should also add a test to input_method_apitest_chromeos.cc /
> chrome/test/data/extensions/api_test/input_method/background.js (which will be
> easier / more robust with the initial state available).

The initial state for IME menu activation is added into the
inputMethodPrivate.getInputConfig API with parameters['isImeMenuActivated'];
And a new test imeMenuConfigTest() is added to
chrome/test/data/extensions/api_test/input_method/background.js to verify it
works fine to get the initial state.
Could you please have another look at this CL? Thank you!

[Azure Wei, 2016-01-20 01:16:34]

Kindly pinging...
stevenjb@, michaelpg@, would you mind taking another look at this CL? Thank you!

[stevenjb, 2016-01-20 01:38:03]

On 2016/01/20 01:16:34, Azure Wei wrote:
> Kindly pinging...
> stevenjb@, michaelpg@, would you mind taking another look at this CL? Thank
you!

That's odd, I don't see isImeMenuActivated in the diff of
input_method_private.json, although I do see it in the file here. Could you try
rebasing / reuploading?

[Azure Wei, 2016-01-20 03:16:13]

On 2016/01/20 01:38:03, stevenjb wrote:
> On 2016/01/20 01:16:34, Azure Wei wrote:
> > Kindly pinging...
> > stevenjb@, michaelpg@, would you mind taking another look at this CL? Thank
> you!
> 
> That's odd, I don't see isImeMenuActivated in the diff of
> input_method_private.json, although I do see it in the file here. Could you
try
> rebasing / reuploading?

Done. The current patch set is up to date.

[Azure Wei, 2016-01-20 04:30:09]

Description was changed from

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API.
This API will be used for IME extensions to opt-in the IME menu on shelf. 

BUG=570761
TEST=Verified the API is available on Chrome OS.
==========

to

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API, which will be
fired when the checkbox in language settings about IME menu is checked or
unchecked.

Modify chrome.inputMethodPrivate.getInputMethodConfig() API to get the initial
state of the activation of IME menu.

These APIs will be used for IME extensions to opt-in the IME menu on shelf. 

BUG=570761
TEST=Verified the API is available on Chrome OS.
==========

[stevenjb, 2016-01-20 18:33:43]

https://codereview.chromium.org/1552743003/diff/200001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/background.js (right):

https://codereview.chromium.org/1552743003/diff/200001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:199: });
This tests the initial state, but not the event. See e.g.
dictionaryChangedTest() above.

https://codereview.chromium.org/1552743003/diff/200001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:206:
ictionaryChangedTest, imeMenuConfigTest]);
'd' missing at beginning of line. (Did this succeed locally?)

https://codereview.chromium.org/1552743003/diff/200001/ui/base/ime/chromeos/i...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/200001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:76: virtual ~ImeMenuObserver() {}
blank line

https://codereview.chromium.org/1552743003/diff/200001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:78: virtual void
ImeMenuActivationChanged(bool is_active) = 0;
private:
 DISALLOW_ASSIGN(...)

[michaelpg, 2016-01-20 21:06:28]

https://codereview.chromium.org/1552743003/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_api.cc:70:
Profile::FromBrowserContext(browser_context())
nit: align w/ previous line

[Azure Wei, 2016-01-21 10:25:08]

https://codereview.chromium.org/1552743003/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1552743003/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_api.cc:70:
Profile::FromBrowserContext(browser_context())
On 2016/01/20 21:06:28, michaelpg wrote:
> nit: align w/ previous line

Done.

https://codereview.chromium.org/1552743003/diff/200001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/background.js (right):

https://codereview.chromium.org/1552743003/diff/200001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:199: });
On 2016/01/20 18:33:43, stevenjb wrote:
> This tests the initial state, but not the event. See e.g.
> dictionaryChangedTest() above.

imeMenuActivationChangedTest() is added to test the state change event.

https://codereview.chromium.org/1552743003/diff/200001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:206:
ictionaryChangedTest, imeMenuConfigTest]);
On 2016/01/20 18:33:43, stevenjb wrote:
> 'd' missing at beginning of line. (Did this succeed locally?)
> 

Sorry about the mistake.

https://codereview.chromium.org/1552743003/diff/200001/ui/base/ime/chromeos/i...
File ui/base/ime/chromeos/input_method_manager.h (right):

https://codereview.chromium.org/1552743003/diff/200001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:76: virtual ~ImeMenuObserver() {}
On 2016/01/20 18:33:43, stevenjb wrote:
> blank line

Done.

https://codereview.chromium.org/1552743003/diff/200001/ui/base/ime/chromeos/i...
ui/base/ime/chromeos/input_method_manager.h:78: virtual void
ImeMenuActivationChanged(bool is_active) = 0;
On 2016/01/20 18:33:43, stevenjb wrote:
> private:
>  DISALLOW_ASSIGN(...)

Done.

[stevenjb, 2016-01-21 19:16:16]

https://codereview.chromium.org/1552743003/diff/240001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
(right):

https://codereview.chromium.org/1552743003/diff/240001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:85: //
Listerner for IME menu event ready.
Listener

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/background.js (right):

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:207:
chrome.test.succeed();
Remove this extra test.succeed() call and add a comment stating that we wait for
the 'ime_menu_ready' message in xtensionInputMethodApiTest.

[Devlin, 2016-01-21 19:30:25]

https://codereview.chromium.org/1552743003/diff/240001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
(right):

https://codereview.chromium.org/1552743003/diff/240001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:88:
browser()->profile()->GetPrefs()->SetBoolean(prefs::kLangugaeImeMenuActivated,
kLangugae?  Is that typo everywhere?

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/background.js (right):

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:204:
function(activation) {
nit: as elsewhere, prefer a variable name of "isActive" rather than "activation"
(which could be anything).

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:205:
chrome.test.assertTrue(!activation);
prefer assertFalse

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:209: // We just
add the listener without receiving any event at first.
Add that the test is continued in the C++; otherwise it's unclear how this will
work.

[Devlin, 2016-01-21 19:36:02]

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/background.js (right):

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:202: function
imeMenuActivationChangedTest() {
Thinking about this more, let's have this one be a separate browser test. 
Ideally, we have two types of extension API tests - those that use
chrome.test.runTests() and are entirely in the JS (possibly with a bit of set
up) and those that do back-and-forth communication using
chrome.test.sendMessage().  Overloading the two is a little bit of a pain,
especially because we might in the future want to do cleanup after runTests()
finishes, like removing all event listeners, and because having this event
listener here could affect future tests added to runTests() if they change the
activation.

[Azure Wei, 2016-01-23 02:40:39]

https://codereview.chromium.org/1552743003/diff/240001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
(right):

https://codereview.chromium.org/1552743003/diff/240001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:85: //
Listerner for IME menu event ready.
On 2016/01/21 19:16:16, stevenjb wrote:
> Listener

Done.

https://codereview.chromium.org/1552743003/diff/240001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:88:
browser()->profile()->GetPrefs()->SetBoolean(prefs::kLangugaeImeMenuActivated,
On 2016/01/21 19:30:25, Devlin (Slow until 1-27) wrote:
> kLangugae?  Is that typo everywhere?

Oh, I've so embarrassed. Corrected them in this cl.

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/background.js (right):

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:202: function
imeMenuActivationChangedTest() {
On 2016/01/21 19:36:02, Devlin (Slow until 1-27) wrote:
> Thinking about this more, let's have this one be a separate browser test. 
> Ideally, we have two types of extension API tests - those that use
> chrome.test.runTests() and are entirely in the JS (possibly with a bit of set
> up) and those that do back-and-forth communication using
> chrome.test.sendMessage().  Overloading the two is a little bit of a pain,
> especially because we might in the future want to do cleanup after runTests()
> finishes, like removing all event listeners, and because having this event
> listener here could affect future tests added to runTests() if they change the
> activation.

Done. The imeMenuConfigTest() and imeMenuActivationChangedTest() are moved to
make a new browser test: ExtensionInputMethodApiTest.ImeMenuActivation.

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:204:
function(activation) {
On 2016/01/21 19:30:25, Devlin (Slow until 1-27) wrote:
> nit: as elsewhere, prefer a variable name of "isActive" rather than
"activation"
> (which could be anything).

Done.

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:205:
chrome.test.assertTrue(!activation);
On 2016/01/21 19:30:25, Devlin (Slow until 1-27) wrote:
> prefer assertFalse

Done.

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:207:
chrome.test.succeed();
On 2016/01/21 19:16:16, stevenjb wrote:
> Remove this extra test.succeed() call and add a comment stating that we wait
for
> the 'ime_menu_ready' message in xtensionInputMethodApiTest.

Done.

https://codereview.chromium.org/1552743003/diff/240001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/background.js:209: // We just
add the listener without receiving any event at first.
On 2016/01/21 19:30:25, Devlin (Slow until 1-27) wrote:
> Add that the test is continued in the C++; otherwise it's unclear how this
will
> work.

Done.

[Devlin, 2016-01-25 20:35:36]

https://codereview.chromium.org/1552743003/diff/260001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
(right):

https://codereview.chromium.org/1552743003/diff/260001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:91:
ExtensionTestMessageListener config_listener("config_ready", false);
Right now, this is never used (but it should be).

https://codereview.chromium.org/1552743003/diff/260001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/ime_menu/background.js
(right):

https://codereview.chromium.org/1552743003/diff/260001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/ime_menu/background.js:27:
chrome.test.runTests(
This style of test would be more suited to just the sendMessage style, rather
than using runTests() at all.  So let's remove this (and all the
chrome.test.succeeds()) and only use chrome.test.sendMessage to indicate when
something should/did happen.

[Azure Wei, 2016-01-27 05:06:26]

https://codereview.chromium.org/1552743003/diff/260001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
(right):

https://codereview.chromium.org/1552743003/diff/260001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:91:
ExtensionTestMessageListener config_listener("config_ready", false);
On 2016/01/25 20:35:36, Devlin (Slow until 1-27) wrote:
> Right now, this is never used (but it should be).

Done.

https://codereview.chromium.org/1552743003/diff/260001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/ime_menu/background.js
(right):

https://codereview.chromium.org/1552743003/diff/260001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/ime_menu/background.js:27:
chrome.test.runTests(
On 2016/01/25 20:35:36, Devlin (Slow until 1-27) wrote:
> This style of test would be more suited to just the sendMessage style, rather
> than using runTests() at all.  So let's remove this (and all the
> chrome.test.succeeds()) and only use chrome.test.sendMessage to indicate when
> something should/did happen.

Done.

[Devlin, 2016-01-27 20:33:27]

https://codereview.chromium.org/1552743003/diff/280001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
(right):

https://codereview.chromium.org/1552743003/diff/280001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:102:
ASSERT_TRUE(RunExtensionTest("input_method/ime_menu")) << message_;
Instead of using RunExtensionTest here, just load the extension.

https://codereview.chromium.org/1552743003/diff/280001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/ime_menu/background.js
(right):

https://codereview.chromium.org/1552743003/diff/280001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/ime_menu/background.js:21:
chrome.test.notifyPass();
We shouldn't need this.

[Azure Wei, 2016-01-28 12:24:58]

https://codereview.chromium.org/1552743003/diff/280001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
(right):

https://codereview.chromium.org/1552743003/diff/280001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc:102:
ASSERT_TRUE(RunExtensionTest("input_method/ime_menu")) << message_;
On 2016/01/27 20:33:27, Devlin (Slow until 1-27) wrote:
> Instead of using RunExtensionTest here, just load the extension.

Done.

https://codereview.chromium.org/1552743003/diff/280001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/ime_menu/background.js
(right):

https://codereview.chromium.org/1552743003/diff/280001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/ime_menu/background.js:21:
chrome.test.notifyPass();
On 2016/01/27 20:33:27, Devlin (Slow until 1-27) wrote:
> We shouldn't need this.

Deleted.

[Azure Wei, 2016-02-02 01:09:50]

Kindly pinging... stevenjb@, michaelpg@, could you please review the latest
patch set? Thank you!

[stevenjb, 2016-02-02 22:30:18]

lgtm

https://codereview.chromium.org/1552743003/diff/300001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/ime_menu/background.js
(right):

https://codereview.chromium.org/1552743003/diff/300001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/ime_menu/background.js:14:
function(isActive) {
indent the block below

[michaelpg, 2016-02-03 17:00:00]

lgtm

[Azure Wei, 2016-02-04 05:10:13]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-02-04 05:10:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, shuchen@chromium.org, rdevlin.cronin@chromium.org,
stevenjb@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1552743003/#ps320001
(title: " ")

[commit-bot: I haz the power, 2016-02-04 05:10:32]

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

[Azure Wei, 2016-02-04 05:12:51]

https://codereview.chromium.org/1552743003/diff/300001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_method/ime_menu/background.js
(right):

https://codereview.chromium.org/1552743003/diff/300001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_method/ime_menu/background.js:14:
function(isActive) {
On 2016/02/02 22:30:18, stevenjb wrote:
> indent the block below

Done. Thanks!

[commit-bot: I haz the power, 2016-02-04 05:26:05]

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

[commit-bot: I haz the power, 2016-02-04 05:26:07]

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

[Azure Wei, 2016-02-04 06:10:16]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-02-04 06:10:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, stevenjb@chromium.org, isherman@chromium.org,
rdevlin.cronin@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1552743003/#ps340001
(title: "Fix include error.")

[commit-bot: I haz the power, 2016-02-04 06:10:34]

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

[commit-bot: I haz the power, 2016-02-04 06:29:51]

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

[commit-bot: I haz the power, 2016-02-04 06:29:52]

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

[Azure Wei, 2016-02-04 06:32:54]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-02-04 06:32:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, stevenjb@chromium.org, isherman@chromium.org,
rdevlin.cronin@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1552743003/#ps360001
(title: " ")

[commit-bot: I haz the power, 2016-02-04 06:34:46]

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

[commit-bot: I haz the power, 2016-02-04 08:12:59]

Description was changed from

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API, which will be
fired when the checkbox in language settings about IME menu is checked or
unchecked.

Modify chrome.inputMethodPrivate.getInputMethodConfig() API to get the initial
state of the activation of IME menu.

These APIs will be used for IME extensions to opt-in the IME menu on shelf. 

BUG=570761
TEST=Verified the API is available on Chrome OS.
==========

to

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API, which will be
fired when the checkbox in language settings about IME menu is checked or
unchecked.

Modify chrome.inputMethodPrivate.getInputMethodConfig() API to get the initial
state of the activation of IME menu.

These APIs will be used for IME extensions to opt-in the IME menu on shelf. 

BUG=570761
TEST=Verified the API is available on Chrome OS.
==========

[commit-bot: I haz the power, 2016-02-04 08:13:02]

Committed patchset #19 (id:360001)

[commit-bot: I haz the power, 2016-02-04 08:15:43]

Description was changed from

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API, which will be
fired when the checkbox in language settings about IME menu is checked or
unchecked.

Modify chrome.inputMethodPrivate.getInputMethodConfig() API to get the initial
state of the activation of IME menu.

These APIs will be used for IME extensions to opt-in the IME menu on shelf. 

BUG=570761
TEST=Verified the API is available on Chrome OS.
==========

to

==========
Add chrome.inputMethodPrivate.onImeMenuActivationChanged API, which will be
fired when the checkbox in language settings about IME menu is checked or
unchecked.

Modify chrome.inputMethodPrivate.getInputMethodConfig() API to get the initial
state of the activation of IME menu.

These APIs will be used for IME extensions to opt-in the IME menu on shelf.

BUG=570761
TEST=Verified the API is available on Chrome OS.

Committed: https://crrev.com/f7a97e05428cb68d7b77e6d04f925772f252f4be
Cr-Commit-Position: refs/heads/master@{#373479}
==========

[commit-bot: I haz the power, 2016-02-04 08:15:45]

Patchset 19 (id:??) landed as
https://crrev.com/f7a97e05428cb68d7b77e6d04f925772f252f4be
Cr-Commit-Position: refs/heads/master@{#373479}