IPC::Message Refactoring: Move POSIX specific bits to PlatformFileAttachment

IPC::Message Refactoring: Move POSIX specific bits to PlatformFileAttachment

This change:

 * Gets rid of MessageAttachmentSet::owning_descriptors_ by moving
   the responsibility to PlatformFileAttachment, where owning_
   member variable is used to track the lifetime
 * Replaces MessageAttachmetnSet::Add*File() and TakePlatformFile()
   with platform agnostic AddAttachment() and GetAttachmentAt()
 * Repalces Message::ReadFile(), Write*File() with ReadAttachment()
   and WriteAttachmente(), which are also platform agnostic.

This also adds MessageAttachment::TakePlatformFile() virtual function,
which will be implemented by upcoming MojoHandleAttachment as well.

This is another preparation for introducing MojoHandleAttachment,
but it also simplifies Message and MessageAttachmentSet.

R=agl@chromium.org
TBR=mtomasz@chromium.org
BUG=448190
Committed: https://crrev.com/1aa788c1f3c5f356b4e06110b5780a1de99c4cd7
Cr-Commit-Position: refs/heads/master@{#314047}

[Hajime Morrita, 2015-01-30 00:34:43]

morrita@chromium.org changed reviewers:
+ mtomasz@chromium.org

[mtomasz, 2015-01-30 08:00:24]

lgtm for file_system_api.h

[agl, 2015-01-30 19:34:02]

https://codereview.chromium.org/883093003/diff/100001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/BUILD.gn#newcode79
ipc/BUILD.gn:79: "ipc_platform_file_attachment.cc",
Why aren't these built on iOS. Is iOS not POSIX?

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message.h
File ipc/ipc_message.h (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message.h#newco...
ipc/ipc_message.h:169: bool WriteAttachment(scoped_refptr<MessageAttachment>
attachment);
comments for these?

// WriteAttachment appends |attachment| to the end of the set. It returns false
iff the set is full.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message.h#newco...
ipc/ipc_message.h:170: bool ReadAttachment(PickleIterator* iter,
// ReadAttachment parses an attachment given the parsing state |iter| and writes
it to |*attachment|. It returns true on success.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
File ipc/ipc_message_attachment_set.cc (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
ipc/ipc_message_attachment_set.cc:130: for (unsigned i = 0; i <
attachments_.size(); ++i) {
s/unsigned/size_t/

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
File ipc/ipc_message_attachment_set.h (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
ipc/ipc_message_attachment_set.h:26: // a platform file or a mojo handle, can be
transmitted over underlying UNIX
"the" before "underlying"

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
ipc/ipc_message_attachment_set.h:48: //   returns: an attachments, or nullptr on
error
s/attachments/attachment/

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_platform_file_a...
File ipc/ipc_platform_file_attachment.h (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_platform_file_a...
ipc/ipc_platform_file_attachment.h:1: // Copyright (c) 2015 The Chromium
Authors. All rights reserved.
Should this file now be named with _posix in the name?

[Hajime Morrita, 2015-01-30 21:01:29]

PTAL?

https://codereview.chromium.org/883093003/diff/100001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/BUILD.gn#newcode79
ipc/BUILD.gn:79: "ipc_platform_file_attachment.cc",
On 2015/01/30 19:34:02, agl wrote:
> Why aren't these built on iOS. Is iOS not POSIX?
Uh, dunno. I just did piggyback what we already have.
Probably I should rename this to *_posix.cc so that
the system automatically takes care of it. Will do.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message.h
File ipc/ipc_message.h (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message.h#newco...
ipc/ipc_message.h:169: bool WriteAttachment(scoped_refptr<MessageAttachment>
attachment);
On 2015/01/30 19:34:02, agl wrote:
> comments for these?
> 
> // WriteAttachment appends |attachment| to the end of the set. It returns
false
> iff the set is full.

Thank you for the suggestion! Adopted.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message.h#newco...
ipc/ipc_message.h:170: bool ReadAttachment(PickleIterator* iter,
On 2015/01/30 19:34:02, agl wrote:
> // ReadAttachment parses an attachment given the parsing state |iter| and
writes
> it to |*attachment|. It returns true on success.

Done.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
File ipc/ipc_message_attachment_set.cc (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
ipc/ipc_message_attachment_set.cc:130: for (unsigned i = 0; i <
attachments_.size(); ++i) {
On 2015/01/30 19:34:02, agl wrote:
> s/unsigned/size_t/

Done.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
File ipc/ipc_message_attachment_set.h (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
ipc/ipc_message_attachment_set.h:26: // a platform file or a mojo handle, can be
transmitted over underlying UNIX
On 2015/01/30 19:34:02, agl wrote:
> "the" before "underlying"

Done.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_message_attachm...
ipc/ipc_message_attachment_set.h:48: //   returns: an attachments, or nullptr on
error
On 2015/01/30 19:34:02, agl wrote:
> s/attachments/attachment/

Done.

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_platform_file_a...
File ipc/ipc_platform_file_attachment.h (right):

https://codereview.chromium.org/883093003/diff/100001/ipc/ipc_platform_file_a...
ipc/ipc_platform_file_attachment.h:1: // Copyright (c) 2015 The Chromium
Authors. All rights reserved.
On 2015/01/30 19:34:02, agl wrote:
> Should this file now be named with _posix in the name?
Yes. You're right.

[agl, 2015-01-30 21:57:55]

LGTM.

I don't see the updated filenames in the CL listing, although I do see them in
the .gypi and GN files. So LGTM assuming that it's something funny with
codereview.

[Hajime Morrita, 2015-01-30 22:39:05]

Thanks for the review!

On 2015/01/30 21:57:55, agl wrote:
> LGTM.
> 
> I don't see the updated filenames in the CL listing, although I do see them in
> the .gypi and GN files. So LGTM assuming that it's something funny with
> codereview.
Ugh, I did wrong with git :-( Adding it and landing...

[Hajime Morrita, 2015-01-30 22:39:13]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2015-01-30 22:40:19]

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

[commit-bot: I haz the power, 2015-01-30 22:43:16]

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

[commit-bot: I haz the power, 2015-01-30 22:43:17]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...)
  ios_rel_device_ninja_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Hajime Morrita, 2015-01-30 23:28:18]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2015-01-30 23:29:20]

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

[commit-bot: I haz the power, 2015-01-30 23:39:14]

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

[commit-bot: I haz the power, 2015-01-30 23:39:15]

Try jobs failed on following builders:
  win8_chromium_gn_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_gn_...)

[Hajime Morrita, 2015-01-30 23:59:02]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2015-01-30 23:59:57]

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

[commit-bot: I haz the power, 2015-01-31 05:46:11]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-01-31 05:48:14]

Patchset 10 (id:??) landed as
https://crrev.com/1aa788c1f3c5f356b4e06110b5780a1de99c4cd7
Cr-Commit-Position: refs/heads/master@{#314047}