IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

The goal of this CL is to modify content_browser's IMEDriver so we have access
to ui::InputMethodChromeOS's functionality in mus+ash.

This CL:
- Modifies ui::InputMethodChromeOS so it can call an ack callback after
  processing events.
- Introduces InputMethodBridge to acts as a bridge between
  ui::mojom::InputMethod and ui::InputMethodChromeOS.
- Introduces RemoteTextInputClient to send commands received from
  ui::InputMethodChromeOS to a remote client over mojo IPC.
- Modifies content_browser's IMEDriver implementation to use
  ui::InputMethodChromeOS via InputMethodBridge.
- Some unittests to verify that InputMethodBridge works correctly.

BUG=665981
Committed: https://crrev.com/b171ff9c2c408898b6678001574019efefc42b09
Cr-Commit-Position: refs/heads/master@{#437955}

[Hadi, 2016-12-05 18:34:09]

Description was changed from

==========
IME for Mus: Use ui::InputChromeOS to provide logic for ime driver.

BUG=NONE
==========

to

==========
IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

BUG=NONE
==========

[Hadi, 2016-12-07 19:47:42]

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

[commit-bot: I haz the power, 2016-12-07 19:48:17]

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

[Hadi, 2016-12-07 19:57:21]

Description was changed from

==========
IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

BUG=NONE
==========

to

==========
IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

The goal of this CL is to modify content_browser's IMEDriver so we have access
to ui::InputMethodChromeOS's functionality in mus+ash.

This CL:
- Modifies ui::InputMethodChromeOS so it can call an ack callback after
  processing events.
- Introduces InputMethodBridge to acts as a bridge between
  ui::mojom::InputMethod and ui::InputMethodChromeOS.
- Introduces RemoteTextInputClient to send commands received from
  ui::InputMethodChromeOS to a remote client over mojo IPC.
- Modifies content_browser's IMEDriver implementation to use
  ui::InputMethodChromeOS via InputMethodBridge.
- Some unittests to verify that InputMethodBridge works correctly.

BUG=665981
==========

[Hadi, 2016-12-07 20:05:01]

moshayedi@chromium.org changed reviewers:
+ sadrul@chromium.org, shuchen@chromium.org, sky@chromium.org

[Hadi, 2016-12-07 20:05:02]

After this CL, CharacterComposer works in mus+ash, and at least one input method
extension works works in mus+ash. I will send follow-up CLs to make sure all
input method extensions work in the mus+ash environment.

shuchen@: Please review changes to input_method_chromeos. The only change is to
add an override of DispatchKeyEvent which also takes an ack callback function,
and makes sure that ack callback is called after processing events.

sky@, sadrul@: Please review the rest of CL.

I am not sure if we should keep content_browser's ime_driver in
chrome/browser/ui/views since it doesn't depend much on browser. Maybe we can
move it to a better place in another CL.

Thanks.

[commit-bot: I haz the power, 2016-12-07 20:19:34]

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

[commit-bot: I haz the power, 2016-12-07 20:19:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Hadi, 2016-12-07 20:43:02]

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

[commit-bot: I haz the power, 2016-12-07 20:43:33]

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

[Hadi, 2016-12-07 20:46:38]

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

[commit-bot: I haz the power, 2016-12-07 20:46:50]

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-12-07 22:00:40]

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

[commit-bot: I haz the power, 2016-12-07 22:00:40]

Dry run: This issue passed the CQ dry run.

[Shu Chen, 2016-12-09 03:30:17]

https://codereview.chromium.org/2557493002/diff/160001/ui/base/ime/input_meth...
File ui/base/ime/input_method_chromeos.cc (right):

https://codereview.chromium.org/2557493002/diff/160001/ui/base/ime/input_meth...
ui/base/ime/input_method_chromeos.cc:142: ack_callback->Run(is_handled);
Not sure whether this has already been considered.
Is it possible to leverage InputMethodDelegate::ProcessKeyEventPostIME()?

What's more;
1) I think it would be better to use generic notification pattern than callback.
2) As the key dispatching is in async, the parameter of the callback should
carry the original events. So the callback can have the knowledge of which key
is handled and which is not.

[Hadi, 2016-12-09 04:40:01]

On 2016/12/09 03:30:17, Shu Chen wrote:
>
https://codereview.chromium.org/2557493002/diff/160001/ui/base/ime/input_meth...
> File ui/base/ime/input_method_chromeos.cc (right):
> 
>
https://codereview.chromium.org/2557493002/diff/160001/ui/base/ime/input_meth...
> ui/base/ime/input_method_chromeos.cc:142: ack_callback->Run(is_handled);
> Not sure whether this has already been considered.
> Is it possible to leverage InputMethodDelegate::ProcessKeyEventPostIME()?

Assuming you mean InputMethodChromeOS::ProcessKeyEventPostIME(), this might be
possible but more difficult.

To put this in context:

When Mus dispatches an event to a client, it also provides an event_id, so
clients can later tell Mus whether they handled the event or not. See comments
for WindowTreeClient::OnWindowInputEvent() in window_tree.mojom.

Client code for keyboard events eventually reaches
InputMethodMus::DispatchKeyEvent(). This function sends the event to the IME
driver, and after IME driver processes the event and we know if the event was
handled by IME or not, it calls the corresponding ack function so Mus knows if
the event was handled by client or not.

Now, we had two options here for API of ui::mojom::ProcessKeyEvent():

(a1) Similar to
WindowTreeClient::OnWindowInputEvent()/WindowTree::OnWindowInputEventAck()
provide some kind of ack_id:
  ui.mojom.InputMethod.ProcessKeyEvent(Event event, int ack_id);
  ui.mojom.TextInputClient.OnEventAck(int ack_id);
(Note that for security reasons, value of ack_id cannot be the same as
WindowTreeClient's event_id).

(a2) Instead of ack_id, provide a callback to ProcessKeyEvent:
 ui.mojom.ProcessKeyEvent(ui.mojom.Event key_event) => (bool handled);

Option #a1 didn't seem to have much advantages to #a2, but implementing it was
more cumbersome. It requires keeping queue of event_ids, mapping between
WindowTreeClient event_id's and IME ack_ids, timeout timers, etc. Since #a2
meant Mojo IPC took care of most of these things for us, we went with #a2.

Now, to implement #a2 in content_browser's IME driver which uses
InputMethodChromeOS. We have two options:

(b1) Subclass InputMethodChromeOS, override
InputMethodChromeOS::ProcessKeyEventPostIME() (and change DispatchKeyEvent() to
call ProcessKeyEventPostIME(..., false) if character composer didn't handle the
event). This will need keeping queues, mappings, timeout timers, etc. so in
ProcessKeyEventPostIME() we know which mojo callback we should call.

(b2) Send #a2's callback to InputMethodChromeOS::DispatchKeyEvent(), which in
turn sends it to ProcessKeyEventPostIME(), to avoid the queues, etc in #b1.

(b3) Overload InputMethodChromeOS::DispatchKeyEvent() to accept a callback with
same signature as ProcessKeyEventPostIME(). Regular DispatchKeyEvent() calls
this overload with InputMethodChromeOS::ProcessKeyEventPostIME(), and
content_browser's IME driver calls this with a wrapper around the mojo callback.
Note that #b1 and this is that the callback here knows which mojo callback it is
calling, but to have the same thing in #b1 we need to manually keep track of the
mojo callback's.

I did #b2 here, but maybe #b3 might be a bit more generic, while as simple as
#b2. #b1 will require minimal changes to InputMethodChromeOS, but will make IME
driver's implementation more difficult.

> What's more;
> 1) I think it would be better to use generic notification pattern than
callback.
> 2) As the key dispatching is in async, the parameter of the callback should
> carry the original events. So the callback can have the knowledge of which key
> is handled and which is not.

Agreed, but here the callback for ui.mojom.ProcessKeyEvent() is bound to the
mojo method call, so including the event in the callback might be redundant
since the users of the API can keep track of the origin event themselves when
creating a callback using base::Bind().

Thanks.

[Shu Chen, 2016-12-10 12:38:52]

On 2016/12/09 04:40:01, Hadi wrote:
> On 2016/12/09 03:30:17, Shu Chen wrote:
> >
>
https://codereview.chromium.org/2557493002/diff/160001/ui/base/ime/input_meth...
> > File ui/base/ime/input_method_chromeos.cc (right):
> > 
> >
>
https://codereview.chromium.org/2557493002/diff/160001/ui/base/ime/input_meth...
> > ui/base/ime/input_method_chromeos.cc:142: ack_callback->Run(is_handled);
> > Not sure whether this has already been considered.
> > Is it possible to leverage InputMethodDelegate::ProcessKeyEventPostIME()?
> 
> Assuming you mean InputMethodChromeOS::ProcessKeyEventPostIME(), this might be
> possible but more difficult.
> 
> To put this in context:
> 
> When Mus dispatches an event to a client, it also provides an event_id, so
> clients can later tell Mus whether they handled the event or not. See comments
> for WindowTreeClient::OnWindowInputEvent() in window_tree.mojom.
> 
> Client code for keyboard events eventually reaches
> InputMethodMus::DispatchKeyEvent(). This function sends the event to the IME
> driver, and after IME driver processes the event and we know if the event was
> handled by IME or not, it calls the corresponding ack function so Mus knows if
> the event was handled by client or not.
> 
> Now, we had two options here for API of ui::mojom::ProcessKeyEvent():
> 
> (a1) Similar to
> WindowTreeClient::OnWindowInputEvent()/WindowTree::OnWindowInputEventAck()
> provide some kind of ack_id:
>   ui.mojom.InputMethod.ProcessKeyEvent(Event event, int ack_id);
>   ui.mojom.TextInputClient.OnEventAck(int ack_id);
> (Note that for security reasons, value of ack_id cannot be the same as
> WindowTreeClient's event_id).
> 
> (a2) Instead of ack_id, provide a callback to ProcessKeyEvent:
>  ui.mojom.ProcessKeyEvent(ui.mojom.Event key_event) => (bool handled);
> 
> Option #a1 didn't seem to have much advantages to #a2, but implementing it was
> more cumbersome. It requires keeping queue of event_ids, mapping between
> WindowTreeClient event_id's and IME ack_ids, timeout timers, etc. Since #a2
> meant Mojo IPC took care of most of these things for us, we went with #a2.
> 
> Now, to implement #a2 in content_browser's IME driver which uses
> InputMethodChromeOS. We have two options:
> 
> (b1) Subclass InputMethodChromeOS, override
> InputMethodChromeOS::ProcessKeyEventPostIME() (and change DispatchKeyEvent()
to
> call ProcessKeyEventPostIME(..., false) if character composer didn't handle
the
> event). This will need keeping queues, mappings, timeout timers, etc. so in
> ProcessKeyEventPostIME() we know which mojo callback we should call.
> 
> (b2) Send #a2's callback to InputMethodChromeOS::DispatchKeyEvent(), which in
> turn sends it to ProcessKeyEventPostIME(), to avoid the queues, etc in #b1.
> 
> (b3) Overload InputMethodChromeOS::DispatchKeyEvent() to accept a callback
with
> same signature as ProcessKeyEventPostIME(). Regular DispatchKeyEvent() calls
> this overload with InputMethodChromeOS::ProcessKeyEventPostIME(), and
> content_browser's IME driver calls this with a wrapper around the mojo
callback.
> Note that #b1 and this is that the callback here knows which mojo callback it
is
> calling, but to have the same thing in #b1 we need to manually keep track of
the
> mojo callback's.
> 
> I did #b2 here, but maybe #b3 might be a bit more generic, while as simple as
> #b2. #b1 will require minimal changes to InputMethodChromeOS, but will make
IME
> driver's implementation more difficult.
> 
> > What's more;
> > 1) I think it would be better to use generic notification pattern than
> callback.
> > 2) As the key dispatching is in async, the parameter of the callback should
> > carry the original events. So the callback can have the knowledge of which
key
> > is handled and which is not.
> 
> Agreed, but here the callback for ui.mojom.ProcessKeyEvent() is bound to the
> mojo method call, so including the event in the callback might be redundant
> since the users of the API can keep track of the origin event themselves when
> creating a callback using base::Bind().
> 
> Thanks.

Thanks for the clarification. The change looks good to me now.

[Shu Chen, 2016-12-10 12:39:35]

lgtm

[sky, 2016-12-12 02:16:21]

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc:10: :
client_(new RemoteTextInputClient(std::move(client))),
MakeUnique where possible.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc
(right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc:25:
enum CompositionEventType { SET, CONFIRM, CLEAR, INSERT_TEXT, INSERT_CHAR };
enum class.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc:40:
run_loop_.reset(new base::RunLoop);
Again, MakeUnique where possible.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/noop_input_method.h (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/noop_input_method.h:14: class NoOpInputMethod
: public ui::mojom::InputMethod {
NoOp makes me thing this does nothing, when it runs the callback. WDYT of
SimpleInputMethod? If you really REALLY prefer NoOp, then capitalization of
guards should be NO_OP.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/noop_input_method.h:21:
ui::mojom::TextInputMode text_input_mode) override {}
Don't inline these (see style guide).

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/remote_text_input_client.cc (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/remote_text_input_client.cc:50:
NOTIMPLEMENTED();
Are these going to be spammy? Can you define NOTIMPLEMENTED_POLICY to 5 early on
(see ui/views/mus/native_widget_mus.cc for what I mean).

https://codereview.chromium.org/2557493002/diff/160001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/test/BUILD.gn#n...
chrome/test/BUILD.gn:4794:
"../browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc",
Shouldn't this only be inclined on chromeos?

[Hadi, 2016-12-12 15:29:13]

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

[commit-bot: I haz the power, 2016-12-12 15:29:38]

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

[Hadi, 2016-12-12 16:02:25]

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

[commit-bot: I haz the power, 2016-12-12 16:03:08]

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

[Hadi, 2016-12-12 16:04:58]

Thanks for feedback. All were addressed.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc:10: :
client_(new RemoteTextInputClient(std::move(client))),
On 2016/12/12 02:16:21, sky wrote:
> MakeUnique where possible.

Done.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc
(right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc:25:
enum CompositionEventType { SET, CONFIRM, CLEAR, INSERT_TEXT, INSERT_CHAR };
On 2016/12/12 02:16:21, sky wrote:
> enum class.

Done.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc:40:
run_loop_.reset(new base::RunLoop);
On 2016/12/12 02:16:21, sky wrote:
> Again, MakeUnique where possible.

Done.

Couple of "new"'s are still in ime_driver_mus.cc, which will be removed soon
when addressing crbug.com/673358.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/noop_input_method.h (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/noop_input_method.h:14: class NoOpInputMethod
: public ui::mojom::InputMethod {
On 2016/12/12 02:16:21, sky wrote:
> NoOp makes me thing this does nothing, when it runs the callback. WDYT of
> SimpleInputMethod? If you really REALLY prefer NoOp, then capitalization of
> guards should be NO_OP.

Done.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/noop_input_method.h:21:
ui::mojom::TextInputMode text_input_mode) override {}
On 2016/12/12 02:16:21, sky wrote:
> Don't inline these (see style guide).

Done.

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/ime_driver/remote_text_input_client.cc (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/ime_driver/remote_text_input_client.cc:50:
NOTIMPLEMENTED();
On 2016/12/12 02:16:21, sky wrote:
> Are these going to be spammy? Can you define NOTIMPLEMENTED_POLICY to 5 early
on
> (see ui/views/mus/native_widget_mus.cc for what I mean).

Done.

https://codereview.chromium.org/2557493002/diff/160001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2557493002/diff/160001/chrome/test/BUILD.gn#n...
chrome/test/BUILD.gn:4794:
"../browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc",
On 2016/12/12 02:16:21, sky wrote:
> Shouldn't this only be inclined on chromeos?

Done.

[sky, 2016-12-12 16:13:35]

LGTM

[commit-bot: I haz the power, 2016-12-12 16:22:27]

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

[commit-bot: I haz the power, 2016-12-12 16:22:28]

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

[Hadi, 2016-12-12 16:29:05]

The CQ bit was checked by moshayedi@chromium.org

[Hadi, 2016-12-12 16:29:05]

The patchset sent to the CQ was uploaded after l-g-t-m from shuchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2557493002/#ps220001
(title: "More cleanup.")

[commit-bot: I haz the power, 2016-12-12 16:29:31]

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

[sadrul, 2016-12-12 18:50:16]

Would it make sense to move the ime_driver out of chrome, because right now, it
doesn't look like it has any dependency on the rest of chrome? Or is this going
to grow dependency on other chromeos-IME things living in
chrome/browser/chromeos/?

https://codereview.chromium.org/2557493002/diff/220001/ui/aura/mus/text_input...
File ui/aura/mus/text_input_client_impl.cc (left):

https://codereview.chromium.org/2557493002/diff/220001/ui/aura/mus/text_input...
ui/aura/mus/text_input_client_impl.cc:42: DCHECK(key_event->is_char());
Why remove the DCHECK?

https://codereview.chromium.org/2557493002/diff/220001/ui/views/mus/text_inpu...
File ui/views/mus/text_input_client_impl.cc (left):

https://codereview.chromium.org/2557493002/diff/220001/ui/views/mus/text_inpu...
ui/views/mus/text_input_client_impl.cc:42: DCHECK(key_event->is_char());
Does this DCHECK() no longer hold?

[Hadi, 2016-12-12 19:10:22]

Thanks for comments.

On 2016/12/12 18:50:16, sadrul wrote:
> Would it make sense to move the ime_driver out of chrome, because right now,
it
> doesn't look like it has any dependency on the rest of chrome? Or is this
going
> to grow dependency on other chromeos-IME things living in
> chrome/browser/chromeos/?

I'll file a bug for this to investigate this more.

> 
>
https://codereview.chromium.org/2557493002/diff/220001/ui/aura/mus/text_input...
> File ui/aura/mus/text_input_client_impl.cc (left):
> 
>
https://codereview.chromium.org/2557493002/diff/220001/ui/aura/mus/text_input...
> ui/aura/mus/text_input_client_impl.cc:42: DCHECK(key_event->is_char());
> Why remove the DCHECK?
> 
>
https://codereview.chromium.org/2557493002/diff/220001/ui/views/mus/text_inpu...
> File ui/views/mus/text_input_client_impl.cc (left):
> 
>
https://codereview.chromium.org/2557493002/diff/220001/ui/views/mus/text_inpu...
> ui/views/mus/text_input_client_impl.cc:42: DCHECK(key_event->is_char());
> Does this DCHECK() no longer hold?

InputMethodChromeOS may send non-character events to InsertChar(), and
implementations of ui::TextInputClient (e.g. views::Textfield) doesn't require
the event to be character event. So this is not necessary. Keeping this check
requires checking and modifying what InputMethodChromeOS generates, which
doesn't
seem necessary.

[sadrul, 2016-12-12 19:11:33]

sg. lgtm

[Hadi, 2016-12-12 21:32:38]

The CQ bit was unchecked by moshayedi@chromium.org

[Hadi, 2016-12-12 21:32:43]

The CQ bit was checked by moshayedi@chromium.org

[commit-bot: I haz the power, 2016-12-12 21:35:05]

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-12-12 23:29:42]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1481578363966590,
"parent_rev": "38a0d9d782cfd28ef2347416468dda4dab737282", "commit_rev":
"8ddb06de03eded6cd6f2b5355babfd3c7922d259"}

[commit-bot: I haz the power, 2016-12-12 23:30:09]

Description was changed from

==========
IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

The goal of this CL is to modify content_browser's IMEDriver so we have access
to ui::InputMethodChromeOS's functionality in mus+ash.

This CL:
- Modifies ui::InputMethodChromeOS so it can call an ack callback after
  processing events.
- Introduces InputMethodBridge to acts as a bridge between
  ui::mojom::InputMethod and ui::InputMethodChromeOS.
- Introduces RemoteTextInputClient to send commands received from
  ui::InputMethodChromeOS to a remote client over mojo IPC.
- Modifies content_browser's IMEDriver implementation to use
  ui::InputMethodChromeOS via InputMethodBridge.
- Some unittests to verify that InputMethodBridge works correctly.

BUG=665981
==========

to

==========
IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

The goal of this CL is to modify content_browser's IMEDriver so we have access
to ui::InputMethodChromeOS's functionality in mus+ash.

This CL:
- Modifies ui::InputMethodChromeOS so it can call an ack callback after
  processing events.
- Introduces InputMethodBridge to acts as a bridge between
  ui::mojom::InputMethod and ui::InputMethodChromeOS.
- Introduces RemoteTextInputClient to send commands received from
  ui::InputMethodChromeOS to a remote client over mojo IPC.
- Modifies content_browser's IMEDriver implementation to use
  ui::InputMethodChromeOS via InputMethodBridge.
- Some unittests to verify that InputMethodBridge works correctly.

BUG=665981

Review-Url: https://codereview.chromium.org/2557493002
==========

[commit-bot: I haz the power, 2016-12-12 23:30:17]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-12-12 23:32:06]

Description was changed from

==========
IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

The goal of this CL is to modify content_browser's IMEDriver so we have access
to ui::InputMethodChromeOS's functionality in mus+ash.

This CL:
- Modifies ui::InputMethodChromeOS so it can call an ack callback after
  processing events.
- Introduces InputMethodBridge to acts as a bridge between
  ui::mojom::InputMethod and ui::InputMethodChromeOS.
- Introduces RemoteTextInputClient to send commands received from
  ui::InputMethodChromeOS to a remote client over mojo IPC.
- Modifies content_browser's IMEDriver implementation to use
  ui::InputMethodChromeOS via InputMethodBridge.
- Some unittests to verify that InputMethodBridge works correctly.

BUG=665981

Review-Url: https://codereview.chromium.org/2557493002
==========

to

==========
IME for Mus: Use ui::InputMethodChromeOS to provide logic for ime driver.

The goal of this CL is to modify content_browser's IMEDriver so we have access
to ui::InputMethodChromeOS's functionality in mus+ash.

This CL:
- Modifies ui::InputMethodChromeOS so it can call an ack callback after
  processing events.
- Introduces InputMethodBridge to acts as a bridge between
  ui::mojom::InputMethod and ui::InputMethodChromeOS.
- Introduces RemoteTextInputClient to send commands received from
  ui::InputMethodChromeOS to a remote client over mojo IPC.
- Modifies content_browser's IMEDriver implementation to use
  ui::InputMethodChromeOS via InputMethodBridge.
- Some unittests to verify that InputMethodBridge works correctly.

BUG=665981

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

[commit-bot: I haz the power, 2016-12-12 23:32:08]

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