While loading url in current tab should check if any prerender page avaliable.

While loading url in current tab should check if any prerender page avaliable.

This patch with check if any Prerender page is available for
the URL trying to load (while disposition = CURRENT_TAB),
if available then replace with prerender contents else load
the url.

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

[Jitu( very slow this week), 2014-07-28 14:50:48]

PTAL...

Thanks

[Bernhard Bauer, 2014-07-28 15:00:05]

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
File chrome/browser/android/tab_android.cc (right):

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:82: bool SwapWithPrerender(const GURL&
url, chrome::NavigateParams* params) {
MaybeSwapWithPrerender()?

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:87: if (prerenderer &&
prerenderer->UsePrerenderedPage(url, params))
When is |prerenderer| false?

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:92: return prerender_manager &&
When is |prerender_manager| false?

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:208: if (!swapped_with_prerender) {
You can inline this check.

[Bernhard Bauer, 2014-07-28 17:34:16]

Also, can you please file a bug for this, so it can be appropriately triaged?

[Jitu( very slow this week), 2014-07-30 13:48:28]

Fixed the review comments...

PTAL

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
File chrome/browser/android/tab_android.cc (right):

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:82: bool SwapWithPrerender(const GURL&
url, chrome::NavigateParams* params) {
On 2014/07/28 15:00:05, Bernhard Bauer wrote:
> MaybeSwapWithPrerender()?

using only this function MaybeSwapWithPrerender() is fine here.
No need to to use UsePrerenderedPage() as this doing the same thing but just
updating the target_contents.

Which we have taken care in HandlePopupNavigation()

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:87: if (prerenderer &&
prerenderer->UsePrerenderedPage(url, params))
On 2014/07/28 15:00:05, Bernhard Bauer wrote:
> When is |prerenderer| false?

Removed this as not required.

Thanks.

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:92: return prerender_manager &&
On 2014/07/28 15:00:05, Bernhard Bauer wrote:
> When is |prerender_manager| false?

Changed.

Thanks

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:208: if (!swapped_with_prerender) {
On 2014/07/28 15:00:05, Bernhard Bauer wrote:
> You can inline this check.

Done.

Thanks

[Bernhard Bauer, 2014-07-30 14:19:46]

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
File chrome/browser/android/tab_android.cc (right):

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:82: bool SwapWithPrerender(const GURL&
url, chrome::NavigateParams* params) {
On 2014/07/30 13:48:28, Jitu wrote:
> On 2014/07/28 15:00:05, Bernhard Bauer wrote:
> > MaybeSwapWithPrerender()?
> 
> using only this function MaybeSwapWithPrerender() is fine here.
> No need to to use UsePrerenderedPage() as this doing the same thing but just
> updating the target_contents.
> 
> Which we have taken care in HandlePopupNavigation()

What I was suggesting was to name this method MaybeSwapWithPrerender(), as it is
not guaranteed to swap with a prerendered page.

https://codereview.chromium.org/426623004/diff/1/chrome/browser/android/tab_a...
chrome/browser/android/tab_android.cc:92: return prerender_manager &&
On 2014/07/30 13:48:28, Jitu wrote:
> On 2014/07/28 15:00:05, Bernhard Bauer wrote:
> > When is |prerender_manager| false?
> 
> Changed.
> 
> Thanks

If |prerender_manager| can be NULL, the original code is fine, but you didn't
answer my question. Under what circumstances is this null? It would be good to
explain those circumstances in a comment. If it actually can't be NULL, you can
remove the whole check.

[Jitu( very slow this week), 2014-07-31 06:12:53]

prerender_manager can be NULL in below piece of code.

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pre...

And for other comment i have renamed the API as suggested.

[Bernhard Bauer, 2014-07-31 08:11:00]

https://codereview.chromium.org/426623004/diff/40001/chrome/browser/android/t...
File chrome/browser/android/tab_android.cc (right):

https://codereview.chromium.org/426623004/diff/40001/chrome/browser/android/t...
chrome/browser/android/tab_android.cc:90: } else {
Don't put an "else" after a branch that ends with a return statement; Just move
the "else" branch one scope up. And braces are unnecessary for one-line bodies.

[Jitu( very slow this week), 2014-07-31 08:19:47]

Changes done as per suggested.

PTAL...

Thanks

https://codereview.chromium.org/426623004/diff/40001/chrome/browser/android/t...
File chrome/browser/android/tab_android.cc (right):

https://codereview.chromium.org/426623004/diff/40001/chrome/browser/android/t...
chrome/browser/android/tab_android.cc:90: } else {
On 2014/07/31 08:11:00, Bernhard Bauer wrote:
> Don't put an "else" after a branch that ends with a return statement; Just
move
> the "else" branch one scope up. And braces are unnecessary for one-line
bodies.

Done.

[Bernhard Bauer, 2014-07-31 08:22:24]

lgtm

[Jitu( very slow this week), 2014-07-31 08:22:58]

On 2014/07/31 08:22:24, Bernhard Bauer wrote:
> lgtm

Thanks a lot

[Jitu( very slow this week), 2014-07-31 08:23:01]

The CQ bit was checked by jitendra.ks@samsung.com

[commit-bot: I haz the power, 2014-07-31 08:26:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jitendra.ks@samsung.com/426623004/60001

[Jitu( very slow this week), 2014-07-31 08:33:42]

Failing in Presubmit

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/android/tab_android.cc

Plz help me in this.

[Bernhard Bauer, 2014-07-31 08:35:56]

On 2014/07/31 08:33:42, Jitu wrote:
> Failing in Presubmit
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for these files:
>     chrome/browser/android/tab_android.cc
> 
> Plz help me in this.

David, your OWNERS approval is needed.

[commit-bot: I haz the power, 2014-07-31 10:06:26]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-07-31 10:10:36]

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

[commit-bot: I haz the power, 2014-07-31 10:10:37]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Jitu( very slow this week), 2014-08-01 11:37:51]

@David,

This patch needed your permission.

PTAL...

[Jitu( very slow this week), 2014-08-01 17:41:03]

The CQ bit was checked by jitendra.ks@samsung.com

[commit-bot: I haz the power, 2014-08-01 17:46:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jitendra.ks@samsung.com/426623004/60001

[commit-bot: I haz the power, 2014-08-01 18:59:52]

Change committed as 287059