Implementing EvictedDomainCookieCounter to keep user metrics on cookie eviction and reinstatement.

chrome/browser/net/evicted_domain_cookie_counter.cc

Index: chrome/browser/net/evicted_domain_cookie_counter.cc
diff --git a/chrome/browser/net/evicted_domain_cookie_counter.cc b/chrome/browser/net/evicted_domain_cookie_counter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e5d794fdd9949be3479bb4f143455326cd1f6356
--- /dev/null
+++ b/chrome/browser/net/evicted_domain_cookie_counter.cc
@@ -0,0 +1,195 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/net/evicted_domain_cookie_counter.h"
+
+#include <algorithm>
+#include <vector>
+
+#include "base/metrics/histogram.h"
+#include "base/stl_util.h"
+#include "base/string_util.h"
+#include "chrome/browser/google/google_util.h"
+#include "net/cookies/canonical_cookie.h"
+
+namespace chrome_browser_net {
+
+using base::Time;
+using base::TimeDelta;
+
+namespace {
+
+const size_t kMaxEvictedDomainCookies = 500;
+const size_t kPurgeEvictedDomainCookies = 100;
+
+class DelegateImpl : public EvictedDomainCookieCounter::Delegate {
+ public:
+  DelegateImpl();
+
+  // EvictedDomainCookieCounter::Delegate implementation.
+  virtual void Report(const EvictedDomainCookieCounter::EvictedCookie& ec,
+                      const Time& reinstatement_time) OVERRIDE;
+  virtual Time CurrentTime() OVERRIDE;
+};
+
+DelegateImpl::DelegateImpl() {}
+
+void DelegateImpl::Report(const EvictedDomainCookieCounter::EvictedCookie& ec,
+                          const Time& reinstatement_time) {
+  TimeDelta reinstatement_delay(reinstatement_time - ec.eviction_time_);
+  // Need to duplicate HISTOGRAM_CUSTOM_TIMES(), since it is a macro that
+  // defines a static variable.
+  if (ec.is_google_) {
+    HISTOGRAM_CUSTOM_TIMES("Cookie.ReinstatedCookiesGoogle",
+                           reinstatement_delay,
+                           TimeDelta::FromSeconds(1),
+                           TimeDelta::FromDays(7),
+                           50);
+  } else {
+    HISTOGRAM_CUSTOM_TIMES("Cookie.ReinstatedCookiesOther",
+                           reinstatement_delay,
+                           TimeDelta::FromSeconds(1),
+                           TimeDelta::FromDays(7),
+                           50);
+  }
+}
+
+Time DelegateImpl::CurrentTime() {
+  return Time::Now();
+}
+
+// Computes a key for |cc| compatible with CanonicalCookie::IsEquivalent(),
+// i.e., IsEquivalent(a, b) ==> GetKey(a) == GetKey(b).
+EvictedDomainCookieCounter::EvictedCookieKey GetKey(

[erikwright (departed), 2013/03/27 01:33:44]

Hmm, OK, I guess private static for this is better than making the typedef
public.

[huangs, 2013/03/28 22:56:21]

Done; moved back to private static, made typedef private.

+    const net::CanonicalCookie& cc) {
+  return cc.Domain() + ";" + cc.Path() + ";" + cc.Name();
+}
+
+}  // namespace
+
+EvictedDomainCookieCounter::EvictedDomainCookieCounter(
+    scoped_refptr<net::CookieMonster::Delegate> next_cookie_monster_delegate)
+    : next_cookie_monster_delegate_(next_cookie_monster_delegate),
+      delegate_(new DelegateImpl),
+      max_size_(kMaxEvictedDomainCookies),
+      purge_count_(kPurgeEvictedDomainCookies) {
+}
+
+EvictedDomainCookieCounter::EvictedDomainCookieCounter(
+    scoped_refptr<net::CookieMonster::Delegate> next_cookie_monster_delegate,
+    scoped_ptr<Delegate> delegate,
+    size_t max_size,
+    size_t purge_count)
+    : next_cookie_monster_delegate_(next_cookie_monster_delegate),
+      delegate_(delegate.Pass()),
+      max_size_(max_size),
+      purge_count_(purge_count) {
+  DCHECK(delegate_);
+  DCHECK(purge_count < max_size_);
+}
+
+EvictedDomainCookieCounter::~EvictedDomainCookieCounter() {
+  STLDeleteContainerPairSecondPointers(evicted_cookies_.begin(),
+                                       evicted_cookies_.end());
+}
+
+size_t EvictedDomainCookieCounter::GetStorageSize() const {
+  return evicted_cookies_.size();
+}
+
+void EvictedDomainCookieCounter::OnCookieChanged(const net::CanonicalCookie& cc,
+                                                 bool removed,
+                                                 ChangeCause cause) {
+  EvictedDomainCookieCounter::EvictedCookieKey key(GetKey(cc));
+  Time current(delegate_->CurrentTime());
+  if (removed) {
+    if (cause == net::CookieMonster::Delegate::CHANGE_COOKIE_EVICTED)
+      StoreEvictedCookie(key, cc, current);
+  } else {  // Includes adds or updates.
+    ProcessNewCookie(key, cc, current);
+  }
+
+  if (next_cookie_monster_delegate_)
+    next_cookie_monster_delegate_->OnCookieChanged(cc, removed, cause);
+}
+
+void EvictedDomainCookieCounter::GarbageCollect(const Time& current) {
+  if (evicted_cookies_.size() <= max_size_)
+    return;
+
+  // From |evicted_cookies_|, removed all expired cookies, and remove cookies
+  // with the oldest |eviction_time_| so that |size_goal| is attained.
+  size_t size_goal = max_size_ - purge_count_;
+  // Bound on number of non-expired cookies to remove.
+  size_t remove_quota = evicted_cookies_.size() - size_goal;
+  DCHECK(remove_quota > 0);
+
+  std::vector<EvictedCookieMap::iterator> remove_list;
+  remove_list.reserve(evicted_cookies_.size());
+
+  EvictedCookieMap::iterator it = evicted_cookies_.begin();
+  while (it != evicted_cookies_.end()) {
+    if (it->second->is_expired(current)) {
+      delete it->second;
+      evicted_cookies_.erase(it++); // Post-increment idiom for in-loop removal.
+      remove_quota -= !!remove_quota;  // Decrease until 0 is reached.

[erikwright (departed), 2013/03/27 01:33:44]

This is exceptionally clever but decreases readability.

if (remove_quota)
  --remove_quota;

Although an extra line, still better.

[huangs, 2013/03/28 22:56:21]

Done.

+    } else {
+      if (remove_quota)  // Don't bother storing if quota met.
+        remove_list.push_back(it);
+      ++it;
+    }
+  }
+
+  // Comparator for sorting, to make recently evicted cookies appear earlier.
+  struct {
+    bool operator() (const EvictedCookieMap::iterator ec1,
+                     const EvictedCookieMap::iterator ec2) {
+      return ec1->second->eviction_time_ < ec2->second->eviction_time_;
+    }
+  } comparator;
+
+  // Free the oldest |remove_quota| non-expired cookies.
+  std::partial_sort(remove_list.begin(), remove_list.begin() + remove_quota,
+                    remove_list.end(), comparator);
+  for (size_t i = 0; i < remove_quota; ++i) {
+    delete remove_list[i]->second;
+    evicted_cookies_.erase(remove_list[i]);
+  }
+
+  // Apply stricter check if non-expired cookies were deleted.
+  DCHECK(remove_quota ? evicted_cookies_.size() == size_goal :
+         evicted_cookies_.size() <= size_goal);
+}
+
+void EvictedDomainCookieCounter::StoreEvictedCookie(
+    const EvictedCookieKey& key,
+    const net::CanonicalCookie& cc,
+    const Time& current) {
+  bool is_google = google_util::IsGoogleHostname(
+      cc.Domain(), google_util::ALLOW_SUBDOMAIN);
+  EvictedCookie* ec = new EvictedCookie(current, cc.ExpiryDate(), is_google);
+  std::pair<EvictedCookieMap::iterator, bool> prev_entry =
+      evicted_cookies_.insert(EvictedCookieMap::value_type(key, ec));
+  if (!prev_entry.second) {
+    NOTREACHED();
+    delete prev_entry.first->second;

[erikwright (departed), 2013/03/27 01:33:44]

and, I suppose:
prev_entry.first->second = ec

[huangs, 2013/03/28 22:56:21]

That was a genuine mistake; I thought that insert would take place!

+  }
+
+  GarbageCollect(current);
+}
+
+void EvictedDomainCookieCounter::ProcessNewCookie(
+    const EvictedCookieKey& key,
+    const net::CanonicalCookie& cc,
+    const Time& current) {
+  EvictedCookieMap::iterator it = evicted_cookies_.find(key);
+  if (it != evicted_cookies_.end()) {
+    if (!it->second->is_expired(current))
+      delegate_->Report(*it->second, current);  // Reinstatement.
+    delete it->second;
+    evicted_cookies_.erase(it);
+  }
+}
+
+}  // namespace chrome_browser_net