Factor out inner-classes out of ChildProcessLauncher.

Factor out inner-classes out of ChildProcessLauncher.

Factoring out ChildProcessAllocator and SpawnData out of
ChildProcessLauncher.
Also:
- simplifying ChildProcessAllocator by lazy allocating on access rather
  than explicitly.
- removing forWarmUp from SpawnData has it can be handled as a parameter
  passed around in ChildProcessLauncher.


BUG=702316
Review-Url: https://codereview.chromium.org/2795113003
Cr-Commit-Position: refs/heads/master@{#463916}
Committed: https://chromium.googlesource.com/chromium/src/+/185a6fa4442ecee538c9f7539c3bc7f52bb0ea36

[Jay Civelli, 2017-04-04 23:35:12]

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

[commit-bot: I haz the power, 2017-04-04 23:35:48]

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-04-04 23:39:07]

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

[commit-bot: I haz the power, 2017-04-04 23:39:07]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Jay Civelli, 2017-04-05 16:15:17]

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

[commit-bot: I haz the power, 2017-04-05 16:15:46]

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-04-05 17:03:57]

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

[commit-bot: I haz the power, 2017-04-05 17:03:58]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Jay Civelli, 2017-04-11 00:01:35]

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

[commit-bot: I haz the power, 2017-04-11 00:02:53]

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

[Jay Civelli, 2017-04-11 00:20:54]

Description was changed from

==========
Factor out ChildProcessConnection.

BUG=
==========

to

==========
**********##########**********##########**********##########**********##
Factor out inner-classes out of ChildProcessLauncher.

Factoring out ChildProcessAllocator and SpawnData out of
ChildProcessLauncher.
Also:
- simplifying ChildProcessAllocator by lazy allocating on access rather
  than explicitly.
- removing forWarmUp from SpawnData has it can be handled as a parameter
  passed around in ChildProcessLauncher.


BUG=
==========

[Jay Civelli, 2017-04-11 00:24:51]

Description was changed from

==========
**********##########**********##########**********##########**********##
Factor out inner-classes out of ChildProcessLauncher.

Factoring out ChildProcessAllocator and SpawnData out of
ChildProcessLauncher.
Also:
- simplifying ChildProcessAllocator by lazy allocating on access rather
  than explicitly.
- removing forWarmUp from SpawnData has it can be handled as a parameter
  passed around in ChildProcessLauncher.


BUG=
==========

to

==========
Factor out inner-classes out of ChildProcessLauncher.

Factoring out ChildProcessAllocator and SpawnData out of
ChildProcessLauncher.
Also:
- simplifying ChildProcessAllocator by lazy allocating on access rather
  than explicitly.
- removing forWarmUp from SpawnData has it can be handled as a parameter
  passed around in ChildProcessLauncher.


BUG=702316
==========

[Jay Civelli, 2017-04-11 00:25:20]

jcivelli@chromium.org changed reviewers:
+ boliu@chromium.org

[Jay Civelli, 2017-04-11 16:31:04]

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

[commit-bot: I haz the power, 2017-04-11 16:31:30]

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-04-11 17:56:53]

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

[commit-bot: I haz the power, 2017-04-11 17:56:54]

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

[Jay Civelli, 2017-04-11 21:50:23]

Patchset #1 (id:1) has been deleted

[Jay Civelli, 2017-04-11 21:50:33]

Patchset #1 (id:20001) has been deleted

[Jay Civelli, 2017-04-11 21:50:40]

Patchset #1 (id:40001) has been deleted

[Jay Civelli, 2017-04-11 21:50:50]

Patchset #1 (id:60001) has been deleted

[Jay Civelli, 2017-04-11 21:50:58]

Patchset #1 (id:80001) has been deleted

[Jay Civelli, 2017-04-11 21:51:06]

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

[commit-bot: I haz the power, 2017-04-11 21:51:44]

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-04-11 23:05:36]

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

[commit-bot: I haz the power, 2017-04-11 23:05:37]

Dry run: This issue passed the CQ dry run.

[boliu, 2017-04-12 00:20:16]

lgtm w/ nits

btw, perhaps we should merge crbug.com/702316 and crbug.com/689758 :p

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java
(right):

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java:59:
private static final Object sAllocatorLock = new Object();
nit: move static vars above instance vars, but below static final ones

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java:65:
// As the default value it uses PrivilegedProcessService0.
nit: this comment makes no sense to me..

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java:174:
private static String getClassNameOfService(
nit: same, group static methods together, perhaps even above the instance vars

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
(right):

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:165:
// TODO(jcivelli): it should be safe to pass a null Context here as it is used
to initialize
assert in getAllocator that the cases where a new allocator is created that
context is not null?

[Jay Civelli, 2017-04-12 00:47:15]

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java
(right):

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java:59:
private static final Object sAllocatorLock = new Object();
On 2017/04/12 00:20:16, boliu wrote:
> nit: move static vars above instance vars, but below static final ones

Done.

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java:65:
// As the default value it uses PrivilegedProcessService0.
On 2017/04/12 00:20:15, boliu wrote:
> nit: this comment makes no sense to me..

Me neither, I had just moved it from its original location.
Replaced with something that hopefully makes more sense.

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java:174:
private static String getClassNameOfService(
On 2017/04/12 00:20:15, boliu wrote:
> nit: same, group static methods together, perhaps even above the instance vars

Moved them up.

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
(right):

https://codereview.chromium.org/2795113003/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:165:
// TODO(jcivelli): it should be safe to pass a null Context here as it is used
to initialize
On 2017/04/12 00:20:16, boliu wrote:
> assert in getAllocator that the cases where a new allocator is created that
> context is not null?

We could, but you'll get an NPE with a nice stack-trace anyway.

[Jay Civelli, 2017-04-12 00:47:19]

The CQ bit was checked by jcivelli@chromium.org

[Jay Civelli, 2017-04-12 00:47:19]

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/2795113003/#ps120001
(title: "Addressed boliu@'s comments.")

[commit-bot: I haz the power, 2017-04-12 00:47:54]

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-04-12 03:21:55]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491958039502890,
"parent_rev": "70adfd46aefb86021a640567bdc07d6b6c73a066", "commit_rev":
"185a6fa4442ecee538c9f7539c3bc7f52bb0ea36"}

[commit-bot: I haz the power, 2017-04-12 03:22:51]

Description was changed from

==========
Factor out inner-classes out of ChildProcessLauncher.

Factoring out ChildProcessAllocator and SpawnData out of
ChildProcessLauncher.
Also:
- simplifying ChildProcessAllocator by lazy allocating on access rather
  than explicitly.
- removing forWarmUp from SpawnData has it can be handled as a parameter
  passed around in ChildProcessLauncher.


BUG=702316
==========

to

==========
Factor out inner-classes out of ChildProcessLauncher.

Factoring out ChildProcessAllocator and SpawnData out of
ChildProcessLauncher.
Also:
- simplifying ChildProcessAllocator by lazy allocating on access rather
  than explicitly.
- removing forWarmUp from SpawnData has it can be handled as a parameter
  passed around in ChildProcessLauncher.


BUG=702316

Review-Url: https://codereview.chromium.org/2795113003
Cr-Commit-Position: refs/heads/master@{#463916}
Committed:
https://chromium.googlesource.com/chromium/src/+/185a6fa4442ecee538c9f7539c3b...
==========

[commit-bot: I haz the power, 2017-04-12 03:22:52]

Committed patchset #2 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/185a6fa4442ecee538c9f7539c3b...