ipc: Change the timing of the construction of the attachment broker.

ipc: Change the timing of the construction of the attachment broker.

This CL ensures that the privileged attachment broker is constructed earlier in
the process of creating a child process. This fixes a race condition for single
process mode where the child thread's attachment broker would become the global
attachment broker.

BUG=534539
Committed: https://crrev.com/8758b4bd5bd210552606fa39d084aef7e786abd1
Cr-Commit-Position: refs/heads/master@{#350899}

[erikchen, 2015-09-24 21:28:23]

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

[erikchen, 2015-09-24 21:28:24]

avi: Please review.

[erikchen, 2015-09-25 01:13:31]

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

[erikchen, 2015-09-25 01:13:31]

tsepez: Please review.

[Avi (use Gerrit), 2015-09-25 14:29:27]

lgtm

Nice!

[Tom Sepez, 2015-09-25 15:37:34]

lgtm

https://codereview.chromium.org/1367013003/diff/1/content/common/child_proces...
File content/common/child_process_host_impl.cc (right):

https://codereview.chromium.org/1367013003/diff/1/content/common/child_proces...
content/common/child_process_host_impl.cc:116: // life cycle of making a child
process. This ensures that when a test is
nit: just "life cycle of a child process".

https://codereview.chromium.org/1367013003/diff/1/content/common/child_proces...
content/common/child_process_host_impl.cc:117: // being run one of the single
process modes, the global attachment broker is
nit: "being run in one of the"

https://codereview.chromium.org/1367013003/diff/1/ipc/attachment_broker.cc
File ipc/attachment_broker.cc (right):

https://codereview.chromium.org/1367013003/diff/1/ipc/attachment_broker.cc#ne...
ipc/attachment_broker.cc:17: CHECK(!g_attachment_broker);
check vs. DCHECK? your call.  I'd expect a DCHECK.

[erikchen, 2015-09-25 19:27:04]

https://codereview.chromium.org/1367013003/diff/1/content/common/child_proces...
File content/common/child_process_host_impl.cc (right):

https://codereview.chromium.org/1367013003/diff/1/content/common/child_proces...
content/common/child_process_host_impl.cc:116: // life cycle of making a child
process. This ensures that when a test is
On 2015/09/25 15:37:34, Tom Sepez wrote:
> nit: just "life cycle of a child process".

Done.

https://codereview.chromium.org/1367013003/diff/1/content/common/child_proces...
content/common/child_process_host_impl.cc:117: // being run one of the single
process modes, the global attachment broker is
On 2015/09/25 15:37:34, Tom Sepez wrote:
> nit: "being run in one of the"

Done.

https://codereview.chromium.org/1367013003/diff/1/ipc/attachment_broker.cc
File ipc/attachment_broker.cc (right):

https://codereview.chromium.org/1367013003/diff/1/ipc/attachment_broker.cc#ne...
ipc/attachment_broker.cc:17: CHECK(!g_attachment_broker);
On 2015/09/25 15:37:34, Tom Sepez wrote:
> check vs. DCHECK? your call.  I'd expect a DCHECK.

I think this is important to check, even in official release builds.

[erikchen, 2015-09-25 19:27:13]

The CQ bit was checked by erikchen@chromium.org

[erikchen, 2015-09-25 19:27:13]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1367013003/#ps40001
(title: "Comments from tsepez.")

[commit-bot: I haz the power, 2015-09-25 19:27:50]

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

[commit-bot: I haz the power, 2015-09-25 20:35:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-09-25 20:36:41]

Patchset 3 (id:??) landed as
https://crrev.com/8758b4bd5bd210552606fa39d084aef7e786abd1
Cr-Commit-Position: refs/heads/master@{#350899}

[erikchen, 2015-09-28 20:25:05]

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

The reason for reverting is: Still causes problems in at least one Dr. Memory
test. https://code.google.com/p/chromium/issues/detail?id=536891
Requires further investigation - Reverting for now.