[IME] Releases key events to default handler if no handler in the active IME extension.

[IME] Releases key events to Chromium's default behavior if no handler defined in the active IME extension.

BUG=337082
TEST=Locally verified in sandbox.

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

[Shu Chen, 2014-02-13 07:21:19]

Hiro, can you please review this cl?

Thanks,
Shu

[Hiro Komatsu, 2014-02-13 09:39:50]

Please describe whose handler is mentioned in the description just in case.  I
guess it's onKeyEvent's though.

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:167: if
(!extensions::ExtensionSystem::Get(profile_)
Please add a comment above this block.

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:171:
InputMethodEngineInterface* engine =
How about calling InputImeEventRouter::OnKeyEventHandled?

[Shu Chen, 2014-02-13 13:13:28]

On 2014/02/13 09:39:50, Hiro Komatsu wrote:
> Please describe whose handler is mentioned in the description just in case.  I
> guess it's onKeyEvent's though.
Sorry for misleading. For "default handler", I meant the default behavior of
chrome.
> 
>
https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
> File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):
> 
>
https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
> chrome/browser/extensions/api/input_ime/input_ime_api.cc:167: if
> (!extensions::ExtensionSystem::Get(profile_)
> Please add a comment above this block.
> 
>
https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
> chrome/browser/extensions/api/input_ime/input_ime_api.cc:171:
> InputMethodEngineInterface* engine =
> How about calling InputImeEventRouter::OnKeyEventHandled?

[Shu Chen, 2014-02-13 13:13:41]

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:167: if
(!extensions::ExtensionSystem::Get(profile_)
On 2014/02/13 09:39:50, Hiro Komatsu wrote:
> Please add a comment above this block.

Done.

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:171:
InputMethodEngineInterface* engine =
I had thought about it. To use InputImeEventRouter::OnKeyEventHandled(), the
below InputImeEventRouter::AddRequest() should be called. InputImeEventRouter
maintains an internal map to retrieve the key_data from the request_id.
For performance consideration, I would prefer to return here sooner rather than
reuse InputImeEventRouter.

On 2014/02/13 09:39:50, Hiro Komatsu wrote:
> How about calling InputImeEventRouter::OnKeyEventHandled?

[Hiro Komatsu, 2014-02-14 05:04:06]

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:171:
InputMethodEngineInterface* engine =
On 2014/02/13 13:13:41, Shu Chen wrote:
> I had thought about it. To use InputImeEventRouter::OnKeyEventHandled(), the
> below InputImeEventRouter::AddRequest() should be called. InputImeEventRouter
> maintains an internal map to retrieve the key_data from the request_id.
> For performance consideration, I would prefer to return here sooner rather
than
> reuse InputImeEventRouter.
> 
> On 2014/02/13 09:39:50, Hiro Komatsu wrote:
> > How about calling InputImeEventRouter::OnKeyEventHandled?

I see the point.  Since AddRequest is not so expensive and this speed does not
matter for user's typing, I prefer using the existing function.

[Shu Chen, 2014-02-14 05:27:11]

On 2014/02/14 05:04:06, Hiro Komatsu wrote:
>
https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
> File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):
> 
>
https://codereview.chromium.org/162973002/diff/30001/chrome/browser/extension...
> chrome/browser/extensions/api/input_ime/input_ime_api.cc:171:
> InputMethodEngineInterface* engine =
> On 2014/02/13 13:13:41, Shu Chen wrote:
> > I had thought about it. To use InputImeEventRouter::OnKeyEventHandled(), the
> > below InputImeEventRouter::AddRequest() should be called.
InputImeEventRouter
> > maintains an internal map to retrieve the key_data from the request_id.
> > For performance consideration, I would prefer to return here sooner rather
> than
> > reuse InputImeEventRouter.
> > 
> > On 2014/02/13 09:39:50, Hiro Komatsu wrote:
> > > How about calling InputImeEventRouter::OnKeyEventHandled?
> 
> I see the point.  Since AddRequest is not so expensive and this speed does not
> matter for user's typing, I prefer using the existing function.

OK. I've done a small refactoring. Please take another look.

[Hiro Komatsu, 2014-02-14 05:32:14]

lgtm

lgtm assuming you will add the 'const' keyword.

https://codereview.chromium.org/162973002/diff/160001/chrome/browser/extensio...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/162973002/diff/160001/chrome/browser/extensio...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:170: std::string
request_id = ime_event_router->AddRequest(engine_id, key_data);
const.

[Shu Chen, 2014-02-14 05:41:56]

https://codereview.chromium.org/162973002/diff/160001/chrome/browser/extensio...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/162973002/diff/160001/chrome/browser/extensio...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:170: std::string
request_id = ime_event_router->AddRequest(engine_id, key_data);
On 2014/02/14 05:32:15, Hiro Komatsu wrote:
> const.

Done.

[Shu Chen, 2014-02-14 05:42:40]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2014-02-14 05:43:14]

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

[commit-bot: I haz the power, 2014-02-14 15:02:59]

Change committed as 251320