ipc: Clean up interface of attachment broker.

ipc: Clean up interface of attachment broker.

AttachmentBrokerUnprivileged now has a method
DesignateBrokerCommunicationChannel which is used by non-broker processes to
designate an IPC::Channel as the communication medium for brokerable attachment
messages. IPC::Channel has a new member attachment_broker_endpoint_ which causes
it to pass all messages through the attachment broker before forwarding them to
the listener.

BUG=493414
Committed: https://crrev.com/9a06836982214f2edced21bbd1615b49e7f231bf
Cr-Commit-Position: refs/heads/master@{#341143}

[erikchen, 2015-07-29 22:22:54]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-29 22:23:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1269553003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1269553003/1

[commit-bot: I haz the power, 2015-07-29 22:42:21]

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

[commit-bot: I haz the power, 2015-07-29 22:42:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[erikchen, 2015-07-29 23:18:15]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-29 23:18:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1269553003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1269553003/20001

[erikchen, 2015-07-29 23:22:07]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-29 23:22:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1269553003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1269553003/40001

[commit-bot: I haz the power, 2015-07-29 23:51:26]

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

[commit-bot: I haz the power, 2015-07-29 23:51:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[erikchen, 2015-07-30 00:18:43]

erikchen@chromium.org changed reviewers:
+ tsepez@chromium.org

[erikchen, 2015-07-30 00:18:43]

tsepez: Please review.

Patch set 3 passes the CQ (except for GN compile failures, which got addressed
in patch set 4).

[erikchen, 2015-07-30 00:18:54]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-30 00:19:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1269553003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1269553003/60001

[Tom Sepez, 2015-07-30 00:35:47]

This seems reasonable.

https://codereview.chromium.org/1269553003/diff/60001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/1269553003/diff/60001/ipc/BUILD.gn#newcode89
ipc/BUILD.gn:89: "ipc_channel_posix.cc",
Did you mean to do this?

https://codereview.chromium.org/1269553003/diff/60001/ipc/attachment_broker_u...
File ipc/attachment_broker_unprivileged.h (right):

https://codereview.chromium.org/1269553003/diff/60001/ipc/attachment_broker_u...
ipc/attachment_broker_unprivileged.h:24: // communicate brokerable attachments
with the broker process..
nit: ..

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc.gypi
File ipc/ipc.gypi (left):

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc.gypi#oldcode100
ipc/ipc.gypi:100: 'ipc_channel.cc',
ditto

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc_channel.cc
File ipc/ipc_channel.cc (right):

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc_channel.cc#newc...
ipc/ipc_channel.cc:13: #endif  // !defined(OS_NACL_SFI)
nit: maybe combine this with line 15

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc_channel.cc#newc...
ipc/ipc_channel.cc:26: Channel::Channel() : attachment_broker_endpoint_(false)
{}
Hmm.  If this were in the header, could we avoid the gyp/GN changes?

[commit-bot: I haz the power, 2015-07-30 00:48:04]

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

[commit-bot: I haz the power, 2015-07-30 00:48:05]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[erikchen, 2015-07-30 01:38:04]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[erikchen, 2015-07-30 01:38:19]

tsepez: PTAL

https://codereview.chromium.org/1269553003/diff/60001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/1269553003/diff/60001/ipc/BUILD.gn#newcode89
ipc/BUILD.gn:89: "ipc_channel_posix.cc",
On 2015/07/30 00:35:47, Tom Sepez wrote:
> Did you mean to do this?

Yes, but it's no longer relevant now that I moved the constructor to the header
of ipc_channel.

https://codereview.chromium.org/1269553003/diff/60001/ipc/attachment_broker_u...
File ipc/attachment_broker_unprivileged.h (right):

https://codereview.chromium.org/1269553003/diff/60001/ipc/attachment_broker_u...
ipc/attachment_broker_unprivileged.h:24: // communicate brokerable attachments
with the broker process..
On 2015/07/30 00:35:47, Tom Sepez wrote:
> nit: ..

Done.

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc.gypi
File ipc/ipc.gypi (left):

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc.gypi#oldcode100
ipc/ipc.gypi:100: 'ipc_channel.cc',
On 2015/07/30 00:35:47, Tom Sepez wrote:
> ditto

Yes, but it's no longer relevant now that I moved the constructor to the header
of ipc_channel.

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc_channel.cc
File ipc/ipc_channel.cc (right):

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc_channel.cc#newc...
ipc/ipc_channel.cc:13: #endif  // !defined(OS_NACL_SFI)
On 2015/07/30 00:35:47, Tom Sepez wrote:
> nit: maybe combine this with line 15

Moving the constructor to the header means these changes are no longer
necessary.

https://codereview.chromium.org/1269553003/diff/60001/ipc/ipc_channel.cc#newc...
ipc/ipc_channel.cc:26: Channel::Channel() : attachment_broker_endpoint_(false)
{}
On 2015/07/30 00:35:47, Tom Sepez wrote:
> Hmm.  If this were in the header, could we avoid the gyp/GN changes?

Good suggestion! Done.

[commit-bot: I haz the power, 2015-07-30 01:38:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1269553003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1269553003/80001

[commit-bot: I haz the power, 2015-07-30 01:52:21]

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

[commit-bot: I haz the power, 2015-07-30 01:52:21]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Tom Sepez, 2015-07-30 15:44:33]

lgtm

[erikchen, 2015-07-30 16:16:17]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-07-30 16:16:39]

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

[commit-bot: I haz the power, 2015-07-30 17:19:44]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-07-30 17:20:17]

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

[Avi (use Gerrit), 2015-07-30 19:37:45]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1259953005/ by avi@chromium.org.

The reason for reverting is: Causes failures in:

IPCAttachmentBrokerPrivilegedWinTest.SendHandle
IPCAttachmentBrokerPrivilegedWinTest.SendHandleToSelf
IPCAttachmentBrokerPrivilegedWinTest.SendHandleWithoutPermissions

on the Win7 Tests (dbg)(1) bot

http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28....

[erikchen, 2015-07-30 20:48:44]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1264543003/ by erikchen@chromium.org.

The reason for reverting is: Causes errors on win7.

http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28....