customtabs: Initialize the spare renderer using the new WebContents method.

customtabs: Initialize the spare renderer using the new WebContents method.

Initializing the spare renderer by navigating to about:blank was a
temporary hack, replace it with the proper (and more recent) method from
WebContents.

BUG=614325
Committed: https://crrev.com/edf077f77fefcb92e447c50578f6c5c4297db4ad
Cr-Commit-Position: refs/heads/master@{#400147}

[Benoit L, 2016-06-07 15:18:27]

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

[Benoit L, 2016-06-07 15:18:27]

One less hack! \o/

[Benoit L, 2016-06-08 09:02:49]

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

[commit-bot: I haz the power, 2016-06-08 09:02:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2043833003/1

[pasko, 2016-06-08 09:24:02]

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:462:
if (mShouldReplaceCurrentEntry) params.setShouldReplaceCurrentEntry(true);
mShouldReplaceCurrentEntry is now always false, right?

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

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:235:
* TODO(lizeb): Replace this with a cleaner method. See crbug.com/521729.
this TODO should go away, and the comment above should be updated

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:240:
mSpareWebContents = WebContentsFactory.createWebContents(false, false, true);
it would be more readable with printing out the names of the parameters:
createWebContents(false /* incognito */,
                  false /* initiallyHidden */,
                  true /* initializeRenderer */);

[commit-bot: I haz the power, 2016-06-08 10:13:23]

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

[commit-bot: I haz the power, 2016-06-08 10:13:24]

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

[Benoit L, 2016-06-08 16:39:06]

Thanks!

All done.

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
(right):

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:462:
if (mShouldReplaceCurrentEntry) params.setShouldReplaceCurrentEntry(true);
On 2016/06/08 09:24:02, pasko wrote:
> mShouldReplaceCurrentEntry is now always false, right?

Done.

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

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:235:
* TODO(lizeb): Replace this with a cleaner method. See crbug.com/521729.
On 2016/06/08 09:24:02, pasko wrote:
> this TODO should go away, and the comment above should be updated

Done.

https://codereview.chromium.org/2043833003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:240:
mSpareWebContents = WebContentsFactory.createWebContents(false, false, true);
On 2016/06/08 09:24:02, pasko wrote:
> it would be more readable with printing out the names of the parameters:
> createWebContents(false /* incognito */,
>                   false /* initiallyHidden */,
>                   true /* initializeRenderer */);

Done.

[pasko, 2016-06-08 19:18:59]

overall looking good, a couple more readability polishing proposals

https://codereview.chromium.org/2043833003/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/WebContentsFactory.java
(right):

https://codereview.chromium.org/2043833003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/WebContentsFactory.java:38:
public static WebContents createWebContents(
overloading just for an extra parameter, where the only extra functionality
comes from a single value of that parameter (i.e. false)?

There would be a more readable variant of it:
public static WebContents createWebContentsWithWarmRenderer(
    boolean incognito, boolean initiallyHidden) {

WDYT?

Pardon for not proposing it initially :/

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

https://codereview.chromium.org/2043833003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:235:
true /* initializeRenderer */);
nit: formatting: this looks better:
        mSpareWebContents = WebContentsFactory.createWebContents(false /*
incognito */,
                                                                 false /*
initiallyHidden*/,
                                                                 true /*
initializeRenderer */);

this also looks better to me:
        mSpareWebContents = WebContentsFactory.createWebContents(
            false /* incognito */, false /* initiallyHidden*/, true /*
initializeRenderer */);

both variants fit 100 chars

[Benoit L, 2016-06-15 11:29:36]

Done, thanks!

[pasko, 2016-06-15 11:56:30]

lgtm thanks

[Benoit L, 2016-06-15 14:33:32]

lizeb@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Benoit L, 2016-06-15 14:33:32]

tedchoc@chromium.org: Please review changes in 
WebContentsFactory.

[Ted C, 2016-06-16 00:09:22]

lgtm

poor sad about:blank...all it wants to do it help!!

[Benoit L, 2016-06-16 07:49:49]

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

[commit-bot: I haz the power, 2016-06-16 07:51:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2043833003/40001

[commit-bot: I haz the power, 2016-06-16 08:50:30]

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

[commit-bot: I haz the power, 2016-06-16 08:50:30]

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

[Benoit L, 2016-06-16 09:23:39]

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

[commit-bot: I haz the power, 2016-06-16 09:23:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2043833003/60001

[commit-bot: I haz the power, 2016-06-16 09:56:09]

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

[commit-bot: I haz the power, 2016-06-16 09:56:10]

Dry run: This issue passed the CQ dry run.

[Benoit L, 2016-06-16 13:57:58]

The CQ bit was checked by lizeb@chromium.org

[Benoit L, 2016-06-16 13:57:58]

The patchset sent to the CQ was uploaded after l-g-t-m from pasko@chromium.org,
tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2043833003/#ps60001
(title: "Proguard fix for a test.")

[commit-bot: I haz the power, 2016-06-16 13:58:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2043833003/60001

[commit-bot: I haz the power, 2016-06-16 14:02:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-16 14:02:10]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-16 14:03:42]

Description was changed from

==========
customtabs: Initialize the spare renderer using the new WebContents method.

Initializing the spare renderer by navigating to about:blank was a
temporary hack, replace it with the proper (and more recent) method from
WebContents.

BUG=614325
==========

to

==========
customtabs: Initialize the spare renderer using the new WebContents method.

Initializing the spare renderer by navigating to about:blank was a
temporary hack, replace it with the proper (and more recent) method from
WebContents.

BUG=614325

Committed: https://crrev.com/edf077f77fefcb92e447c50578f6c5c4297db4ad
Cr-Commit-Position: refs/heads/master@{#400147}
==========

[commit-bot: I haz the power, 2016-06-16 14:03:43]

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