Fixes targeting the unique creation times invariant in the CookieMonster:

	* Make sure we don't import cookies with identical creation times.
	* DCHECK that duplicate cookie list insertion succeeds (it
	  silently didn't when there were not unique creation times.)
	* Confirm that we eliminate all the duplicats we find on cookie
	  import.
	* Make Methods allowing setting of creation time private.
	* Create performance test for import (involved refactoring
	  backing store mock.)

This change does increase the performance cost on import, and hence adds
to startup time.  However, the increase for importing 15000 cookies was only
5 ms, so I think that's acceptable to prevent crashes.

rdsmith-macbookpro:~/tmp $ perfparse base_perf_import.txt new_perf_import.txt
                                     base_perf_import.txt   new_perf_import.txt
CookieMonsterTest.TestImport
  Cookie_monster_import_from_store   26.36 +/- 0.88         31.38 +/- 1.1

BUG=43188
TEST=net_unittest --gtest_filter=CookieMonsterTest.* (including two new tests.), Linux & Windows

Review URL: http://codereview.chromium.org/3070001

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

5 files changed, 405 insertions, 171 deletions

diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 4c46014..795d3ea8 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -185,13 +185,29 @@ void CookieMonster::InitStore() {
   // This prevents multiple vector growth / copies as we append cookies.
   cookies.reserve(kNumCookiesTotal);
   store_->Load(&cookies);
+
+  // Avoid ever letting cookies with duplicate creation times into the store;
+  // that way we don't have to worry about what sections of code are safe
+  // to call while it's in that state.
+  std::set<int64> creation_times;
   for (std::vector<KeyedCanonicalCookie>::const_iterator it = cookies.begin();
        it != cookies.end(); ++it) {
-    InternalInsertCookie(it->first, it->second, false);
+    int64 cookie_creation_time = it->second->CreationDate().ToInternalValue();
+
+    if (creation_times.insert(cookie_creation_time).second) {
+      InternalInsertCookie(it->first, it->second, false);
+    } else {
+      LOG(ERROR) << StringPrintf("Found cookies with duplicate creation "
+                                 "times in backing store: "
+                                 "{name='%s', domain='%s', path='%s'}",
+                                 it->second->Name().c_str(),
+                                 it->first.c_str(),
+                                 it->second->Path().c_str());
+    }
   }
 
   // After importing cookies from the PersistentCookieStore, verify that
-  // none of our constraints are violated.
+  // none of our other constraints are violated.
   //
   // In particular, the backing store might have given us duplicate cookies.
   EnsureCookiesMapIsValid();
@@ -218,9 +234,6 @@ void CookieMonster::EnsureCookiesMapIsValid() {
   // Record how many duplicates were found in the database.
   // See InitializeHistograms() for details.
   histogram_cookie_deletion_cause_->Add(num_duplicates_trimmed);
-
-  // TODO(eroman): Should also verify that there are no cookies with the same
-  // creation time, since that is assumed to be unique by the rest of the code.
 }
 
 // Our strategy to find duplicates is:
@@ -294,13 +307,18 @@ int CookieMonster::TrimDuplicateCookiesForHost(
 
     // We save the iterator into |cookies_| rather than the actual cookie
     // pointer, since we may need to delete it later.
-    list.insert(it);
+    bool insert_success = list.insert(it).second;
+    DCHECK(insert_success) <<
+        "Duplicate creation times found in duplicate cookie name scan.";
   }
 
   // If there were no duplicates, we are done!
   if (num_duplicates == 0)
     return 0;
 
+  // Make sure we find everything below that we did above.
+  int num_duplicates_found = 0;
+
   // Otherwise, delete all the duplicate cookies, both from our in-memory store
   // and from the backing store.
   for (EquivalenceMap::iterator it = equivalent_cookies.begin();
@@ -311,6 +329,7 @@ int CookieMonster::TrimDuplicateCookiesForHost(
 
     if (dupes.size() <= 1)
       continue;  // This cookiename/path has no duplicates.
+    num_duplicates_found += dupes.size() - 1;
 
     // Since |dups| is sorted by creation time (descending), the first cookie
     // is the most recent one, so we will keep it. The rest are duplicates.
@@ -334,6 +353,7 @@ int CookieMonster::TrimDuplicateCookiesForHost(
                            DELETE_COOKIE_DUPLICATE_IN_BACKING_STORE);
     }
   }
+  DCHECK_EQ(num_duplicates, num_duplicates_found);
 
   return num_duplicates;
 }
@@ -654,13 +674,7 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions(
     const std::string& cookie_line,
     const Time& creation_time_or_null,
     const CookieOptions& options) {
-  AutoLock autolock(lock_);
-
-  if (!HasCookieableScheme(url)) {
-    return false;
-  }
-
-  InitIfNecessary();
+  lock_.AssertAcquired();
 
   COOKIE_DLOG(INFO) << "SetCookie() line: " << cookie_line;
 
@@ -706,6 +720,20 @@ bool CookieMonster::SetCookieWithCreationTimeAndOptions(
   return SetCanonicalCookie(&cc, creation_time, options);
 }
 
+bool CookieMonster::SetCookieWithCreationTime(const GURL& url,
+                                              const std::string& cookie_line,
+                                              const base::Time& creation_time) {
+  AutoLock autolock(lock_);
+
+  if (!HasCookieableScheme(url)) {
+    return false;
+  }
+
+  InitIfNecessary();
+  return SetCookieWithCreationTimeAndOptions(url, cookie_line, creation_time,
+                                             CookieOptions());
+}
+
 bool CookieMonster::SetCookieWithDetails(
     const GURL& url, const std::string& name, const std::string& value,
     const std::string& domain, const std::string& path,
@@ -1030,6 +1058,14 @@ static bool CookieSorter(CookieMonster::CanonicalCookie* cc1,
 bool CookieMonster::SetCookieWithOptions(const GURL& url,
                                          const std::string& cookie_line,
                                          const CookieOptions& options) {
+  AutoLock autolock(lock_);
+
+  if (!HasCookieableScheme(url)) {
+    return false;
+  }
+
+  InitIfNecessary();
+
   return SetCookieWithCreationTimeAndOptions(url, cookie_line, Time(), options);
 }
 
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index 544192f..ab479c4 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -20,6 +20,7 @@
 #include "base/scoped_ptr.h"
 #include "base/time.h"
 #include "net/base/cookie_store.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
 
 class GURL;
 
@@ -107,18 +108,6 @@ class CookieMonster : public CookieStore {
                             const base::Time& expiration_time,
                             bool secure, bool http_only);
 
-  // Exposed for unit testing.
-  bool SetCookieWithCreationTimeAndOptions(const GURL& url,
-                                           const std::string& cookie_line,
-                                           const base::Time& creation_time,
-                                           const CookieOptions& options);
-  bool SetCookieWithCreationTime(const GURL& url,
-                                 const std::string& cookie_line,
-                                 const base::Time& creation_time) {
-    return SetCookieWithCreationTimeAndOptions(url, cookie_line, creation_time,
-                                               CookieOptions());
-  }
-
   // Returns all the cookies, for use in management UI, etc. This does not mark
   // the cookies as having been accessed.
   CookieList GetAllCookies();
@@ -167,6 +156,14 @@ class CookieMonster : public CookieStore {
  private:
   ~CookieMonster();
 
+  // Testing support.
+  friend class CookieMonsterTest;
+  FRIEND_TEST(CookieMonsterTest, TestCookieDeleteAllCreatedAfterTimestamp);
+  FRIEND_TEST(CookieMonsterTest, TestCookieDeleteAllCreatedBetweenTimestamps);
+  bool SetCookieWithCreationTime(const GURL& url,
+                                 const std::string& cookie_line,
+                                 const base::Time& creation_time);
+
   // Called by all non-static functions to ensure that the cookies store has
   // been initialized. This is not done during creating so it doesn't block
   // the window showing.
@@ -220,6 +217,15 @@ class CookieMonster : public CookieStore {
                             CanonicalCookie* cc,
                             bool sync_to_store);
 
+  // Helper function that sets cookies with more control.
+  // Not exposed as we don't want callers to have the ability
+  // to specify (potentially duplicate) creation times.
+  bool SetCookieWithCreationTimeAndOptions(const GURL& url,
+                                           const std::string& cookie_line,
+                                           const base::Time& creation_time,
+                                           const CookieOptions& options);
+
+
   // Helper function that sets a canonical cookie, deleting equivalents and
   // performing garbage collection.
   bool SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
diff --git a/net/base/cookie_monster_perftest.cc b/net/base/cookie_monster_perftest.cc
index 3d1e7a7..042f971 100644
--- a/net/base/cookie_monster_perftest.cc
+++ b/net/base/cookie_monster_perftest.cc
@@ -1,12 +1,13 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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 "net/base/cookie_monster.h"
 #include "base/perftimer.h"
 #include "base/string_util.h"
-#include "net/base/cookie_monster.h"
-#include "testing/gtest/include/gtest/gtest.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/cookie_monster_store_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
   class ParsedCookieTest : public testing::Test { };
@@ -161,3 +162,36 @@ TEST(CookieMonsterTest, TestDomainTree) {
   }
   timer2.Done();
 }
+
+TEST(CookieMonsterTest, TestImport) {
+  scoped_refptr<MockPersistentCookieStore> store(new MockPersistentCookieStore);
+  std::vector<net::CookieMonster::KeyedCanonicalCookie> initial_cookies;
+
+  // We want to setup a fairly large backing store, with 300 domains of 50
+  // cookies each.  Creation times must be unique.
+  int64 time_tick(base::Time::Now().ToInternalValue());
+
+  for (int domain_num = 0; domain_num < 300; domain_num++) {
+    std::string domain_name(StringPrintf(".Domain_%d.com", domain_num));
+    std::string gurl("www" + domain_name);
+    for (int cookie_num = 0; cookie_num < 50; cookie_num++) {
+      std::string cookie_line(StringPrintf("Cookie_%d=1; Path=/", cookie_num));
+      AddKeyedCookieToList(gurl, cookie_line,
+                           base::Time::FromInternalValue(time_tick++),
+                           &initial_cookies);
+    }
+  }
+
+  store->SetLoadExpectation(true, initial_cookies);
+
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store, NULL));
+
+  // Import will happen on first access.
+  GURL gurl("www.google.com");
+  net::CookieOptions options;
+  PerfTimeLogger timer("Cookie_monster_import_from_store");
+  cm->GetCookiesWithOptions(gurl, options);
+  timer.Done();
+}
+
+
diff --git a/net/base/cookie_monster_store_test.h b/net/base/cookie_monster_store_test.h
new file mode 100644
index 0000000..4b1cc52
--- /dev/null
+++ b/net/base/cookie_monster_store_test.h
@@ -0,0 +1,151 @@
+// Copyright (c) 2010 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.
+
+// This file contains test infrastructure for multiple files
+// (current cookie_monster_unittest.cc and cookie_monster_perftest.cc)
+// that need to test out CookieMonster interactions with the backing store.
+// It should only be included by test code.
+
+#include "base/time.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+// Describes a call to one of the 3 functions of PersistentCookieStore.
+struct CookieStoreCommand {
+  enum Type {
+    ADD,
+    UPDATE_ACCESS_TIME,
+    REMOVE,
+  };
+
+  CookieStoreCommand(Type type,
+                     const std::string& key,
+                     const net::CookieMonster::CanonicalCookie& cookie)
+      : type(type), key(key), cookie(cookie) {}
+
+  Type type;
+  std::string key;  // Only applicable to the ADD command.
+  net::CookieMonster::CanonicalCookie cookie;
+};
+
+// Implementation of PersistentCookieStore that captures the
+// received commands and saves them to a list.
+// The result of calls to Load() can be configured using SetLoadExpectation().
+class MockPersistentCookieStore
+    : public net::CookieMonster::PersistentCookieStore {
+ public:
+  typedef std::vector<CookieStoreCommand> CommandList;
+
+  MockPersistentCookieStore() : load_return_value_(true) {
+  }
+
+  virtual bool Load(
+      std::vector<net::CookieMonster::KeyedCanonicalCookie>* out_cookies) {
+    bool ok = load_return_value_;
+    if (ok)
+      *out_cookies = load_result_;
+    return ok;
+  }
+
+  virtual void AddCookie(const std::string& key,
+                         const net::CookieMonster::CanonicalCookie& cookie) {
+    commands_.push_back(
+        CookieStoreCommand(CookieStoreCommand::ADD, key, cookie));
+  }
+
+  virtual void UpdateCookieAccessTime(
+      const net::CookieMonster::CanonicalCookie& cookie) {
+    commands_.push_back(CookieStoreCommand(
+        CookieStoreCommand::UPDATE_ACCESS_TIME, std::string(), cookie));
+  }
+
+  virtual void DeleteCookie(
+      const net::CookieMonster::CanonicalCookie& cookie) {
+    commands_.push_back(
+        CookieStoreCommand(CookieStoreCommand::REMOVE, std::string(), cookie));
+  }
+
+  void SetLoadExpectation(
+      bool return_value,
+      const std::vector<net::CookieMonster::KeyedCanonicalCookie>& result) {
+    load_return_value_ = return_value;
+    load_result_ = result;
+  }
+
+  const CommandList& commands() const {
+    return commands_;
+  }
+
+ private:
+  CommandList commands_;
+
+  // Deferred result to use when Load() is called.
+  bool load_return_value_;
+  std::vector<net::CookieMonster::KeyedCanonicalCookie> load_result_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockPersistentCookieStore);
+};
+
+// Mock for CookieMonster::Delegate
+class MockCookieMonsterDelegate : public net::CookieMonster::Delegate {
+ public:
+  typedef std::pair<net::CookieMonster::CanonicalCookie, bool>
+      CookieNotification;
+
+  MockCookieMonsterDelegate() {}
+
+  virtual void OnCookieChanged(
+      const net::CookieMonster::CanonicalCookie& cookie,
+      bool removed) {
+    CookieNotification notification(cookie, removed);
+    changes_.push_back(notification);
+  }
+
+  const std::vector<CookieNotification>& changes() const { return changes_; }
+
+  void reset() { changes_.clear(); }
+
+ private:
+  virtual ~MockCookieMonsterDelegate() {}
+
+  std::vector<CookieNotification> changes_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockCookieMonsterDelegate);
+};
+
+// Helper to build a list of KeyedCanonicalCookies.
+static void AddKeyedCookieToList(
+    const std::string& key,
+    const std::string& cookie_line,
+    const base::Time& creation_time,
+    std::vector<net::CookieMonster::KeyedCanonicalCookie>* out_list) {
+
+  // Parse the cookie line.
+  net::CookieMonster::ParsedCookie pc(cookie_line);
+  EXPECT_TRUE(pc.IsValid());
+
+  // This helper is simplistic in interpreting a parsed cookie, in order to
+  // avoid duplicated CookieMonster's CanonPath() and CanonExpiration()
+  // functions. Would be nice to export them, and re-use here.
+  EXPECT_FALSE(pc.HasMaxAge());
+  EXPECT_TRUE(pc.HasPath());
+  base::Time cookie_expires = pc.HasExpires() ?
+      net::CookieMonster::ParseCookieTime(pc.Expires()) : base::Time();
+  std::string cookie_path = pc.Path();
+
+  scoped_ptr<net::CookieMonster::CanonicalCookie> cookie(
+      new net::CookieMonster::CanonicalCookie(
+          pc.Name(), pc.Value(), key, cookie_path,
+          pc.IsSecure(), pc.IsHttpOnly(),
+          creation_time, creation_time,
+          !cookie_expires.is_null(),
+          cookie_expires));
+
+  out_list->push_back(
+      net::CookieMonster::KeyedCanonicalCookie(
+          key, cookie.release()));
+}
+
+}  // namespace
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index c61a782..39bfbf5 100644..100755
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -14,8 +14,11 @@
 #include "base/time.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/cookie_monster.h"
+#include "net/base/cookie_monster_store_test.h" // For CookieStore Mock
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace net {
+
 using base::Time;
 using base::TimeDelta;
 
@@ -24,142 +27,6 @@ namespace {
 class ParsedCookieTest : public testing::Test { };
 class CookieMonsterTest : public testing::Test { };
 
-// Describes a call to one of the 3 functions of PersistentCookieStore.
-struct CookieStoreCommand {
-  enum Type {
-    ADD,
-    UPDATE_ACCESS_TIME,
-    REMOVE,
-  };
-
-  CookieStoreCommand(Type type,
-                     const std::string& key,
-                     const net::CookieMonster::CanonicalCookie& cookie)
-      : type(type), key(key), cookie(cookie) {}
-
-  Type type;
-  std::string key;  // Only applicable to the ADD command.
-  net::CookieMonster::CanonicalCookie cookie;
-};
-
-// Implementation of PersistentCookieStore that captures the
-// received commands and saves them to a list.
-// The result of calls to Load() can be configured using SetLoadExpectation().
-class MockPersistentCookieStore
-    : public net::CookieMonster::PersistentCookieStore {
- public:
-  typedef std::vector<CookieStoreCommand> CommandList;
-
-  MockPersistentCookieStore() : load_return_value_(true) {
-  }
-
-  virtual bool Load(
-      std::vector<net::CookieMonster::KeyedCanonicalCookie>* out_cookies) {
-    bool ok = load_return_value_;
-    if (ok)
-      *out_cookies = load_result_;
-    return ok;
-  }
-
-  virtual void AddCookie(const std::string& key,
-                         const net::CookieMonster::CanonicalCookie& cookie) {
-    commands_.push_back(
-        CookieStoreCommand(CookieStoreCommand::ADD, key, cookie));
-  }
-
-  virtual void UpdateCookieAccessTime(
-      const net::CookieMonster::CanonicalCookie& cookie) {
-    commands_.push_back(CookieStoreCommand(
-        CookieStoreCommand::UPDATE_ACCESS_TIME, std::string(), cookie));
-  }
-
-  virtual void DeleteCookie(
-      const net::CookieMonster::CanonicalCookie& cookie) {
-    commands_.push_back(
-        CookieStoreCommand(CookieStoreCommand::REMOVE, std::string(), cookie));
-  }
-
-  void SetLoadExpectation(
-      bool return_value,
-      const std::vector<net::CookieMonster::KeyedCanonicalCookie>& result) {
-    load_return_value_ = return_value;
-    load_result_ = result;
-  }
-
-  const CommandList& commands() const {
-    return commands_;
-  }
-
- private:
-  CommandList commands_;
-
-  // Deferred result to use when Load() is called.
-  bool load_return_value_;
-  std::vector<net::CookieMonster::KeyedCanonicalCookie> load_result_;
-
-  DISALLOW_COPY_AND_ASSIGN(MockPersistentCookieStore);
-};
-
-// Mock for CookieMonster::Delegate
-class MockCookieMonsterDelegate : public net::CookieMonster::Delegate {
- public:
-  typedef std::pair<net::CookieMonster::CanonicalCookie, bool>
-      CookieNotification;
-
-  MockCookieMonsterDelegate() {}
-
-  virtual void OnCookieChanged(
-      const net::CookieMonster::CanonicalCookie& cookie,
-      bool removed) {
-    CookieNotification notification(cookie, removed);
-    changes_.push_back(notification);
-  }
-
-  const std::vector<CookieNotification>& changes() const { return changes_; }
-
-  void reset() { changes_.clear(); }
-
- private:
-  virtual ~MockCookieMonsterDelegate() {}
-
-  std::vector<CookieNotification> changes_;
-
-  DISALLOW_COPY_AND_ASSIGN(MockCookieMonsterDelegate);
-};
-
-// Helper to build a list of KeyedCanonicalCookies.
-void AddKeyedCookieToList(
-    const std::string& key,
-    const std::string& cookie_line,
-    const Time& creation_time,
-    std::vector<net::CookieMonster::KeyedCanonicalCookie>* out_list) {
-
-  // Parse the cookie line.
-  net::CookieMonster::ParsedCookie pc(cookie_line);
-  EXPECT_TRUE(pc.IsValid());
-
-  // This helper is simplistic in interpreting a parsed cookie, in order to
-  // avoid duplicated CookieMonster's CanonPath() and CanonExpiration()
-  // functions. Would be nice to export them, and re-use here.
-  EXPECT_FALSE(pc.HasMaxAge());
-  EXPECT_TRUE(pc.HasPath());
-  Time cookie_expires = pc.HasExpires() ?
-      net::CookieMonster::ParseCookieTime(pc.Expires()) : Time();
-  std::string cookie_path = pc.Path();
-
-  scoped_ptr<net::CookieMonster::CanonicalCookie> cookie(
-      new net::CookieMonster::CanonicalCookie(
-          pc.Name(), pc.Value(), key, cookie_path,
-          pc.IsSecure(), pc.IsHttpOnly(),
-          creation_time, creation_time,
-          !cookie_expires.is_null(),
-          cookie_expires));
-
-  out_list->push_back(
-      net::CookieMonster::KeyedCanonicalCookie(
-          key, cookie.release()));
-}
-
 // Helper for DeleteAllForHost test; repopulates CM with same layout
 // each time.
 const char* kTopLevelDomainPlus1 = "http://www.harvard.edu";
@@ -247,7 +114,6 @@ void PopulateCmForDeleteAllForHost(scoped_refptr<net::CookieMonster> cm) {
 
 }  // namespace
 
-
 TEST(ParsedCookieTest, TestBasic) {
   net::CookieMonster::ParsedCookie pc("a=b");
   EXPECT_TRUE(pc.IsValid());
@@ -1490,7 +1356,10 @@ TEST(CookieMonsterTest, DontImportDuplicateCookies) {
       new MockPersistentCookieStore);
 
   // We will fill some initial cookies into the PersistentCookieStore,
-  // to simulate a database with 4 duplicates.
+  // to simulate a database with 4 duplicates.  Note that we need to
+  // be careful not to have any duplicate creation times at all (as it's a
+  // violation of a CookieMonster invariant) even if Time::Now() doesn't
+  // move between calls.
   std::vector<net::CookieMonster::KeyedCanonicalCookie> initial_cookies;
 
   // Insert 4 cookies with name "X" on path "/", with varying creation
@@ -1529,13 +1398,13 @@ TEST(CookieMonsterTest, DontImportDuplicateCookies) {
 
   AddKeyedCookieToList("www.google.com",
                        "X=a2; path=/2; expires=Mon, 18-Apr-22 22:50:14 GMT",
-                       Time::Now() + TimeDelta::FromDays(1),
+                       Time::Now() + TimeDelta::FromDays(2),
                        &initial_cookies);
 
   // Insert 1 cookie with name "Y" on path "/".
   AddKeyedCookieToList("www.google.com",
                        "Y=a; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT",
-                       Time::Now() + TimeDelta::FromDays(9),
+                       Time::Now() + TimeDelta::FromDays(10),
                        &initial_cookies);
 
   // Inject our initial cookies into the mock PersistentCookieStore.
@@ -1560,6 +1429,74 @@ TEST(CookieMonsterTest, DontImportDuplicateCookies) {
   EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[3].type);
 }
 
+// Tests importing from a persistent cookie store that contains cookies
+// with duplicate creation times.  This situation should be handled by
+// dropping the cookies before insertion/visibility to user.
+//
+// This is a regression test for: http://crbug.com/43188.
+TEST(CookieMonsterTest, DontImportDuplicateCreationTimes) {
+  GURL url_google("http://www.google.com/");
+
+  scoped_refptr<MockPersistentCookieStore> store(
+      new MockPersistentCookieStore);
+
+  Time now(Time::Now());
+  Time earlier(now - TimeDelta::FromDays(1));
+
+  // Insert 8 cookies, four with the current time as creation times, and
+  // four with the earlier time as creation times.  We should only get
+  // two cookies remaining, but which two (other than that there should
+  // be one from each set) will be random.
+  std::vector<net::CookieMonster::KeyedCanonicalCookie> initial_cookies;
+  AddKeyedCookieToList("www.google.com",
+                       "X=1; path=/",
+                       now,
+                       &initial_cookies);
+  AddKeyedCookieToList("www.google.com",
+                       "X=2; path=/",
+                       now,
+                       &initial_cookies);
+  AddKeyedCookieToList("www.google.com",
+                       "X=3; path=/",
+                       now,
+                       &initial_cookies);
+  AddKeyedCookieToList("www.google.com",
+                       "X=4; path=/",
+                       now,
+                       &initial_cookies);
+
+  AddKeyedCookieToList("www.google.com",
+                       "Y=1; path=/",
+                       earlier,
+                       &initial_cookies);
+  AddKeyedCookieToList("www.google.com",
+                       "Y=2; path=/",
+                       earlier,
+                       &initial_cookies);
+  AddKeyedCookieToList("www.google.com",
+                       "Y=3; path=/",
+                       earlier,
+                       &initial_cookies);
+  AddKeyedCookieToList("www.google.com",
+                       "Y=4; path=/",
+                       earlier,
+                       &initial_cookies);
+
+  // Inject our initial cookies into the mock PersistentCookieStore.
+  store->SetLoadExpectation(true, initial_cookies);
+
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(store, NULL));
+
+  net::CookieMonster::CookieList list(cm->GetAllCookies());
+  EXPECT_EQ(2U, list.size());
+  // Confirm that we have one of each.
+  std::string name1(list[0].Name());
+  std::string name2(list[1].Name());
+  EXPECT_TRUE(name1 == "X" || name2 == "X");
+  EXPECT_TRUE(name1 == "Y" || name2 == "Y");
+  EXPECT_NE(name1, name2);
+}
+
 TEST(CookieMonsterTest, Delegate) {
   GURL url_google(kUrlGoogle);
 
@@ -1715,8 +1652,6 @@ TEST(CookieMonsterTest, SetCookieWithDetails) {
   ASSERT_TRUE(++it == cookies.end());
 }
 
-
-
 TEST(CookieMonsterTest, DeleteAllForHost) {
   scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL, NULL));
 
@@ -1779,3 +1714,75 @@ TEST(CookieMonsterTest, DeleteAllForHost) {
                                 std::string("/dir1/dir2/xxx"))));
 
 }
+
+TEST(CookieMonsterTest, UniqueCreationTime) {
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL, NULL));
+  GURL url_google(kUrlGoogle);
+  net::CookieOptions options;
+
+  // Add in three cookies through every public interface to the
+  // CookieMonster and confirm that none of them have duplicate
+  // creation times.
+
+  // SetCookieWithCreationTime and SetCookieWithCreationTimeAndOptions
+  // are not included as they aren't going to be public for very much
+  // longer.
+
+  // SetCookie, SetCookies, SetCookiesWithOptions,
+  // SetCookieWithOptions, SetCookieWithDetails
+
+  cm->SetCookie(url_google, "SetCookie1=A");
+  cm->SetCookie(url_google, "SetCookie2=A");
+  cm->SetCookie(url_google, "SetCookie3=A");
+
+  {
+    std::vector<std::string> cookie_lines;
+    cookie_lines.push_back("setCookies1=A");
+    cookie_lines.push_back("setCookies2=A");
+    cookie_lines.push_back("setCookies4=A");
+    cm->SetCookies(url_google, cookie_lines);
+  }
+
+  {
+    std::vector<std::string> cookie_lines;
+    cookie_lines.push_back("setCookiesWithOptions1=A");
+    cookie_lines.push_back("setCookiesWithOptions2=A");
+    cookie_lines.push_back("setCookiesWithOptions3=A");
+
+    cm->SetCookiesWithOptions(url_google, cookie_lines, options);
+  }
+
+  cm->SetCookieWithOptions(url_google, "setCookieWithOptions1=A", options);
+  cm->SetCookieWithOptions(url_google, "setCookieWithOptions2=A", options);
+  cm->SetCookieWithOptions(url_google, "setCookieWithOptions3=A", options);
+
+  cm->SetCookieWithDetails(url_google, "setCookieWithDetails1", "A",
+                           ".google.com", "/", Time(), false, false);
+  cm->SetCookieWithDetails(url_google, "setCookieWithDetails2", "A",
+                           ".google.com", "/", Time(), false, false);
+  cm->SetCookieWithDetails(url_google, "setCookieWithDetails3", "A",
+                           ".google.com", "/", Time(), false, false);
+
+  // Now we check
+  net::CookieMonster::CookieList cookie_list(cm->GetAllCookies());
+  typedef std::map<int64, net::CookieMonster::CanonicalCookie> TimeCookieMap;
+  TimeCookieMap check_map;
+  for (net::CookieMonster::CookieList::const_iterator it = cookie_list.begin();
+       it != cookie_list.end(); it++) {
+    const int64 creation_date = it->CreationDate().ToInternalValue();
+    TimeCookieMap::const_iterator
+        existing_cookie_it(check_map.find(creation_date));
+    EXPECT_TRUE(existing_cookie_it == check_map.end())
+        << "Cookie " << it->Name() << " has same creation date ("
+        << it->CreationDate().ToInternalValue()
+        << ") as previously entered cookie "
+        << existing_cookie_it->second.Name();
+
+    if (existing_cookie_it == check_map.end()) {
+      check_map.insert(TimeCookieMap::value_type(
+          it->CreationDate().ToInternalValue(), *it));
+    }
+  }
+}
+
+} // namespace