android: Move PendingSpawnQueue into ChildConnectionAllocator

android: Move PendingSpawnQueue into ChildConnectionAllocator

This simplifies locking between allocator and queue:
{allocate or enqueue} and {free and dequeue} need to be atomic
operations, and this was previously achieved by exposing the lock in
PendingSpawnQueue outside of its class. See
https://codereview.chromium.org/1725643003

By moving PendingSpawnQueue, can get rid of the lock in Queue
entirely and just use the Allocator lock instead.

Everything else is more or less fallout from this change:
* Pass entire SpawnData to allocate call so it may be enqueued
* Add isForWarmup to SpawnData; warmup allocations are never queued
* Rename PendingSpawnData to SpawnData since it's no longer just for
  pending allocations
* Add comments
* clang-format made some questionable format choices

BUG=689758
Review-Url: https://codereview.chromium.org/2689673007
Cr-Commit-Position: refs/heads/master@{#450201}
Committed: https://chromium.googlesource.com/chromium/src/+/22b31729529579fea43962f4bd0d20a760d027f5

[boliu, 2017-02-12 01:20:43]

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

[commit-bot: I haz the power, 2017-02-12 01:20: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-02-12 02:03:02]

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

[commit-bot: I haz the power, 2017-02-12 02:03:03]

Dry run: This issue passed the CQ dry run.

[boliu, 2017-02-13 18:18:30]

Description was changed from

==========
[NOT FOR COMMIT]

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

to

==========
android: Move PendingSpawnQueue into ChildConnectionAllocator

This simplifies locking between allocator and queue:
{allocate or enqueue} and {free and dequeue} need to be atomic
operations, and this was previously achieved by exposing the lock in
PendingSpawnQueue outside of its class. See
https://codereview.chromium.org/1725643003

By moving PendingSpawnQueue, can get rid of the lock in Queue entirely
and just use the Allocator lock instead.

Everything else is more or less fallout from this change:
* Pass entire SpawnData to allocate call so it may be enqueued
* Add isForWarmup to SpawnData; warmup allocations are never queued
* Rename PendingSpawnData to SpawnData since it's no longer just for
  pending allocations
* Add comments
* clang-format made some questionable format choices

BUG=689758
==========

[boliu, 2017-02-13 18:18:41]

Description was changed from

==========
android: Move PendingSpawnQueue into ChildConnectionAllocator

This simplifies locking between allocator and queue:
{allocate or enqueue} and {free and dequeue} need to be atomic
operations, and this was previously achieved by exposing the lock in
PendingSpawnQueue outside of its class. See
https://codereview.chromium.org/1725643003

By moving PendingSpawnQueue, can get rid of the lock in Queue entirely
and just use the Allocator lock instead.

Everything else is more or less fallout from this change:
* Pass entire SpawnData to allocate call so it may be enqueued
* Add isForWarmup to SpawnData; warmup allocations are never queued
* Rename PendingSpawnData to SpawnData since it's no longer just for
  pending allocations
* Add comments
* clang-format made some questionable format choices

BUG=689758
==========

to

==========
android: Move PendingSpawnQueue into ChildConnectionAllocator

This simplifies locking between allocator and queue:
{allocate or enqueue} and {free and dequeue} need to be atomic
operations, and this was previously achieved by exposing the lock in
PendingSpawnQueue outside of its class. See
https://codereview.chromium.org/1725643003

By moving PendingSpawnQueue, can get rid of the lock in Queue
entirely and just use the Allocator lock instead.

Everything else is more or less fallout from this change:
* Pass entire SpawnData to allocate call so it may be enqueued
* Add isForWarmup to SpawnData; warmup allocations are never queued
* Rename PendingSpawnData to SpawnData since it's no longer just for
  pending allocations
* Add comments
* clang-format made some questionable format choices

BUG=689758
==========

[boliu, 2017-02-13 18:21:00]

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

[boliu, 2017-02-13 18:21:01]

ptal

[Maria, 2017-02-14 00:59:41]

lgtm

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

https://codereview.chromium.org/2689673007/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:72:
private final Queue<SpawnData> mPendingSpawnQueue = new LinkedList<>();
Should this comment also mention that this uses mConnectionLock to synchronize?

https://codereview.chromium.org/2689673007/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:603:
CALLBACK_FOR_RENDERER_PROCESS, true /* inSandbox */, params);
these new calls with a bunch of comments make me think we should consider
builder pattern. Not for this CL, but maybe later?

[boliu, 2017-02-14 01:34:20]

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

https://codereview.chromium.org/2689673007/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:72:
private final Queue<SpawnData> mPendingSpawnQueue = new LinkedList<>();
On 2017/02/14 00:59:41, Maria wrote:
> Should this comment also mention that this uses mConnectionLock to
synchronize?

Done.

https://codereview.chromium.org/2689673007/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:603:
CALLBACK_FOR_RENDERER_PROCESS, true /* inSandbox */, params);
On 2017/02/14 00:59:41, Maria wrote:
> these new calls with a bunch of comments make me think we should consider
> builder pattern. Not for this CL, but maybe later?

Hmm.. If we want to force caller to make a decision instead of taking default,
then builder doesn't help. Don't really know if that's important or not though.

Anyway, aesthetics is pretty low on my list of things to fix :p

[boliu, 2017-02-14 01:34:54]

The CQ bit was checked by boliu@chromium.org

[boliu, 2017-02-14 01:34:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
mariakhomenko@chromium.org
Link to the patchset: https://codereview.chromium.org/2689673007/#ps80001
(title: "review")

[commit-bot: I haz the power, 2017-02-14 01:36:00]

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-02-14 02:18:19]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487036094080040,
"parent_rev": "bc76f8f1aa38027b2eb0f2a5c7616d9ad9ae36ca", "commit_rev":
"22b31729529579fea43962f4bd0d20a760d027f5"}

[commit-bot: I haz the power, 2017-02-14 02:18:52]

Description was changed from

==========
android: Move PendingSpawnQueue into ChildConnectionAllocator

This simplifies locking between allocator and queue:
{allocate or enqueue} and {free and dequeue} need to be atomic
operations, and this was previously achieved by exposing the lock in
PendingSpawnQueue outside of its class. See
https://codereview.chromium.org/1725643003

By moving PendingSpawnQueue, can get rid of the lock in Queue
entirely and just use the Allocator lock instead.

Everything else is more or less fallout from this change:
* Pass entire SpawnData to allocate call so it may be enqueued
* Add isForWarmup to SpawnData; warmup allocations are never queued
* Rename PendingSpawnData to SpawnData since it's no longer just for
  pending allocations
* Add comments
* clang-format made some questionable format choices

BUG=689758
==========

to

==========
android: Move PendingSpawnQueue into ChildConnectionAllocator

This simplifies locking between allocator and queue:
{allocate or enqueue} and {free and dequeue} need to be atomic
operations, and this was previously achieved by exposing the lock in
PendingSpawnQueue outside of its class. See
https://codereview.chromium.org/1725643003

By moving PendingSpawnQueue, can get rid of the lock in Queue
entirely and just use the Allocator lock instead.

Everything else is more or less fallout from this change:
* Pass entire SpawnData to allocate call so it may be enqueued
* Add isForWarmup to SpawnData; warmup allocations are never queued
* Rename PendingSpawnData to SpawnData since it's no longer just for
  pending allocations
* Add comments
* clang-format made some questionable format choices

BUG=689758

Review-Url: https://codereview.chromium.org/2689673007
Cr-Commit-Position: refs/heads/master@{#450201}
Committed:
https://chromium.googlesource.com/chromium/src/+/22b31729529579fea43962f4bd0d...
==========

[commit-bot: I haz the power, 2017-02-14 02:18:53]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/22b31729529579fea43962f4bd0d...