Use renderer spellchecker for windows.

Use renderer spellchecker for windows.

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

[jam, 2009-11-11 02:01:37]

lgtm with these comments

http://codereview.chromium.org/372075/diff/2001/3004
File base/platform_file.h (right):

http://codereview.chromium.org/372075/diff/2001/3004#newcode24
Line 24: typedef PlatformFile PlatformFileForTransit;
Adding this typedef at this layer seems to break the existing abstractions,
since base shouldn't know anything about IPC.  Perhaps these should be moved off
to somewhere in chrome\common?

Also, probably good to get whoever is changing base\file* to look at these
changes.

[jam, 2009-11-11 02:01:56]

lgtm with these comments


http://codereview.chromium.org/372075/diff/2001/3004
File base/platform_file.h (right):

http://codereview.chromium.org/372075/diff/2001/3004#newcode24
Line 24: typedef PlatformFile PlatformFileForTransit;
Adding this typedef at this layer seems to break the existing
abstractions, since base shouldn't know anything about IPC.  Perhaps
these should be moved off to somewhere in chrome\common?

Also, probably good to get whoever is changing base\file* to look at
these changes.

http://codereview.chromium.org/372075

[Evan Stade, 2009-11-11 20:25:59]

http://codereview.chromium.org/372075/diff/2001/3004
File base/platform_file.h (right):

http://codereview.chromium.org/372075/diff/2001/3004#newcode24
Line 24: typedef PlatformFile PlatformFileForTransit;
On 2009/11/11 02:01:37, John Abd-El-Malek wrote:
> Adding this typedef at this layer seems to break the existing abstractions,
> since base shouldn't know anything about IPC.  Perhaps these should be moved
off
> to somewhere in chrome\common?

well I would say it actually sort of belongs in ipc/ but I could find no file
that seemed appropriate in either ipc/ or common/. I decided this was the
cleanest place, alternate suggestions welcome.

> 
> Also, probably good to get whoever is changing base\file* to look at these
> changes.

someone is changing base/file*? I could ask on chromium-dev but I haven't heard
anything about that.

[brettw, 2009-11-12 00:28:58]

On 2009/11/11 20:25:59, Evan Stade wrote:
> http://codereview.chromium.org/372075/diff/2001/3004
> File base/platform_file.h (right):
> 
> http://codereview.chromium.org/372075/diff/2001/3004#newcode24
> Line 24: typedef PlatformFile PlatformFileForTransit;
> On 2009/11/11 02:01:37, John Abd-El-Malek wrote:
> > Adding this typedef at this layer seems to break the existing abstractions,
> > since base shouldn't know anything about IPC.  Perhaps these should be moved
> off
> > to somewhere in chrome\common?

I think ipc/ipc_message_utils.h is the current best place for dumping this kind
of thing. However, I think I'd actually say a new file in ipc/ would be better
(to avoid bringing in PlatformFile to every IPC thing that doesn't need it.

[Evan Stade, 2009-11-12 21:25:25]

updated location of the typedef, fixed unit tests, all trybots are green.

[jam, 2009-11-12 21:59:17]

lgtm

http://codereview.chromium.org/372075/diff/2001/3004
File base/platform_file.h (right):

http://codereview.chromium.org/372075/diff/2001/3004#newcode24
Line 24: typedef PlatformFile PlatformFileForTransit;
On 2009/11/11 20:26:00, Evan Stade wrote:
> On 2009/11/11 02:01:37, John Abd-El-Malek wrote:
> > Adding this typedef at this layer seems to break the existing abstractions,
> > since base shouldn't know anything about IPC.  Perhaps these should be moved
> off
> > to somewhere in chrome\common?
> 
> well I would say it actually sort of belongs in ipc/ but I could find no file
> that seemed appropriate in either ipc/ or common/. I decided this was the
> cleanest place, alternate suggestions welcome.
> 
> > 
> > Also, probably good to get whoever is changing base\file* to look at these
> > changes.
> 
> someone is changing base/file*? I could ask on chromium-dev but I haven't
heard
> anything about that.

I meant by whoever has made recent changes to this file.  If you can check with
someone there that they're ok with the proposed solution before checking in,
that would be great.

[Evan Stade, 2009-11-12 22:02:47]

heh, 5 of the last 10 changes to file_util.h are me.

+Mark Mentovai: are you ok with the ipc/ changes?

[Mark Mentovai, 2009-11-12 22:15:30]

Seems fine.

http://codereview.chromium.org/372075/diff/9016/9036
File ipc/ipc_platform_file.cc (right):

http://codereview.chromium.org/372075/diff/9016/9036#newcode10
Line 10: const PlatformFileForTransit& transit) {
It's almost worth inlining this in the .h and not even adding a .cc, but it's
your call.

[Evan Stade, 2009-11-12 22:17:13]

http://codereview.chromium.org/372075/diff/9016/9036
File ipc/ipc_platform_file.cc (right):

http://codereview.chromium.org/372075/diff/9016/9036#newcode10
Line 10: const PlatformFileForTransit& transit) {
On 2009/11/12 22:15:30, Mark Mentovai wrote:
> It's almost worth inlining this in the .h and not even adding a .cc, but it's
> your call.

when I did that, the trybots complained about duplicate symbols