Implementing EvictedDomainCookieCounter to keep statistics on cookie eviction and reinstatement.

BUG=225765

Review URL: https://chromiumcodereview.appspot.com/12805010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192866 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/net/evicted_domain_cookie_counter.cc

+// 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& evicted_cookie,
+      const Time& reinstatement_time) OVERRIDE;
+  virtual Time CurrentTime() const OVERRIDE;
+};
+
+DelegateImpl::DelegateImpl() {}
+
+void DelegateImpl::Report(
+    const EvictedDomainCookieCounter::EvictedCookie& evicted_cookie,
+    const Time& reinstatement_time) {
+  TimeDelta reinstatement_delay(
+      reinstatement_time - evicted_cookie.eviction_time);
+  // Need to duplicate HISTOGRAM_CUSTOM_TIMES(), since it is a macro that
+  // defines a static variable.
+  if (evicted_cookie.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() const {
+  return Time::Now();
+}
+
+}  // namespace
+
+EvictedDomainCookieCounter::EvictedDomainCookieCounter(
+    scoped_refptr<net::CookieMonster::Delegate> next_cookie_monster_delegate)
+    : next_cookie_monster_delegate_(next_cookie_monster_delegate),
+      cookie_counter_delegate_(new DelegateImpl),
+      max_size_(kMaxEvictedDomainCookies),
+      purge_count_(kPurgeEvictedDomainCookies) {
+}
+
+EvictedDomainCookieCounter::EvictedDomainCookieCounter(
+    scoped_refptr<net::CookieMonster::Delegate> next_cookie_monster_delegate,
+    scoped_ptr<Delegate> cookie_counter_delegate,
+    size_t max_size,
+    size_t purge_count)
+    : next_cookie_monster_delegate_(next_cookie_monster_delegate),
+      cookie_counter_delegate_(cookie_counter_delegate.Pass()),
+      max_size_(max_size),
+      purge_count_(purge_count) {
+  DCHECK(cookie_counter_delegate_);
+  DCHECK_LT(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& cookie,
+    bool removed,
+    ChangeCause cause) {
+  EvictedDomainCookieCounter::EvictedCookieKey key(GetKey(cookie));
+  Time current_time(cookie_counter_delegate_->CurrentTime());
+  if (removed) {
+    if (cause == net::CookieMonster::Delegate::CHANGE_COOKIE_EVICTED)
+      StoreEvictedCookie(key, cookie, current_time);
+  } else {  // Includes adds or updates.
+    ProcessNewCookie(key, cookie, current_time);
+  }
+
+  if (next_cookie_monster_delegate_)
+    next_cookie_monster_delegate_->OnCookieChanged(cookie, removed, cause);
+}
+
+// static
+EvictedDomainCookieCounter::EvictedCookieKey
+    EvictedDomainCookieCounter::GetKey(const net::CanonicalCookie& cookie) {
+  return cookie.Domain() + ";" + cookie.Path() + ";" + cookie.Name();
+}
+
+// static
+bool EvictedDomainCookieCounter::CompareEvictedCookie(
+    const EvictedCookieMap::iterator evicted_cookie1,
+    const EvictedCookieMap::iterator evicted_cookie2) {
+  return evicted_cookie1->second->eviction_time
+      < evicted_cookie2->second->eviction_time;
+}
+
+void EvictedDomainCookieCounter::GarbageCollect(const Time& current_time) {
+  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_GT(remove_quota, 0u);
+
+  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_time)) {
+      delete it->second;
+      evicted_cookies_.erase(it++); // Post-increment idiom for in-loop removal.
+      if (remove_quota)
+        --remove_quota;
+    } else {
+      if (remove_quota)  // Don't bother storing if quota met.
+        remove_list.push_back(it);
+      ++it;
+    }
+  }
+
+  // Free the oldest |remove_quota| non-expired cookies.
+  std::partial_sort(remove_list.begin(), remove_list.begin() + remove_quota,
+                    remove_list.end(), CompareEvictedCookie);
+  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& cookie,
+    const Time& current_time) {
+  bool is_google = google_util::IsGoogleHostname(
+      cookie.Domain(), google_util::ALLOW_SUBDOMAIN);
+  EvictedCookie* evicted_cookie =
+      new EvictedCookie(current_time, cookie.ExpiryDate(), is_google);
+  std::pair<EvictedCookieMap::iterator, bool> prev_entry =
+      evicted_cookies_.insert(
+          EvictedCookieMap::value_type(key, evicted_cookie));
+  if (!prev_entry.second) {
+    NOTREACHED();
+    delete prev_entry.first->second;
+    prev_entry.first->second = evicted_cookie;
+  }
+
+  GarbageCollect(current_time);
+}
+
+void EvictedDomainCookieCounter::ProcessNewCookie(
+    const EvictedCookieKey& key,
+    const net::CanonicalCookie& cc,
+    const Time& current_time) {
+  EvictedCookieMap::iterator it = evicted_cookies_.find(key);
+  if (it != evicted_cookies_.end()) {
+    if (!it->second->is_expired(current_time))  // Reinstatement.
+      cookie_counter_delegate_->Report(*it->second, current_time);
+    delete it->second;
+    evicted_cookies_.erase(it);
+  }
+}
+
+}  // namespace chrome_browser_net

chrome/browser/net/evicted_domain_cookie_counter.h

+// 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.
+
+#ifndef CHROME_BROWSER_NET_EVICTED_DOMAIN_COOKIE_COUNTER_H_
+#define CHROME_BROWSER_NET_EVICTED_DOMAIN_COOKIE_COUNTER_H_
+
+#include <map>
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/time.h"
+#include "net/cookies/cookie_monster.h"
+
+namespace net {
+class CanonicalCookie;
+}  // namespace net
+
+namespace chrome_browser_net {
+
+// The Evicted Domain Cookie Counter generates statistics on "wrongly evicted"
+// cookies, i.e., cookies that were "evicted" (on reaching domain cookie limit)
+// but are then "reinstated" later because they were important. A specific
+// scenario is as follows: a long-lived login session cookie gets evicted owing
+// to its age, thereby forcing the user to lose session, and is reinstated when
+// the user re-authenticates.
+//
+// A solution to the above problem is the Cookie Priority Field, which enables
+// servers to protect important cookies, thereby decreasing the chances that
+// these cookies are wrongly evicted. To measure the effectiveness of this
+// solution, we will compare eviction user metrics before vs. after the fix.
+//
+// Specifically, we wish to record user metrics on "reinstatement delay", i.e.,
+// the duration between eviction and reinstatement of cookie. We expect that
+// after the fix, average reinstatement delays will increase, since low priority
+// cookies are less likely to be reinstated after eviction.
+//
+// Metrics for Google domains are tracked separately.
+//
+class EvictedDomainCookieCounter : public net::CookieMonster::Delegate {
+ public:
+  // Structure to store sanitized data from CanonicalCookie.
+  struct EvictedCookie {
+    EvictedCookie(base::Time eviction_time_in,
+                  base::Time expiry_time_in,
+                  bool is_google_in)
+        : eviction_time(eviction_time_in),
+          expiry_time(expiry_time_in),
+          is_google(is_google_in) {}
+
+    bool is_expired(const base::Time& current_time) const {
+      return !expiry_time.is_null() && current_time >= expiry_time;
+    }
+
+    base::Time eviction_time;
+    base::Time expiry_time;
+    bool is_google;
+  };
+
+  class Delegate {
+   public:
+    virtual ~Delegate() {}
+
+    // Called when a stored evicted cookie is reinstated.
+    virtual void Report(const EvictedCookie& evicted_cookie,
+                        const base::Time& reinstatement_time) = 0;
+
+    // Getter of time is placed here to enable mocks.
+    virtual base::Time CurrentTime() const = 0;
+  };
+
+  // |next_cookie_monster_delegate| can be NULL.
+  explicit EvictedDomainCookieCounter(
+      scoped_refptr<net::CookieMonster::Delegate> next_cookie_monster_delegate);
+
+  // Constructor exposed for testing only.
+  EvictedDomainCookieCounter(
+      scoped_refptr<net::CookieMonster::Delegate> next_cookie_monster_delegate,
+      scoped_ptr<Delegate> cookie_counter_delegate,
+      size_t max_size,
+      size_t purge_count);
+
+  // Returns the number of evicted cookies stored.
+  size_t GetStorageSize() const;
+
+  // CookieMonster::Delegate implementation.
+  virtual void OnCookieChanged(const net::CanonicalCookie& cookie,
+                               bool removed,
+                               ChangeCause cause) OVERRIDE;
+
+ private:
+  // Identifier of an evicted cookie.
+  typedef std::string EvictedCookieKey;
+
+  // Storage class of evicted cookie.
+  typedef std::map<EvictedCookieKey, EvictedCookie*> EvictedCookieMap;
+
+  virtual ~EvictedDomainCookieCounter();
+
+  // Computes key for |cookie| compatible with CanonicalCookie::IsEquivalent(),
+  // i.e., IsEquivalent(a, b) ==> GetKey(a) == GetKey(b).
+  static EvictedCookieKey GetKey(const net::CanonicalCookie& cookie);
+
+  // Comparator for sorting, to make recently evicted cookies appear earlier.
+  static bool CompareEvictedCookie(
+      const EvictedCookieMap::iterator evicted_cookie1,
+      const EvictedCookieMap::iterator evicted_cookie2);
+
+  // If too many evicted cookies are stored, delete the expired ones, then
+  // delete cookies that were evicted the longest, until size limit reached.
+  void GarbageCollect(const base::Time& current_time);
+
+  // Called when a cookie is evicted. Adds the evicted cookie to storage,
+  // possibly replacing an existing equivalent cookie.
+  void StoreEvictedCookie(const EvictedCookieKey& key,
+                          const net::CanonicalCookie& cookie,
+                          const base::Time& current_time);
+
+  // Called when a new cookie is added. If reinstatement occurs, then notifies
+  // |cookie_counter_delegate_| and then removes the evicted cookie.
+  void ProcessNewCookie(const EvictedCookieKey& key,
+                        const net::CanonicalCookie& cookie,
+                        const base::Time& current_time);
+
+  // Another delegate to forward events to.
+  scoped_refptr<net::CookieMonster::Delegate> next_cookie_monster_delegate_;
+
+  scoped_ptr<Delegate> cookie_counter_delegate_;
+
+  EvictedCookieMap evicted_cookies_;
+
+  // Capacity of the evicted cookie storage, before garbage collection occurs.
+  const size_t max_size_;
+
+  // After garbage collection, size reduces to <= |max_size_| - |purge_count_|.
+  const size_t purge_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(EvictedDomainCookieCounter);
+};
+
+}  // namespace chrome_browser_net
+
+#endif  // CHROME_BROWSER_NET_EVICTED_DOMAIN_COOKIE_COUNTER_H_

chrome/browser/profiles/profile_io_data.cc

@@ -36,6 +36,7 @@
 #include "chrome/browser/net/chrome_http_user_agent_settings.h"
 #include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
+#include "chrome/browser/net/evicted_domain_cookie_counter.h"
 #include "chrome/browser/net/load_time_stats.h"
 #include "chrome/browser/net/proxy_service_factory.h"
 #include "chrome/browser/net/resource_prefetch_predictor_observer.h"
@@ -229,7 +230,8 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
       base::Bind(&GetProfileOnUI, g_browser_process->profile_manager(),
                  profile);
   params->cookie_monster_delegate =
-      new ChromeCookieMonsterDelegate(profile_getter);
+      new chrome_browser_net::EvictedDomainCookieCounter(
+          new ChromeCookieMonsterDelegate(profile_getter));
   params->extension_info_map =
       extensions::ExtensionSystem::Get(profile)->info_map();
 

chrome/chrome_browser.gypi

@@ -1017,6 +1017,8 @@
         'browser/net/dns_probe_job.h',
         'browser/net/dns_probe_service.cc',
         'browser/net/dns_probe_service.h',
+        'browser/net/evicted_domain_cookie_counter.cc',
+        'browser/net/evicted_domain_cookie_counter.h',
         'browser/net/gaia/gaia_oauth_consumer.h',
         'browser/net/gaia/gaia_oauth_fetcher.cc',
         'browser/net/gaia/gaia_oauth_fetcher.h',

chrome/chrome_tests_unit.gypi

@@ -895,6 +895,7 @@
         'browser/net/connection_tester_unittest.cc',
         'browser/net/dns_probe_job_unittest.cc',
         'browser/net/dns_probe_service_unittest.cc',
+        'browser/net/evicted_domain_cookie_counter_unittest.cc',
         'browser/net/gaia/gaia_oauth_fetcher_unittest.cc',
         'browser/net/http_pipelining_compatibility_client_unittest.cc',
         'browser/net/http_server_properties_manager_unittest.cc',

net/cookies/canonical_cookie.h

@@ -82,7 +82,7 @@ class NET_EXPORT CanonicalCookie {
     return !domain_.empty() && domain_[0] == '.'; }
   bool IsHostCookie() const { return !IsDomainCookie(); }
 
-  bool IsExpired(const base::Time& current) {
+  bool IsExpired(const base::Time& current) const {
     return !expiry_date_.is_null() && current >= expiry_date_;
   }
 

net/cookies/cookie_monster.h

@@ -646,7 +646,7 @@ class NET_EXPORT CookieMonster : public CookieStore {
   DISALLOW_COPY_AND_ASSIGN(CookieMonster);
 };
 
-class CookieMonster::Delegate
+class NET_EXPORT CookieMonster::Delegate
     : public base::RefCountedThreadSafe<CookieMonster::Delegate> {
  public:
   // The publicly relevant reasons a cookie might be changed.