Exclude in-use origins from storage evictions for all QuotaEvictionPolicies.

Exclude in-use origins from storage evictions for all QuotaEvictionPolicies.

This CL adds exceptions to the QuotaEvictionPolicy interface which are
used to prevent in-use origins from being evicted and implements handling
of origin exceptions in the SiteEngagementEvictionPolicy.

BUG=464234
Committed: https://crrev.com/8216dbd75d4fe78a7bccdb4b46505f3468f902ce
Cr-Commit-Position: refs/heads/master@{#355236}

[calamity, 2015-09-18 04:30:41]

Patchset #1 (id:1) has been deleted

[calamity, 2015-09-21 05:53:52]

calamity@chromium.org changed reviewers:
+ raymes@chromium.org

[raymes, 2015-09-22 07:16:11]

https://codereview.chromium.org/1354543002/diff/60001/chrome/browser/engageme...
File chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc
(right):

https://codereview.chromium.org/1354543002/diff/60001/chrome/browser/engageme...
chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc:160: GURL
CalculateEvictionOrigin(const std::map<GURL, int64>& usage,
nit: suggest renaming this to CalculatEvictionOriginWithExceptions

https://codereview.chromium.org/1354543002/diff/60001/storage/browser/quota/q...
File storage/browser/quota/quota_manager.cc (right):

https://codereview.chromium.org/1354543002/diff/60001/storage/browser/quota/q...
storage/browser/quota/quota_manager.cc:1479: }
I'm confused a bit by this stuff - but I see you've just copied it from below!

e.g. why do we consider access_notified_origins_ below but not here. Also why
don't we consider origins_in_error_ below? =\

[calamity, 2015-09-23 01:55:22]

https://codereview.chromium.org/1354543002/diff/60001/chrome/browser/engageme...
File chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc
(right):

https://codereview.chromium.org/1354543002/diff/60001/chrome/browser/engageme...
chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc:160: GURL
CalculateEvictionOrigin(const std::map<GURL, int64>& usage,
On 2015/09/22 07:16:10, raymes wrote:
> nit: suggest renaming this to CalculatEvictionOriginWithExceptions

Good call. Done.

https://codereview.chromium.org/1354543002/diff/60001/storage/browser/quota/q...
File storage/browser/quota/quota_manager.cc (right):

https://codereview.chromium.org/1354543002/diff/60001/storage/browser/quota/q...
storage/browser/quota/quota_manager.cc:1479: }
On 2015/09/22 07:16:10, raymes wrote:
> I'm confused a bit by this stuff - but I see you've just copied it from below!
> 
> e.g. why do we consider access_notified_origins_ below but not here. Also why
> don't we consider origins_in_error_ below? =\

Good question. Okay, so, access_notified_origins_ is populated during the
eviction origin task and accounts for origins accessed during the task. ie These
lists represent the exceptions before the task is launched and
access_notified_origins_ are exceptions that arise during the task. (I'm not
sure why origins_in_use_ isn't just checked again below. Maybe it was deemed not
performant?)

I'm not sure why something similar isn't done for the origins in error, or in
fact why origins in error need to excepted from eviction.

[raymes, 2015-09-23 02:27:51]

Your changes lgtm. Perhaps the storage folks will have comments on the above.

[calamity, 2015-09-23 02:38:06]

calamity@chromium.org changed reviewers:
+ michaeln@chromium.org

[calamity, 2015-09-23 02:38:06]

+michaeln for storage/ OWNERS

[calamity, 2015-10-19 06:40:11]

Patchset #6 (id:120001) has been deleted

[calamity, 2015-10-19 06:40:53]

Rebased this on top of the sweeping changes to the QuotaManager patchset. PTAL.

[michaeln, 2015-10-21 00:56:13]

lgtm

https://codereview.chromium.org/1354543002/diff/160001/chrome/browser/engagem...
File chrome/browser/engagement/site_engagement_eviction_policy.cc (right):

https://codereview.chromium.org/1354543002/diff/160001/chrome/browser/engagem...
chrome/browser/engagement/site_engagement_eviction_policy.cc:62: if (overuse >
max_overuse && exceptions.count(origin) == 0) {
you could use !ContainsKey(exceptions, ori) from stl_util.h for clarity

https://codereview.chromium.org/1354543002/diff/160001/storage/browser/quota/...
File storage/browser/quota/quota_manager.cc (right):

https://codereview.chromium.org/1354543002/diff/160001/storage/browser/quota/...
storage/browser/quota/quota_manager.cc:1499:
access_notified_origins_.find(origin) != access_notified_origins_.end()) {
could also use ContainsKey() for these tests

[calamity, 2015-10-21 04:27:10]

https://codereview.chromium.org/1354543002/diff/160001/chrome/browser/engagem...
File chrome/browser/engagement/site_engagement_eviction_policy.cc (right):

https://codereview.chromium.org/1354543002/diff/160001/chrome/browser/engagem...
chrome/browser/engagement/site_engagement_eviction_policy.cc:62: if (overuse >
max_overuse && exceptions.count(origin) == 0) {
On 2015/10/21 00:56:13, michaeln wrote:
> you could use !ContainsKey(exceptions, ori) from stl_util.h for clarity

Done.

https://codereview.chromium.org/1354543002/diff/160001/storage/browser/quota/...
File storage/browser/quota/quota_manager.cc (right):

https://codereview.chromium.org/1354543002/diff/160001/storage/browser/quota/...
storage/browser/quota/quota_manager.cc:1499:
access_notified_origins_.find(origin) != access_notified_origins_.end()) {
On 2015/10/21 00:56:13, michaeln wrote:
> could also use ContainsKey() for these tests

Done.

[calamity, 2015-10-21 04:27:21]

The CQ bit was checked by calamity@chromium.org

[calamity, 2015-10-21 04:27:21]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org,
michaeln@chromium.org
Link to the patchset: https://codereview.chromium.org/1354543002/#ps180001
(title: "address comment")

[commit-bot: I haz the power, 2015-10-21 04:27:28]

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

[commit-bot: I haz the power, 2015-10-21 04:42:54]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2015-10-21 04:43:54]

Patchset 8 (id:??) landed as
https://crrev.com/8216dbd75d4fe78a7bccdb4b46505f3468f902ce
Cr-Commit-Position: refs/heads/master@{#355236}