android: Post onServiceConnected to launcher thread

android: Post onServiceConnected to launcher thread

This involves refactoring warm up connection. Previously, start used to
wait on launcher thread for the warm up connection to finish
StartCallbacks, which happens in onServiceConnected on the UI thread.
This isn't possible anymore if onServiceConnected is posted to launcher
thread.

Upside is everything is on launcher thread now, so it's a "simple"
matter of keeping more state. Add |sSpareConnectionStartCallback| to
save the StartCallback of an allocation if StartCallback of the warm up
has not finished StartCallback yet.

Note it's not ok to skip the warm up connection when it is not ready,
because that defeats the point warm up and also will allow webview to
launch more than one service.

This change allows removal of sSpareConnectionLock.

BUG=689758
Review-Url: https://codereview.chromium.org/2822803002
Cr-Commit-Position: refs/heads/master@{#465155}
Committed: https://chromium.googlesource.com/chromium/src/+/223e40da117757e1ddc9d4e5eb71d563f50254c9

[boliu, 2017-04-14 22:55:37]

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

[commit-bot: I haz the power, 2017-04-14 22:56:02]

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

[boliu, 2017-04-15 00:55:14]

The CQ bit was unchecked by boliu@chromium.org

[boliu, 2017-04-15 22:14:57]

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

[commit-bot: I haz the power, 2017-04-15 22:15:08]

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-15 22:49:45]

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

[commit-bot: I haz the power, 2017-04-15 22:49:46]

Dry run: This issue passed the CQ dry run.

[boliu, 2017-04-17 16:08:15]

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

[boliu, 2017-04-17 16:08:15]

ptal, this is part 2 of thread unification, and depends on part 1
(https://codereview.chromium.org/2821583002/), so review part 1 first :p

maria for review
jcivelli fyi
rsesek fyi too, this makes using warmup conenction async rather than blocking

[Robert Sesek, 2017-04-17 18:29:16]

rsesek@chromium.org changed reviewers:
+ rsesek@chromium.org

[Robert Sesek, 2017-04-17 18:29:16]

LGTM

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

https://codereview.chromium.org/2822803002/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:280:
clearSpareConnection();
Why is this only done in the condition?

[boliu, 2017-04-17 18:34:18]

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

https://codereview.chromium.org/2822803002/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:280:
clearSpareConnection();
On 2017/04/17 18:29:16, Robert Sesek wrote:
> Why is this only done in the condition?

If there is no chained callback, then it means onChildStarted for the warm up
happened before anything tried to use the warm up. Clearing it in that case
would be like warming up a connection and then immediately throwing it away.

[Robert Sesek, 2017-04-17 18:37:04]

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

https://codereview.chromium.org/2822803002/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:280:
clearSpareConnection();
On 2017/04/17 18:34:18, boliu wrote:
> On 2017/04/17 18:29:16, Robert Sesek wrote:
> > Why is this only done in the condition?
> 
> If there is no chained callback, then it means onChildStarted for the warm up
> happened before anything tried to use the warm up. Clearing it in that case
> would be like warming up a connection and then immediately throwing it away.

Acknowledged. Might be worth leaving a note since that's a little unobvious.

[boliu, 2017-04-17 18:57:13]

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

https://codereview.chromium.org/2822803002/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:280:
clearSpareConnection();
On 2017/04/17 18:37:04, Robert Sesek wrote:
> On 2017/04/17 18:34:18, boliu wrote:
> > On 2017/04/17 18:29:16, Robert Sesek wrote:
> > > Why is this only done in the condition?
> > 
> > If there is no chained callback, then it means onChildStarted for the warm
up
> > happened before anything tried to use the warm up. Clearing it in that case
> > would be like warming up a connection and then immediately throwing it away.
> 
> Acknowledged. Might be worth leaving a note since that's a little unobvious.

Added a comment.

[Maria, 2017-04-18 04:41:42]

lgtm

[boliu, 2017-04-18 04:49:04]

The CQ bit was checked by boliu@chromium.org

[boliu, 2017-04-18 04:49:04]

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

[commit-bot: I haz the power, 2017-04-18 04:49:21]

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-18 06:25:05]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492490944403450,
"parent_rev": "dcf200334b52ac07d81baaec5257e4a964fa3137", "commit_rev":
"223e40da117757e1ddc9d4e5eb71d563f50254c9"}

[commit-bot: I haz the power, 2017-04-18 06:25:14]

Description was changed from

==========
android: Post onServiceConnected to launcher thread

This involves refactoring warm up connection. Previously, start used to
wait on launcher thread for the warm up connection to finish
StartCallbacks, which happens in onServiceConnected on the UI thread.
This isn't possible anymore if onServiceConnected is posted to launcher
thread.

Upside is everything is on launcher thread now, so it's a "simple"
matter of keeping more state. Add |sSpareConnectionStartCallback| to
save the StartCallback of an allocation if StartCallback of the warm up
has not finished StartCallback yet.

Note it's not ok to skip the warm up connection when it is not ready,
because that defeats the point warm up and also will allow webview to
launch more than one service.

This change allows removal of sSpareConnectionLock.

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

to

==========
android: Post onServiceConnected to launcher thread

This involves refactoring warm up connection. Previously, start used to
wait on launcher thread for the warm up connection to finish
StartCallbacks, which happens in onServiceConnected on the UI thread.
This isn't possible anymore if onServiceConnected is posted to launcher
thread.

Upside is everything is on launcher thread now, so it's a "simple"
matter of keeping more state. Add |sSpareConnectionStartCallback| to
save the StartCallback of an allocation if StartCallback of the warm up
has not finished StartCallback yet.

Note it's not ok to skip the warm up connection when it is not ready,
because that defeats the point warm up and also will allow webview to
launch more than one service.

This change allows removal of sSpareConnectionLock.

BUG=689758

Review-Url: https://codereview.chromium.org/2822803002
Cr-Commit-Position: refs/heads/master@{#465155}
Committed:
https://chromium.googlesource.com/chromium/src/+/223e40da117757e1ddc9d4e5eb71...
==========

[commit-bot: I haz the power, 2017-04-18 06:25:15]

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