customtabs: Create a spare WebContents in warmup() on non low-end devices.

customtabs: Create a spare WebContents in warmup() on non low-end devices.

A pre-created renderer is not specific to a given navigation, which
means that we can create it in warmup(). This is not done on low-end
devices, as the memory cost is more significant in this case.

The behavior is:
- When warmup() is called, create a spare WebContents, unless there is
  already a prerendered one. This WebContents requests a renderer.
- If this renderer dies, then don't revive it.
- A prerender destroys this WebContents, as it cannot currently re-use
  the same one.
- Calling warmup() again when the spare WebContents has been taken will
  re-create a new one.

This is checked by tests.

BUG=548704
Committed: https://crrev.com/c9f5539fb08bf8d5979b2de283f27b936cdc33bb
Cr-Commit-Position: refs/heads/master@{#356894}

[Benoit L, 2015-10-28 16:50:07]

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

[pasko, 2015-10-29 13:38:58]

lgtm with nits

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
(right):

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:134:
private static void oneTimeInitialization(final ChromeApplication app) {
since the one time-ness check is outside this function, let's call it
Initialize().

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java
(right):

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java:151:
private void assertSpareWebContentsNotNullAndDestroy() {
nit: s/Destroy/CanBeDestroyed/

[Yusuf, 2015-10-29 16:35:27]

lgtm with nits as well

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
(right):

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:172:
final boolean firstTime = mWarmupHasBeenCalled.compareAndSet(false, true);
and lets call this !initialized

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:515:
destroySpareWebContents();
since it is a spare one, can we just reuse that? is that any more efficient than
creating a new one?

[Benoit L, 2015-10-29 17:12:36]

Thanks!

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
(right):

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:134:
private static void oneTimeInitialization(final ChromeApplication app) {
On 2015/10/29 13:38:58, pasko wrote:
> since the one time-ness check is outside this function, let's call it
> Initialize().

Done.

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:172:
final boolean firstTime = mWarmupHasBeenCalled.compareAndSet(false, true);
On 2015/10/29 16:35:27, Yusuf wrote:
> and lets call this !initialized

Done.

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:515:
destroySpareWebContents();
On 2015/10/29 16:35:27, Yusuf wrote:
> since it is a spare one, can we just reuse that? is that any more efficient
than
> creating a new one?

Unfortunately, the prerender logic cannot use an existing renderer. I looked at
how to change that, but it does not seem trivial, so destroying the renderer
seems to be the better alternative for now.

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java
(right):

https://codereview.chromium.org/1420913007/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java:151:
private void assertSpareWebContentsNotNullAndDestroy() {
On 2015/10/29 13:38:58, pasko wrote:
> nit: s/Destroy/CanBeDestroyed/

There is no assertion here that we can destroy the WebContents, it is just
destroying it to avoid having too many renderers laying around (which could make
tests fail).

[Benoit L, 2015-10-29 17:34:10]

The CQ bit was checked by lizeb@chromium.org

[Benoit L, 2015-10-29 17:34:10]

The patchset sent to the CQ was uploaded after l-g-t-m from pasko@chromium.org,
yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/1420913007/#ps40001
(title: "Naming.")

[commit-bot: I haz the power, 2015-10-29 17:35:58]

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

[commit-bot: I haz the power, 2015-10-29 18:25:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-29 18:26:54]

Patchset 3 (id:??) landed as
https://crrev.com/c9f5539fb08bf8d5979b2de283f27b936cdc33bb
Cr-Commit-Position: refs/heads/master@{#356894}