Make DispatchKeyEventPostIME() asynchronous.

Make DispatchKeyEventPostIME() asynchronous.

This change:
- Adds an overloaded version of InputMethodDelegate::DispatchKeyEventPostIME()
with a callback option.
- Uses the async version of DispatchKeyEventPostIME() in InputMethodChromeOS.
- Adds ui.mojom.TextInputClient.DispatchKeyEventPostIME().

This is because DispatchKeyEventPostIME() isn't really *Post* IME, and calls
to it are sometimes intervened with the logic in InputMethodChromeOS to 
provide KeyDown and KeyUp events in addition to KeyPress events to textfields.

This CL calls ui.mojom.TextInputClient.DispatchKeyEventPostIME() in these cases
and continues the logic after the dispatch finishes in the client side.

BUG=723792

[Hadi, 2017-06-30 16:01:34]

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

[commit-bot: I haz the power, 2017-06-30 16:01:52]

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, 2017-06-30 16:36:19]

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

[commit-bot: I haz the power, 2017-06-30 16:36:20]

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

[Hadi, 2017-06-30 19:10:51]

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

[commit-bot: I haz the power, 2017-06-30 19:11:22]

Dry run: CQ is trying da patch.

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

[Hadi, 2017-06-30 19:15:53]

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

[commit-bot: I haz the power, 2017-06-30 19:16:03]

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, 2017-06-30 20:30:07]

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

[commit-bot: I haz the power, 2017-06-30 20:30:08]

Dry run: Try jobs failed on following builders:
  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, 2017-07-04 20:08:32]

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

[commit-bot: I haz the power, 2017-07-04 20:08:46]

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, 2017-07-04 20:29:35]

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

[commit-bot: I haz the power, 2017-07-04 20:29:36]

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

[Hadi, 2017-07-04 20:41:24]

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

[commit-bot: I haz the power, 2017-07-04 20:41:27]

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, 2017-07-04 21:32:46]

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

[commit-bot: I haz the power, 2017-07-04 21:32:47]

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

[Hadi, 2017-07-05 17:52:15]

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

[commit-bot: I haz the power, 2017-07-05 17:52:33]

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, 2017-07-05 18:16:01]

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

[commit-bot: I haz the power, 2017-07-05 18:16:02]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Hadi, 2017-07-05 18:27:46]

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

[commit-bot: I haz the power, 2017-07-05 18:28:10]

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, 2017-07-05 18:53:43]

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

[commit-bot: I haz the power, 2017-07-05 18:53:44]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Hadi, 2017-07-05 19:20:07]

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

[commit-bot: I haz the power, 2017-07-05 19:20:22]

Dry run: CQ is trying da patch.

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

[Hadi, 2017-07-05 19:26:59]

Description was changed from

==========
Make DispatchKeyEventPostIME asynchronous.

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

to

==========
Make DispatchKeyEventPostIME() asynchronous.

This change:
- Adds an overloaded version of InputMethodDelegate::DispatchKeyEventPostIME()
with a callback option.
- Uses the async version of DispatchKeyEventPostIME() in InputMethodChromeOS.
- Adds ui.mojom.TextInputClient.DispatchKeyEventPostIME().

This is because DispatchKeyEventPostIME() isn't really *Post* IME, and calls
to it are sometimes intervened with the logic in InputMethodChromeOS to 
provide KeyDown and KeyUp events in addition to KeyPress events to textfields.

This CL calls ui.mojom.TextInputClient.DispatchKeyEventPostIME() in these cases
and continues the logic after the dispatch finishes in the client side.

BUG=723792
==========

[Hadi, 2017-07-05 19:31:33]

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

[Hadi, 2017-07-05 19:31:35]

PTAL.

Since I'm leaving Google in couple of days, I probably won't be able to land
this fix myself. Compiling Chromium in personal laptop takes forever :(

I hope this is useful for next owner. I'll try to add some test coverage while
I'm here.

[commit-bot: I haz the power, 2017-07-05 19:55:34]

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

[commit-bot: I haz the power, 2017-07-05 19:55:35]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Shu Chen, 2017-07-06 03:38:42]

https://codereview.chromium.org/2957173004/diff/180001/ui/base/ime/input_meth...
File ui/base/ime/input_method_delegate.h (right):

https://codereview.chromium.org/2957173004/diff/180001/ui/base/ime/input_meth...
ui/base/ime/input_method_delegate.h:28: virtual ui::EventDispatchDetails
DispatchKeyEventPostIME(
Anything makes you decide not to keep InputMethodDelegate as an interface?

[Shu Chen, 2017-07-07 01:01:11]

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

https://codereview.chromium.org/2957173004/diff/180001/ui/base/ime/input_meth...
ui/base/ime/input_method_chromeos.cc:136: ProcessKeyEventPostIME(event,
std::move(ack_callback), false, is_handled);
Here removes the original checks:
  if (event->type() == ET_KEY_PRESSED || event->type() == ET_KEY_RELEASED)

Is it intentional?

https://codereview.chromium.org/2957173004/diff/180001/ui/base/ime/input_meth...
ui/base/ime/input_method_chromeos.cc:391: return
DispatchKeyEventPostIME(&fabricated_event, std::move(callback));
Here skip the call to event->StopPropagation(), is it intentional?

[Shu Chen, 2017-07-07 01:14:20]

I'm concerned that this cl can easily cause regressions to CJK input methods.

I found some execution order may be changed by this cl.
For example, in ProcessFilteredKeyPressEvent():
Original:
if (NeedInsertChar()) return DispatchKeyEventPostIME(event);

This cl:
if (NeedInsertChar()) return DispatchKeyEventPostIME(event, callback);

The |callback| execute PostProcessFilteredKeyPressEvent() which does a lot.

I recommend to carefully test this cl, especially with CJK input methods.
You can use Chinese Pinyin IME as a typical one.

[Shu Chen, 2017-07-07 01:22:19]

Btw, I don't prefer the double async design like this:

ProcessFilteredKeyPressEvent -> DispatchKeyEventPostIME [async]->
PostProcessFilteredKeyPressEvent -> ProcessKeyEventPostIME ->
DispatchKeyEventPostIME [async]-> ack_callback.

[Shu Chen, 2017-07-07 01:24:25]

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

https://codereview.chromium.org/2957173004/diff/180001/ui/base/ime/input_meth...
ui/base/ime/input_method_chromeos.cc:330: return
ProcessFilteredKeyPressEvent(event, std::move(ack_callback));
Is this a potential dead loop?

ProcessFilteredKeyPressEvent -> DispatchKeyEventPostIME ->
PostProcessFilteredKeyPressEvent -> ProcessKeyEventPostIME ->
ProcessFilteredKeyPressEvent...

[Shu Chen, 2017-07-07 01:24:26]

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

https://codereview.chromium.org/2957173004/diff/180001/ui/base/ime/input_meth...
ui/base/ime/input_method_chromeos.cc:330: return
ProcessFilteredKeyPressEvent(event, std::move(ack_callback));
Is this a potential dead loop?

ProcessFilteredKeyPressEvent -> DispatchKeyEventPostIME ->
PostProcessFilteredKeyPressEvent -> ProcessKeyEventPostIME ->
ProcessFilteredKeyPressEvent...

[Shu Chen, 2017-07-07 01:28:00]

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

[Shu Chen, 2017-07-07 01:28:01]

+suzhe@ for his insight.