Really de-flake ChildProcessLauncherTest#testBindServiceFromMultipleProcesses.

Really de-flake ChildProcessLauncherTest#testBindServiceFromMultipleProcesses.

The previous attempt was a bad band-aid around this underlying issue:
there are multiple paths to enter
ChildProcessConnectionImpl.doSetupConnection(). Failing to
bindToCaller() was only preventing one of those paths in
onServiceConnected(). The connection now tracks this and does not call
setupConnection() if bindToCaller() failed.

BUG=683133, 685052
R=boliu@chromium.org

Review-Url: https://codereview.chromium.org/2655463012
Cr-Commit-Position: refs/heads/master@{#446451}
Committed: https://chromium.googlesource.com/chromium/src/+/4a79d8da197d36b7b28ce24da4e694a12c787183

[Robert Sesek, 2017-01-26 16:31:56]

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

[commit-bot: I haz the power, 2017-01-26 16:32:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 17:13:24]

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

[commit-bot: I haz the power, 2017-01-26 17:13:26]

Dry run: This issue passed the CQ dry run.

[boliu, 2017-01-26 19:00:35]

lgtm

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
File
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java
(right):

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java:418:
allChildrenConnected &= conn == null;
is there any reason to check this?

[Robert Sesek, 2017-01-26 19:27:01]

Thanks!

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
File
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java
(right):

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java:418:
allChildrenConnected &= conn == null;
On 2017/01/26 19:00:35, boliu wrote:
> is there any reason to check this?

Just to ensure the retry only happens once.

[Robert Sesek, 2017-01-26 19:27:04]

The CQ bit was checked by rsesek@chromium.org

[commit-bot: I haz the power, 2017-01-26 19:27:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[boliu, 2017-01-26 19:30:28]

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
File
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java
(right):

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java:418:
allChildrenConnected &= conn == null;
On 2017/01/26 19:27:01, Robert Sesek wrote:
> On 2017/01/26 19:00:35, boliu wrote:
> > is there any reason to check this?
> 
> Just to ensure the retry only happens once.

You can check that after the poll, this just makes the poll more brittle?

[Robert Sesek, 2017-01-26 19:33:06]

The CQ bit was unchecked by rsesek@chromium.org

[Robert Sesek, 2017-01-26 19:47:24]

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
File
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java
(right):

https://codereview.chromium.org/2655463012/diff/1/content/public/android/java...
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java:418:
allChildrenConnected &= conn == null;
On 2017/01/26 19:30:28, boliu wrote:
> On 2017/01/26 19:27:01, Robert Sesek wrote:
> > On 2017/01/26 19:00:35, boliu wrote:
> > > is there any reason to check this?
> > 
> > Just to ensure the retry only happens once.
> 
> You can check that after the poll, this just makes the poll more brittle?

Done.

[Robert Sesek, 2017-01-26 20:24:28]

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

[commit-bot: I haz the power, 2017-01-26 20:25:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 21:11:41]

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

[commit-bot: I haz the power, 2017-01-26 21:11:42]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2017-01-26 21:16:39]

The CQ bit was checked by rsesek@chromium.org

[Robert Sesek, 2017-01-26 21:16:39]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2655463012/#ps20001
(title: "Simpler poll criteria.")

[commit-bot: I haz the power, 2017-01-26 21:17:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 21:22:37]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485465399148650,
"parent_rev": "8fb5e88be8e7123c26507d88584399e6b237be1d", "commit_rev":
"4a79d8da197d36b7b28ce24da4e694a12c787183"}

[commit-bot: I haz the power, 2017-01-26 21:23:09]

Description was changed from

==========
Really de-flake ChildProcessLauncherTest#testBindServiceFromMultipleProcesses.

The previous attempt was a bad band-aid around this underlying issue:
there are multiple paths to enter
ChildProcessConnectionImpl.doSetupConnection(). Failing to
bindToCaller() was only preventing one of those paths in
onServiceConnected(). The connection now tracks this and does not call
setupConnection() if bindToCaller() failed.

BUG=683133, 685052
R=boliu@chromium.org
==========

to

==========
Really de-flake ChildProcessLauncherTest#testBindServiceFromMultipleProcesses.

The previous attempt was a bad band-aid around this underlying issue:
there are multiple paths to enter
ChildProcessConnectionImpl.doSetupConnection(). Failing to
bindToCaller() was only preventing one of those paths in
onServiceConnected(). The connection now tracks this and does not call
setupConnection() if bindToCaller() failed.

BUG=683133, 685052
R=boliu@chromium.org

Review-Url: https://codereview.chromium.org/2655463012
Cr-Commit-Position: refs/heads/master@{#446451}
Committed:
https://chromium.googlesource.com/chromium/src/+/4a79d8da197d36b7b28ce24da4e6...
==========

[commit-bot: I haz the power, 2017-01-26 21:23:10]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4a79d8da197d36b7b28ce24da4e6...