IPC: Get rid of FileDescriptor usage from FileDescriptorSet and Message

IPC: Get rid of FileDescriptor usage from FileDescriptorSet and Message

This is a step toward to killing FileDescriptor.
This change lets FiileDescriptorSet have both Files (for owning fds)
and PlatformFiles (for non-owning fds). Doing this, we no longer
need FileDescriptor which provides |auto_close| flag.

BUG=415294
TEST=ipc_tests, ipc_mojo_unittests
R=agl@chromium.org, jam@hcromium.org, viettrungluu@chromium.org

Committed: https://crrev.com/96693856edc35a538f6ea8f0bd6ab55c75e3e823
Cr-Commit-Position: refs/heads/master@{#296498}

[agl, 2014-09-18 01:17:26]

https://codereview.chromium.org/583473002/diff/1/ipc/file_descriptor_set_posi...
File ipc/file_descriptor_set_posix.cc (left):

https://codereview.chromium.org/583473002/diff/1/ipc/file_descriptor_set_posi...
ipc/file_descriptor_set_posix.cc:86: // There's one more wrinkle: When logging
messages, we may reparse them. So
If you're removing this (is that safe?) then the comment for this function in
the .h file should also be updated.

https://codereview.chromium.org/583473002/diff/1/ipc/ipc_message_utils.cc
File ipc/ipc_message_utils.cc (left):

https://codereview.chromium.org/583473002/diff/1/ipc/ipc_message_utils.cc#old...
ipc/ipc_message_utils.cc:478: if (!valid) {
One hopes this change is correct, but it looks reasonable.

[Hajime Morrita, 2014-09-18 01:29:08]

Thanks for the quick review Adam!
PTAL?

https://codereview.chromium.org/583473002/diff/1/ipc/file_descriptor_set_posi...
File ipc/file_descriptor_set_posix.cc (left):

https://codereview.chromium.org/583473002/diff/1/ipc/file_descriptor_set_posi...
ipc/file_descriptor_set_posix.cc:86: // There's one more wrinkle: When logging
messages, we may reparse them. So
On 2014/09/18 01:17:26, agl wrote:
> If you're removing this (is that safe?) then the comment for this function in
> the .h file should also be updated.

Done.

https://codereview.chromium.org/583473002/diff/1/ipc/ipc_message_utils.cc
File ipc/ipc_message_utils.cc (left):

https://codereview.chromium.org/583473002/diff/1/ipc/ipc_message_utils.cc#old...
ipc/ipc_message_utils.cc:478: if (!valid) {
On 2014/09/18 01:17:26, agl wrote:
> One hopes this change is correct, but it looks reasonable.
Well, I'll preserve existing code for now and change it in another CL.

[agl, 2014-09-18 17:42:32]

LGTM

One nit below. LGTM in order to minimise delay.

https://codereview.chromium.org/583473002/diff/20001/ipc/ipc_message_utils.cc
File ipc/ipc_message_utils.cc (right):

https://codereview.chromium.org/583473002/diff/20001/ipc/ipc_message_utils.cc...
ipc/ipc_message_utils.cc:488: return true;
But now, *r is uninitialised!

[Hajime Morrita, 2014-09-18 17:58:50]

Thanks for the review!
Also, I'm renaming GetDescriptors() to PeekDescriptors()
so that it doesn't transfer the ownership.

https://codereview.chromium.org/583473002/diff/20001/ipc/ipc_message_utils.cc
File ipc/ipc_message_utils.cc (right):

https://codereview.chromium.org/583473002/diff/20001/ipc/ipc_message_utils.cc...
ipc/ipc_message_utils.cc:488: return true;
On 2014/09/18 17:42:32, agl wrote:
> But now, *r is uninitialised!
It is done at the beginning of the function!

[agl, 2014-09-18 18:07:19]

lgtm

https://codereview.chromium.org/583473002/diff/20001/ipc/ipc_message_utils.cc
File ipc/ipc_message_utils.cc (right):

https://codereview.chromium.org/583473002/diff/20001/ipc/ipc_message_utils.cc...
ipc/ipc_message_utils.cc:488: return true;
On 2014/09/18 17:58:50, morrita wrote:
> On 2014/09/18 17:42:32, agl wrote:
> > But now, *r is uninitialised!
> It is done at the beginning of the function!

Ah, indeed!

[Hajime Morrita, 2014-09-18 19:24:01]

Oops. Turned out this doesn't work :-(
The approach isn't bad. But File::Close() cannot be called on IO(IPC) thread
because File is for... FILE thread.
I'll do some prep work on base/ and come back.

[Hajime Morrita, 2014-09-18 19:51:37]

OK, I replaced File with ScopedFD, This is actually better.
Would you mind to take a quick look again and rubberstamp?
Thanks!

[Hajime Morrita, 2014-09-18 21:30:08]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-09-18 21:35:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/583473002/40001

[commit-bot: I haz the power, 2014-09-18 23:39:01]

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

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

Exceeded time limit waiting for builds to trigger.

[agl, 2014-09-18 23:42:42]

lgtm

[Hajime Morrita, 2014-09-18 23:48:04]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-09-18 23:51:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/583473002/40001

[commit-bot: I haz the power, 2014-09-19 01:13:10]

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

[commit-bot: I haz the power, 2014-09-19 01:13:11]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (None)

[Hajime Morrita, 2014-09-19 20:47:45]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-09-19 20:49:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/583473002/60001

[commit-bot: I haz the power, 2014-09-19 21:33:47]

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

[commit-bot: I haz the power, 2014-09-19 21:33:49]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Hajime Morrita, 2014-09-19 23:57:48]

Adam: I'm sorry to bother you but could you take another look?
It turned out that the "read twice" behavior is used by NaclIPCAdaptor
and we cannnot kill it. So I pulled it back.

[Hajime Morrita, 2014-09-24 19:29:44]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-09-24 19:30:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/583473002/80001

[commit-bot: I haz the power, 2014-09-24 20:11:52]

Committed patchset #5 (id:80001) as 4cf312b753490416d0f6953478e9c4e893e05b1f

[commit-bot: I haz the power, 2014-09-24 20:12:33]

Patchset 5 (id:??) landed as
https://crrev.com/96693856edc35a538f6ea8f0bd6ab55c75e3e823
Cr-Commit-Position: refs/heads/master@{#296498}