Implement IME for Mac plugins using the Cocoa event model on 10.6

Implement IME for Mac plugins using the Cocoa event model on 10.6

Based on the implementation of Gecko's Cocoa event model IME handling on 64-bit, but modified to fit Chrome's cross-process event handling.

Because NSTextInputContext is 10.6+, 10.5 will need a cempletely different implementation.

BUG=30905
TEST=Plugins implementing the updated NPAPI Cocoa IME spec should handle IME on 10.6.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62820

[stuartmorgan, 2010-10-13 20:12:48]

This isn't as big a review as it looks like; most of this is just rote plumbing
since I unfortunately need a path from the plugin all the way up to the browser
and then back again.

suzhe, because of the way Cocoa event model plugin IME works (we need to send
all the intermediate key events, then one final string when done, and we want to
show a popup window instead of doing it inline) it doesn't seem like there's any
way to make this anything other than totally separate from the normal IME, as
sad as that makes me. If you see any opportunities for sharing that I'm missing,
please let me know!

[James Su, 2010-10-14 01:47:39]

Great work! I'll review it asap, but it may take sometime.

On 2010/10/13 20:12:48, stuartmorgan wrote:
> This isn't as big a review as it looks like; most of this is just rote
plumbing
> since I unfortunately need a path from the plugin all the way up to the
browser
> and then back again.
> 
> suzhe, because of the way Cocoa event model plugin IME works (we need to send
> all the intermediate key events, then one final string when done, and we want
to
> show a popup window instead of doing it inline) it doesn't seem like there's
any
> way to make this anything other than totally separate from the normal IME, as
> sad as that makes me. If you see any opportunities for sharing that I'm
missing,
> please let me know!

[James Su, 2010-10-15 00:55:32]

Just reviewed the RWHV part,  which looks good to me except the following issue.

http://codereview.chromium.org/3808001/diff/3001/4001
File chrome/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/3808001/diff/3001/4001#newcode1220
chrome/browser/renderer_host/render_widget_host.cc:1220:
view_->PostProcessEventForPluginIme(front_item);
If a keyboard event is handled by the plugin ime, it should no longer be sent to
the browser by calling UnhandledKeyboardEvent() below.

[pink (ping after 24hrs), 2010-10-15 14:00:36]

lgtm

http://codereview.chromium.org/3808001/diff/3001/4019
File third_party/mozilla/LICENSE (right):

http://codereview.chromium.org/3808001/diff/3001/4019#newcode40
third_party/mozilla/LICENSE:40: * Copyright (C) 2009 Apple Inc. All Rights
Reserved.
do we need to let dannyB know we're adding something to third_party/ ?

[stuartmorgan, 2010-10-15 19:29:14]

New version up; addresses review comments and fixes a couple bugs:
- The respondsToSelector check didn't end up working; apparently the method
secretly existed on 10.5 (but returned a completely different object).
- The type check needed to be stricter, to avoid sending the wrong OS events
into NSTextInputContext.

http://codereview.chromium.org/3808001/diff/3001/4001
File chrome/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/3808001/diff/3001/4001#newcode1220
chrome/browser/renderer_host/render_widget_host.cc:1220:
view_->PostProcessEventForPluginIme(front_item);
On 2010/10/15 00:55:32, James Su wrote:
> If a keyboard event is handled by the plugin ime, it should no longer be sent
to
> the browser by calling UnhandledKeyboardEvent() below.

Good catch, thanks! I've added a handled state to PostProcessEventForPluginIme.

http://codereview.chromium.org/3808001/diff/3001/4019
File third_party/mozilla/LICENSE (right):

http://codereview.chromium.org/3808001/diff/3001/4019#newcode40
third_party/mozilla/LICENSE:40: * Copyright (C) 2009 Apple Inc. All Rights
Reserved.
On 2010/10/15 14:00:37, pink wrote:
> do we need to let dannyB know we're adding something to third_party/ ?

I spoke to DannyB; since this is WebKit's license, we already have this text. I
discovered that these files is processed by a tool, so this won't work, but
DannyB was fine with just letting it be covered by WebKit's license (the
alternative being to create a new directory just for this file, so it can have
it's own name).

Evan, since you seem to be the master of the tool: do you have any objection to
the approach in this patch (just noting that it's covered by the WebKit
license)?

[stuartmorgan, 2010-10-15 19:31:24]

Oops, helps to actually add people when asking them questions. Evan, see the
last part of my previous comment; I have an about:credits question.

[Evan Martin, 2010-10-15 19:42:16]

On 2010/10/15 19:31:24, stuartmorgan wrote:
> Oops, helps to actually add people when asking them questions. Evan, see the
> last part of my previous comment; I have an about:credits question.

I think that's fine.

[James Su, 2010-10-15 19:53:30]

LGTM

[pink (ping after 24hrs), 2010-10-15 19:54:40]

cocoa parts lgtm with style nit.

http://codereview.chromium.org/3808001/diff/1024/10005
File chrome/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/3808001/diff/1024/10005#newcode2533
chrome/browser/renderer_host/render_widget_host_view_mac.mm:2533:
ComplexTextInputPanel* input_panel =
in objc, use objC naming (inputPanel, compositedString, etc