[Ash] Introduces new IPC messages to support IME on Win-Ash

[Ash] Introduces new IPC messages to support IME on Win-Ash

This CL introduces new IPC messages between the browser process and the metro_driver process
to support IME functionality on Windows Chrome running under Ash.

Why we need new IPC messages?:
In Ash mode on Windows, we need to use raw IME API (to be precise, Text Services Framework)
in the WinRT  process (hereafter we call it as "metro_driver" process) rather than the
browser process due to some API limitations. However most of platform-neutral, abstracted
IME event handling code are implemented in the browser process. To cope with this situation,
we will use a proxy mechanism between the metro_driver process and the browser process.
In fact, this is what we have been doing between the browser process and the renderer
processes.

Here is the entire WIP CL which describes how these new IPC will be used in the real world.
https://codereview.chromium.org/53553003/

BUG=164964
TEST=none

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

[yukawa, 2013-11-18 14:25:01]

Hi Carlos,

Before sending the review request to security reviewers,
I'd like to get your comment for the new IPC messages
which I want to add to support IME on Ash.

You can see the entire CL in https://codereview.chromium.org/53553003/

Quick question:
In this CL, I intentionally didn't use gfx::Range nor
gfx::Rect because it seems that metro_driver is kept to be
independent from ui/* stuff.  Is this true?

[yukawa, 2013-11-20 22:02:33]

On 2013/11/18 14:25:01, yukawa wrote:
> Hi Carlos,
> 
> Before sending the review request to security reviewers,
> I'd like to get your comment for the new IPC messages
> which I want to add to support IME on Ash.
> 
> You can see the entire CL in https://codereview.chromium.org/53553003/
> 
> Quick question:
> In this CL, I intentionally didn't use gfx::Range nor
> gfx::Rect because it seems that metro_driver is kept to be
> independent from ui/* stuff.  Is this true?

Hi Carlos, Could you take a look at this CL?

[cpu_(ooo_6.6-7.5), 2013-11-22 02:43:44]

yes sorry I'll take a look. Do you plan to merger to m32?

[cpu_(ooo_6.6-7.5), 2013-11-22 02:45:33]

lgtm

You might need a rubber stamp security review but to note the IPC messages are
from a trusted process to trusted process.

[yukawa, 2013-11-22 04:23:39]

Thanks!

+jschuh@ for security review for ipc messages.
Justin, could you take a look?

[jschuh, 2013-11-22 14:15:00]

this is just privileged process to privileged process, right? if so, lgtm.

[yukawa, 2013-11-22 15:06:45]

On 2013/11/22 14:15:00, Justin Schuh wrote:
> this is just privileged process to privileged process, right? if so, lgtm.

Yes. They will be used between the browser process (medium integrity level) and
the metro_driver process (medium integrity level).
Thank you for the review.

[commit-bot: I haz the power, 2013-11-22 15:08:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yukawa@chromium.org/74783010/30001

[commit-bot: I haz the power, 2013-11-22 15:21:49]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[yukawa, 2013-11-22 15:37:48]

+sky@ for the owner review of ui/metro_viewer/ime_types.h and
ui/metro_viewer/metro_viewer.gyp.

Scott, could you take a look?

[sky, 2013-11-22 18:31:34]

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types.h
File ui/metro_viewer/ime_types.h (right):

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types...
ui/metro_viewer/ime_types.h:18: : start_offset(0), end_offset(0), thick(false)
{}
nit: when you wrap, one param per line (like 29-30 below).

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types...
ui/metro_viewer/ime_types.h:34: std::vector<UnderlineInfo> underlines;
Don't you want to more destructor definition to .cc? See 'Stop inlining
constructors and destructors' in style guide for details. The other two are
fine, but that this has a string and vector as members makes for a complex
destructor.

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types...
ui/metro_viewer/ime_types.h:41: : left(0), top(0), right(0), bottom(0) {}
same comment here about one per line.

[yukawa, 2013-11-23 03:44:04]

Thanks! PTAL?

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types.h
File ui/metro_viewer/ime_types.h (right):

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types...
ui/metro_viewer/ime_types.h:18: : start_offset(0), end_offset(0), thick(false)
{}
On 2013/11/22 18:31:34, sky wrote:
> nit: when you wrap, one param per line (like 29-30 below).

Done.

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types...
ui/metro_viewer/ime_types.h:34: std::vector<UnderlineInfo> underlines;
On 2013/11/22 18:31:34, sky wrote:
> Don't you want to more destructor definition to .cc? See 'Stop inlining
> constructors and destructors' in style guide for details. The other two are
> fine, but that this has a string and vector as members makes for a complex
> destructor.

Yes, totally agreed. 
I also put the other constructors into .cc to keep this header simple. There
never be performance issue since this header is used only on Windows where
link-time-code-generation is well established.

https://codereview.chromium.org/74783010/diff/30001/ui/metro_viewer/ime_types...
ui/metro_viewer/ime_types.h:41: : left(0), top(0), right(0), bottom(0) {}
On 2013/11/22 18:31:34, sky wrote:
> same comment here about one per line.

Done.

[sky, 2013-11-24 19:01:16]

LGTM

[commit-bot: I haz the power, 2013-11-24 19:26:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yukawa@chromium.org/74783010/430002

[commit-bot: I haz the power, 2013-11-24 22:41:25]

Change committed as 237010