Enable ChildProcessLauncherTest#testPendingSpawnQueue()

Enable ChildProcessLauncherTest#testPendingSpawnQueue()

The test failed because the child process was not properly initialized and
crashed. Made the child process wait in connection setup until being killed.

A similar scenario can make testServiceCrashedAfterSetup() fail. Updated it to
wait as well.

BUG=483089
Committed: https://crrev.com/81c0851e7b5bc688b0cc3e5d316bd39a6877ac4a
Cr-Commit-Position: refs/heads/master@{#371412}

[pasko, 2016-01-25 18:40:40]

pasko@chromium.org changed reviewers:
+ lizeb@chromium.org, yfriedman@chromium.org

[pasko, 2016-01-25 18:40:40]

yfriedman: please review
lizeb: fyi

[Yaron, 2016-01-25 19:13:00]

lgtm

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

https://codereview.chromium.org/1634753002/diff/20001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java:25:
// channels that are not being set up in this test.
so the idea is to just force the renderer to block until we're satisfied? Seems
viable but could break things if we ever ran multiple tests in one invocation.
:)

[pasko, 2016-01-25 19:38:04]

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

https://codereview.chromium.org/1634753002/diff/20001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java:25:
// channels that are not being set up in this test.
Thank you for fast review!!

On 2016/01/25 19:12:59, Yaron wrote:
> so the idea is to just force the renderer to block until we're satisfied?

Yes.

> Seems viable but could break things if we ever ran multiple tests in one
invocation.
> :)

Do you mean that a child service leaked from one test will confuse various
connectedServicesCountForTesting() checks of another test?

I assumed that we restart the browser process between tests, but I am not sure
we are actually doing that. If not, this may be a reason for another flake.

If there is a way to kill all child processes after a test, I'd definitely love
to do that, otherwise suspicious that we will need to mock out process creation
entirely.

Anyway, I think this CL is a step towards less flake. WDYT?

[pasko, 2016-01-25 19:42:37]

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

[commit-bot: I haz the power, 2016-01-25 19:43:35]

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

[Yaron, 2016-01-25 20:43:51]

lgtm

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

https://codereview.chromium.org/1634753002/diff/20001/content/public/android/...
content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java:25:
// channels that are not being set up in this test.
On 2016/01/25 19:38:04, pasko wrote:
> Thank you for fast review!!
> 
> On 2016/01/25 19:12:59, Yaron wrote:
> > so the idea is to just force the renderer to block until we're satisfied?
> 
> Yes.
> 
> > Seems viable but could break things if we ever ran multiple tests in one
> invocation.
> > :)
> 
> Do you mean that a child service leaked from one test will confuse various
> connectedServicesCountForTesting() checks of another test?

Yep.
> 
> I assumed that we restart the browser process between tests, but I am not sure
> we are actually doing that. If not, this may be a reason for another flake.

No, you're right, we do. There are ways to batch tests so they don't have to go
through a full restart but we don't employ them. I would love to it cause it
would reduce cycle time immensely but it's hard in our current environment.
> 
> If there is a way to kill all child processes after a test, I'd definitely
love
> to do that, otherwise suspicious that we will need to mock out process
creation
> entirely.
> 
> Anyway, I think this CL is a step towards less flake. WDYT?

[commit-bot: I haz the power, 2016-01-25 20:56:20]

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

[commit-bot: I haz the power, 2016-01-25 20:56:21]

Dry run: This issue passed the CQ dry run.

[pasko, 2016-01-26 01:29:25]

The CQ bit was checked by pasko@chromium.org

[commit-bot: I haz the power, 2016-01-26 01:33:38]

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

[commit-bot: I haz the power, 2016-01-26 01:44:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-26 01:47:10]

Description was changed from

==========
Enable ChildProcessLauncherTest#testPendingSpawnQueue()

The test failed because the child process was not properly initialized and
crashed. Made the child process wait in connection setup until being killed.

A similar scenario can make testServiceCrashedAfterSetup() fail. Updated it to
wait as well.

BUG=483089
==========

to

==========
Enable ChildProcessLauncherTest#testPendingSpawnQueue()

The test failed because the child process was not properly initialized and
crashed. Made the child process wait in connection setup until being killed.

A similar scenario can make testServiceCrashedAfterSetup() fail. Updated it to
wait as well.

BUG=483089

Committed: https://crrev.com/81c0851e7b5bc688b0cc3e5d316bd39a6877ac4a
Cr-Commit-Position: refs/heads/master@{#371412}
==========

[commit-bot: I haz the power, 2016-01-26 01:47:11]

Patchset 2 (id:??) landed as
https://crrev.com/81c0851e7b5bc688b0cc3e5d316bd39a6877ac4a
Cr-Commit-Position: refs/heads/master@{#371412}