[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should roughly match what is happening for full prerenders.

Committed: https://crrev.com/fc6a827302dc5a72dddedaab32fe6e48e6a58dfe
Cr-Commit-Position: refs/heads/master@{#441397}

[droger, 2017-01-03 16:53:34]

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

[commit-bot: I haz the power, 2017-01-03 16:53:51]

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

[droger, 2017-01-03 17:00:46]

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

[droger, 2017-01-03 17:00:56]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-01-03 17:01:05]

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

[droger, 2017-01-03 17:19:41]

Description was changed from

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively
soon. This CL prevent this by detecting recent prefetches and cancelling
them with FINAL_STATUS_DUPLICATE.

The threshold used is config.time_to_live, which should roughly match
what is happening for full prerenders.
==========

to

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively
soon. This CL prevent this by detecting recent prefetches and cancelling
them with FINAL_STATUS_DUPLICATE.

The threshold used is PrerenderCongig::time_to_live (5 minutes), which should
roughly match what is happening for full prerenders.
==========

[droger, 2017-01-03 17:21:07]

Description was changed from

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively
soon. This CL prevent this by detecting recent prefetches and cancelling
them with FINAL_STATUS_DUPLICATE.

The threshold used is PrerenderCongig::time_to_live (5 minutes), which should
roughly match what is happening for full prerenders.
==========

to

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting recent prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is PrerenderConfig::time_to_live (5 minutes), which should
roughly match what is happening for full prerenders.
==========

[commit-bot: I haz the power, 2017-01-03 17:56:17]

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

[commit-bot: I haz the power, 2017-01-03 17:56:17]

Dry run: This issue passed the CQ dry run.

[droger, 2017-01-04 13:48:47]

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

[commit-bot: I haz the power, 2017-01-04 13:48:57]

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

[droger, 2017-01-04 14:09:02]

droger@chromium.org changed reviewers:
+ mattcary@chromium.org, pasko@chromium.org

[droger, 2017-01-04 14:13:50]

Description was changed from

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting recent prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is PrerenderConfig::time_to_live (5 minutes), which should
roughly match what is happening for full prerenders.
==========

to

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is PrerenderConfig::time_to_live (5 minutes), which should
roughly match what is happening for full prerenders.
==========

[droger, 2017-01-04 14:17:40]

It may also make sense to use net::HttpCache::kPrefetchReuseMins instead of
time_to_live. That would mean that we don't repeat prefetches as long as they
are fresh enough to avoid revalidation.

[mattcary, 2017-01-04 14:22:29]

On 2017/01/04 14:17:40, droger wrote:
> It may also make sense to use net::HttpCache::kPrefetchReuseMins instead of
> time_to_live. That would mean that we don't repeat prefetches as long as they
> are fresh enough to avoid revalidation.

That may make more sense. Long-term, there's no reason for this to have anything
to do with prerender.

[mattcary, 2017-01-04 14:22:34]

lgtm

[pasko, 2017-01-04 15:00:46]

overall looks good

https://codereview.chromium.org/2614473002/diff/80001/chrome/browser/prerende...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/2614473002/diff/80001/chrome/browser/prerende...
chrome/browser/prerender/prerender_manager.cc:946: Origin origin;
overriding |origin| in this local scope looks dangerous, can it be
|prefetch_origin|?

https://codereview.chromium.org/2614473002/diff/80001/chrome/browser/prerende...
chrome/browser/prerender/prerender_manager.cc:949:
RecordFinalStatusWithoutCreatingPrerenderContents(url, origin,
this should register the origin from the AddPrerender arg, not the origin from
prefetch information, right?

[droger, 2017-01-04 15:14:18]

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

[commit-bot: I haz the power, 2017-01-04 15:14:35]

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

[droger, 2017-01-04 15:14:48]

Thanks. Fixed the bug and used net::HttpCache::kPrefetchReuseMins.

Please take a look.

https://codereview.chromium.org/2614473002/diff/80001/chrome/browser/prerende...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/2614473002/diff/80001/chrome/browser/prerende...
chrome/browser/prerender/prerender_manager.cc:949:
RecordFinalStatusWithoutCreatingPrerenderContents(url, origin,
On 2017/01/04 15:00:45, pasko wrote:
> this should register the origin from the AddPrerender arg, not the origin from
> prefetch information, right?

This is a bug, good catch.
The local variable should not be used, only the parameter from the function is
important.

[pasko, 2017-01-04 15:51:59]

lgtm, thank you!

[commit-bot: I haz the power, 2017-01-04 15:55:39]

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

[commit-bot: I haz the power, 2017-01-04 15:55:40]

Dry run: This issue passed the CQ dry run.

[droger, 2017-01-04 16:02:18]

Description was changed from

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is PrerenderConfig::time_to_live (5 minutes), which should
roughly match what is happening for full prerenders.
==========

to

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should
roughly match what is happening for full prerenders.
==========

[droger, 2017-01-04 16:02:33]

Description was changed from

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should
roughly match what is happening for full prerenders.
==========

to

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should roughly match what is happening for full prerenders.
==========

[droger, 2017-01-04 16:02:39]

The CQ bit was checked by droger@chromium.org

[droger, 2017-01-04 16:02:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
mattcary@chromium.org
Link to the patchset: https://codereview.chromium.org/2614473002/#ps100001
(title: "fix origin bug")

[commit-bot: I haz the power, 2017-01-04 16:03:01]

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

[droger, 2017-01-04 16:03:37]

The CQ bit was unchecked by droger@chromium.org

[droger, 2017-01-04 16:09:02]

The CQ bit was checked by droger@chromium.org

[droger, 2017-01-04 16:09:03]

The patchset sent to the CQ was uploaded after l-g-t-m from pasko@chromium.org,
mattcary@chromium.org
Link to the patchset: https://codereview.chromium.org/2614473002/#ps120001
(title: "Update test to use kPrefetchReuseMins")

[commit-bot: I haz the power, 2017-01-04 16:09:35]

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-01-04 16:51:03]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1483546142612950,
"parent_rev": "47fea1667a80f54a036b05f410198566d5154aa4", "commit_rev":
"aa154f3d9c1c7596e17051ce63269b80dbed5fec"}

[commit-bot: I haz the power, 2017-01-04 16:51:29]

Description was changed from

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should roughly match what is happening for full prerenders.
==========

to

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should roughly match what is happening for full prerenders.

Review-Url: https://codereview.chromium.org/2614473002
==========

[commit-bot: I haz the power, 2017-01-04 16:51:30]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2017-01-04 16:53:42]

Description was changed from

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should roughly match what is happening for full prerenders.

Review-Url: https://codereview.chromium.org/2614473002
==========

to

==========
[NoStatePrefetch] Cancel repeated prefetches with FINAL_STATUS_DUPLICATE

As opposed to a full prerender, a prefetch could be repeated relatively soon.
This CL prevent this by detecting repeated prefetches and cancelling them with
FINAL_STATUS_DUPLICATE.

The threshold used is net::HttpCache::kPrefetchReuseMins (5 minutes), which
should roughly match what is happening for full prerenders.

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

[commit-bot: I haz the power, 2017-01-04 16:53:43]

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