Introduce a delay before a high priority binding is unbound (reland).

Introduce a delay before a high priority binding is unbound (reland).

This patch delays unbinding high priority connections for child services. This
makes sure that the renderers are not killed immediately after they are
unbound, which is neccessary to mitigate technical issues[1] with the way
renderers are being bound and strengthens the mechanism against possible
misbehavior of the system LRU process management.

[1] For instance, a content view can be detached for a quick moment while the
embedder displays a short-lived animation (e.g. opening a link in a background
tab) - we definitely don't want the underlying renderer to die during that
period.

BUG=259576
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213512

[ppi, 2013-07-19 13:27:12]

Take two - could you guys please have another look?

Compared to the previous patch, I removed setting mAttachAsActiveCount to 0 in
stop(). I see that contrary to some comments in ChildProcessLauncher, we
actually don't reuse ChildProcessConnections (we reuse services, but each time
we create a new ChildProcessConnection), so this was probably redundant anyway.
In this patch I also reword the comments I found misleading.

With this patch we will sometimes call mStrongBinding.unbind() twice but this is
safe - the bindings internally keep track of their state and ignore future calls
to unbind().

Please let me know what you guys think!

[klobag.chromium, 2013-07-19 16:19:54]

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java
(right):

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java:365:
ThreadUtils.postOnUiThreadDelayed(new Runnable() {
why need this?

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java:406:
if (mService == null) {
If you move line 405 to above 410, you can leave stop() unchanged. Otherwise can
you add a comment in stop() to explain why we don't reset the count?

[klobag.chromium, 2013-07-19 18:19:04]

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java
(right):

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java:365:
ThreadUtils.postOnUiThreadDelayed(new Runnable() {
On 2013/07/19 16:19:55, klobag.chromium wrote:
> why need this?

Just saw the comment in the other CL, thanks.

This can be called a lot even it only needs to be called once. Can we avoid spam
the message queue?

[ppi, 2013-07-22 13:50:38]

Thanks, Grace! I have addressed your remarks in patch set 2.

Could you guys have another look?

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java
(right):

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java:365:
ThreadUtils.postOnUiThreadDelayed(new Runnable() {
On 2013/07/19 18:19:05, klobag.chromium wrote:
> On 2013/07/19 16:19:55, klobag.chromium wrote:
> > why need this?
> 
> Just saw the comment in the other CL, thanks.
> 
> This can be called a lot even it only needs to be called once. Can we avoid
spam
> the message queue? 

Done.

https://codereview.chromium.org/19803003/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java:406:
if (mService == null) {
On 2013/07/19 16:19:55, klobag.chromium wrote:
> If you move line 405 to above 410, you can leave stop() unchanged. Otherwise
can
> you add a comment in stop() to explain why we don't reset the count?

Thanks, done (moved).

[klobag.chromium, 2013-07-22 18:19:06]

lgtm

[ppi, 2013-07-23 18:43:56]

Thanks, Grace!

Does this look good to you, Jonathan? I will need an OWNER approval to land
this.

[joth, 2013-07-23 18:56:47]

lgtm rubberstamp

[commit-bot: I haz the power, 2013-07-24 18:22:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ppi@chromium.org/19803003/13001

[commit-bot: I haz the power, 2013-07-24 20:49:59]

Change committed as 213512