Remove some PlatformFile uses from NaCl.

Remove some PlatformFile uses from NaCl.

BUG=322664
R=mseaborn@chromium.org, cpu@chromium.org

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

[rvargas (doing something else), 2014-02-14 04:00:31]

PTAL

[Brad Chen, 2014-02-14 06:51:04]

Sehr@ or mseaborn@ might be better reviewers for this than me.


On Fri, Feb 14, 2014 at 5:00 AM, <rvargas@chromium.org> wrote:

> Reviewers: Brad Chen (chromium), cpu,
>
> Message:
> PTAL
>
> Description:
> Remove some PlatformFile uses from NaCl.
>
> BUG=322664
> R=bradchen@chromium.org, cpu@chromium.org
>
> Please review this at https://codereview.chromium.org/165663002/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+78, -82 lines):
>   M components/nacl/browser/nacl_browser.h
>   M components/nacl/browser/nacl_browser.cc
>   M components/nacl/browser/nacl_file_host.cc
>   M components/nacl/browser/nacl_host_message_filter.h
>   M components/nacl/browser/nacl_host_message_filter.cc
>   M components/nacl/browser/nacl_process_host.h
>   M components/nacl/browser/nacl_process_host.cc
>   M components/nacl/browser/pnacl_host.h
>   M components/nacl/browser/pnacl_host.cc
>   M ipc/ipc_platform_file.h
>   M ipc/ipc_platform_file.cc
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[rvargas (doing something else), 2014-02-14 18:51:16]

+Mark

[Mark Seaborn, 2014-02-14 19:00:07]

LGTM

https://codereview.chromium.org/165663002/diff/1/components/nacl/browser/pnac...
File components/nacl/browser/pnacl_host.cc (right):

https://codereview.chromium.org/165663002/diff/1/components/nacl/browser/pnac...
components/nacl/browser/pnacl_host.cc:22: namespace {
Nit: add empty line after this so that the start and end of the namespace block
are consistent

https://codereview.chromium.org/165663002/diff/1/ipc/ipc_platform_file.h
File ipc/ipc_platform_file.h (right):

https://codereview.chromium.org/165663002/diff/1/ipc/ipc_platform_file.h#newc...
ipc/ipc_platform_file.h:44: IPC_EXPORT PlatformFileForTransit
GetFileHandleForProcess(
Might want a TODO to remove this one in favour of the new function?

[rvargas (doing something else), 2014-02-14 19:37:15]

Thanks.

https://codereview.chromium.org/165663002/diff/1/components/nacl/browser/pnac...
File components/nacl/browser/pnacl_host.cc (right):

https://codereview.chromium.org/165663002/diff/1/components/nacl/browser/pnac...
components/nacl/browser/pnacl_host.cc:22: namespace {
On 2014/02/14 19:00:08, Mark Seaborn wrote:
> Nit: add empty line after this so that the start and end of the namespace
block
> are consistent

oops. done.

https://codereview.chromium.org/165663002/diff/1/ipc/ipc_platform_file.h
File ipc/ipc_platform_file.h (right):

https://codereview.chromium.org/165663002/diff/1/ipc/ipc_platform_file.h#newc...
ipc/ipc_platform_file.h:44: IPC_EXPORT PlatformFileForTransit
GetFileHandleForProcess(
On 2014/02/14 19:00:08, Mark Seaborn wrote:
> Might want a TODO to remove this one in favour of the new function?

Initially I was thinking that would be the case, but there's a couple of callers
that don't want the handle closed and in that case passing File would not work,
so I think we'll need both versions long term (probably removing the last
argument from this version)

[cpu_(ooo_6.6-7.5), 2014-02-18 21:39:44]

lgtm

[rvargas (doing something else), 2014-02-18 21:42:34]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-02-18 21:43:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/165663002/150001

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

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

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

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

[rvargas (doing something else), 2014-02-19 14:56:17]

The CQ bit was checked by rvargas@chromium.org

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/165663002/150001

[commit-bot: I haz the power, 2014-02-19 15:09:31]

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

[commit-bot: I haz the power, 2014-02-19 15:09:31]

Try jobs failed on following builders: linux_chromium_rel, mac_chromium_rel

[rvargas (doing something else), 2014-03-03 20:06:09]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-03-03 20:09:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/165663002/150001

[commit-bot: I haz the power, 2014-03-03 20:13:37]

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

[commit-bot: I haz the power, 2014-03-03 20:13:38]

Try jobs failed on following builders: linux_chromium_rel, mac_chromium_rel

[rvargas (doing something else), 2014-03-04 22:51:15]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-03-04 22:51:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/165663002/150001

[commit-bot: I haz the power, 2014-03-05 01:09:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/165663002/150001

[commit-bot: I haz the power, 2014-03-05 08:18:20]

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

[commit-bot: I haz the power, 2014-03-05 08:18:21]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[rvargas (doing something else), 2014-03-05 18:43:58]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-03-05 18:45:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/165663002/150001

[commit-bot: I haz the power, 2014-03-05 20:13:51]

Change committed as 255131