[Offline pages] Reporting correct expiration reason in CleanupTask

[Offline pages] Reporting correct expiration reason in CleanupTask

BUG=705115
Review-Url: https://codereview.chromium.org/2882213003
Cr-Commit-Position: refs/heads/master@{#472269}
Committed: https://chromium.googlesource.com/chromium/src/+/16c45c04cf239a0153842f93c9fa7b5810d0233e

[fgorski, 2017-05-15 22:12:36]

fgorski@chromium.org changed reviewers:
+ petewil@chromium.org

[fgorski, 2017-05-15 22:12:37]

PTAL

[Pete Williamson, 2017-05-15 23:44:19]

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
File components/offline_pages/core/background/cleanup_task.cc (right):

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
components/offline_pages/core/background/cleanup_task.cc:69: // If there are not
expired requests processing is done.
nit not-> not any

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
components/offline_pages/core/background/cleanup_task.cc:88: auto iter =
expired_request_ids_and_reasons_.find(request.request_id());
Could we avoid n**2 behavior here by having a map instead of a vector?

[fgorski, 2017-05-16 17:41:51]

PTAL

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
File components/offline_pages/core/background/cleanup_task.cc (right):

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
components/offline_pages/core/background/cleanup_task.cc:69: // If there are not
expired requests processing is done.
On 2017/05/15 23:44:19, Pete Williamson wrote:
> nit not-> not any

Done.

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
components/offline_pages/core/background/cleanup_task.cc:88: auto iter =
expired_request_ids_and_reasons_.find(request.request_id());
On 2017/05/15 23:44:19, Pete Williamson wrote:
> Could we avoid n**2 behavior here by having a map instead of a vector?

expired_request_ids_and_reasons_ is a map, making the whole thing n*logn, where
n is typically very small. Is there something I am missing here?

Would you prefer an unordered_map?

[Pete Williamson, 2017-05-16 18:13:54]

lgtm

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
File components/offline_pages/core/background/cleanup_task.cc (right):

https://codereview.chromium.org/2882213003/diff/1/components/offline_pages/co...
components/offline_pages/core/background/cleanup_task.cc:88: auto iter =
expired_request_ids_and_reasons_.find(request.request_id());
On 2017/05/16 17:41:51, fgorski wrote:
> On 2017/05/15 23:44:19, Pete Williamson wrote:
> > Could we avoid n**2 behavior here by having a map instead of a vector?
> 
> expired_request_ids_and_reasons_ is a map, making the whole thing n*logn,
where
> n is typically very small. Is there something I am missing here?
> 
> Would you prefer an unordered_map?

I think I was looking at expired_request_ids_, thinking it was
expired_request_ids_and_reasons_.  Code is fine as is.

[fgorski, 2017-05-16 18:20:54]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2017-05-16 18:22:13]

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-05-16 20:59:28]

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

[commit-bot: I haz the power, 2017-05-16 20:59:30]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[fgorski, 2017-05-16 22:04:12]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2017-05-16 22:05:54]

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-05-17 00:45:04]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494972252849880,
"parent_rev": "f43993b0331a3f69a345daa22282ffde07c20e64", "commit_rev":
"16c45c04cf239a0153842f93c9fa7b5810d0233e"}

[commit-bot: I haz the power, 2017-05-17 00:45:14]

Description was changed from

==========
[Offline pages] Reporting correct expiration reason in CleanupTask

BUG=705115
==========

to

==========
[Offline pages] Reporting correct expiration reason in CleanupTask

BUG=705115

Review-Url: https://codereview.chromium.org/2882213003
Cr-Commit-Position: refs/heads/master@{#472269}
Committed:
https://chromium.googlesource.com/chromium/src/+/16c45c04cf239a0153842f93c9fa...
==========

[commit-bot: I haz the power, 2017-05-17 00:45:15]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/16c45c04cf239a0153842f93c9fa...