Split initial load of cookies by domains

chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc

Index: chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc
===================================================================
--- chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc	(revision 103757)
+++ chrome/browser/net/sqlite_persistent_cookie_store_unittest.cc	(working copy)
@@ -16,6 +16,7 @@
 #include "content/browser/browser_thread.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"

[erikwright (departed), 2011/10/06 17:16:47]

platform_test is probably not required anymore?

[guohui, 2011/10/06 18:34:54]

Done.

 
 class SQLitePersistentCookieStoreTest : public testing::Test {
  public:
@@ -23,25 +24,34 @@
       : ui_thread_(BrowserThread::UI),
         db_thread_(BrowserThread::DB),
         io_thread_(BrowserThread::IO),
-        event_(false, false) {
+        event_(false, false),

[erikwright (departed), 2011/10/06 17:16:47]

Remame these to "loaded_event_", "key_loaded_event_", and "db_thread_event_" for
a bit more clarity.

[guohui, 2011/10/06 18:34:54]

Done.

+        event_key_(false, false),
+        event_db_(true, false) {
   }
 
- protected:
   void OnLoaded(
       const std::vector<net::CookieMonster::CanonicalCookie*>& cookies) {
     cookies_ = cookies;
     event_.Signal();
   }
 
-  bool Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
-    bool result =
-        store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
+  void OnKeyLoaded(
+      const std::vector<net::CookieMonster::CanonicalCookie*>& cookies) {
+    cookies_ = cookies;
+    event_key_.Signal();
+  }
+
+  void Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
+    store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
                                 base::Unretained(this)));
     event_.Wait();
     *cookies = cookies_;
-    return result;
   }
 
+  void Wait() {

[erikwright (departed), 2011/10/06 17:16:47]

For clarity of who is waiting on/signaling what, either rename to WaitOnDbEvent
or make it take an Event* and pass base::Unretained(&event_db_) when binding
Wait().

Add a note saying that the method exists because we cannot bind a non-void
returning method as a Closure.

[guohui, 2011/10/06 18:34:54]

Done.

+    event_db_.Wait();
+  }
+
   virtual void SetUp() {
     ui_thread_.Start();
     db_thread_.Start();
@@ -50,7 +60,7 @@
     store_ = new SQLitePersistentCookieStore(
         temp_dir_.path().Append(chrome::kCookieFilename));
     std::vector<net::CookieMonster::CanonicalCookie*> cookies;
-    ASSERT_TRUE(Load(&cookies));
+    Load(&cookies);
     ASSERT_EQ(0u, cookies.size());
     // Make sure the store gets written at least once.
     store_->AddCookie(
@@ -62,10 +72,13 @@
                                             false, false, true));
   }
 
+ protected:
   BrowserThread ui_thread_;
   BrowserThread db_thread_;
   BrowserThread io_thread_;
   base::WaitableEvent event_;
+  base::WaitableEvent event_key_;
+  base::WaitableEvent event_db_;
   std::vector<net::CookieMonster::CanonicalCookie*> cookies_;
   ScopedTempDir temp_dir_;
   scoped_refptr<SQLitePersistentCookieStore> store_;
@@ -118,7 +131,7 @@
       temp_dir_.path().Append(chrome::kCookieFilename));
 
   // Reload and test for persistence
-  ASSERT_TRUE(Load(&cookies));
+  Load(&cookies);
   ASSERT_EQ(1U, cookies.size());
   ASSERT_STREQ("http://foo.bar", cookies[0]->Domain().c_str());
   ASSERT_STREQ("A", cookies[0]->Name().c_str());
@@ -135,10 +148,70 @@
       temp_dir_.path().Append(chrome::kCookieFilename));
 
   // Reload and check if the cookie has been removed.
-  ASSERT_TRUE(Load(&cookies));
+  Load(&cookies);
   ASSERT_EQ(0U, cookies.size());
 }
 
+// Test that priority load of cookies for a specfic domain key could be
+// completed before the entire store is loaded
+TEST_F(SQLitePersistentCookieStoreTest, TestLoadCookiesForKey) {
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+  base::Time t = base::Time::Now() + base::TimeDelta::FromInternalValue(10);
+  // A foo.bar cookie was already added in setup.
+  store_->AddCookie(
+    net::CookieMonster::CanonicalCookie(GURL(), "A", "B",
+                                        "www.aaa.com", "/",
+                                        std::string(), std::string(),
+                                        t, t, t, false, false, true));
+  t += base::TimeDelta::FromInternalValue(10);
+  store_->AddCookie(
+    net::CookieMonster::CanonicalCookie(GURL(), "A", "B",
+                                        "www.bbb.com", "/",
+                                        std::string(), std::string(),
+                                        t, t, t, false, false, true));
+  store_ = NULL;
+
+  scoped_refptr<base::ThreadTestHelper> helper(
+      new base::ThreadTestHelper(
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
+  // Make sure we wait until the destructor has run.
+  ASSERT_TRUE(helper->Run());
+
+  // The store is expected to have three cookies, each with a distinct eTLD+1.
+  store_ = new SQLitePersistentCookieStore(
+    temp_dir_.path().Append(chrome::kCookieFilename));
+  // The event db_thread_ makes sure that the DB thread waits until both Load

[erikwright (departed), 2011/10/06 17:16:47]

"The event db_thread_ ..." => "Posting a blocking task to db_thread_ ..."

[guohui, 2011/10/06 18:34:54]

Done.

+  // and LoadCookiesForKey have been posted to its task queue.
+  db_thread_.PostTask(BrowserThread::DB, FROM_HERE,
+    base::Bind(&SQLitePersistentCookieStoreTest::Wait, base::Unretained(this)));
+  store_->Load(base::Bind(&SQLitePersistentCookieStoreTest::OnLoaded,
+                          base::Unretained(this)));
+  store_->LoadCookiesForKey("aaa.com",
+    base::Bind(&SQLitePersistentCookieStoreTest::OnKeyLoaded,
+               base::Unretained(this)));
+  db_thread_.PostTask(BrowserThread::DB, FROM_HERE,
+    base::Bind(&SQLitePersistentCookieStoreTest::Wait, base::Unretained(this)));
+
+  event_db_.Signal();

[erikwright (departed), 2011/10/06 17:16:47]

In the sample code I provided, there were some comments that illustrated what is
in the queue at each step, and what will occur when we do each Event::Signal().
Can you incorporate them or a corrected/updated version of them please?

[guohui, 2011/10/06 18:34:54]

Done.

+  event_key_.Wait();
+  ASSERT_EQ(event_.IsSignaled(), false);
+  std::set<std::string> cookies_loaded;
+  for (std::vector<net::CookieMonster::CanonicalCookie*>::iterator
+       it = cookies_.begin(); it != cookies_.end(); ++it)
+    cookies_loaded.insert((*it)->Domain().c_str());
+  ASSERT_EQ(cookies_loaded.find("www.aaa.com") != cookies_loaded.end(), true);
+

[erikwright (departed), 2011/10/06 17:16:47]

Can you also assert that cookies_loaded.size() is less than 3? Or store the size
here and than verify that it is bigger after the final load completes?

[guohui, 2011/10/06 18:34:54]

Done.

+  event_db_.Signal();
+  event_.Wait();
+  for (std::vector<net::CookieMonster::CanonicalCookie*>::iterator
+       it = cookies_.begin(); it != cookies_.end(); ++it)
+    cookies_loaded.insert((*it)->Domain().c_str());
+  ASSERT_EQ(3U, cookies_loaded.size());
+  ASSERT_EQ(cookies_loaded.find("http://foo.bar") != cookies_loaded.end(),
+            true);
+  ASSERT_EQ(cookies_loaded.find("www.bbb.com") != cookies_loaded.end(), true);
+}
+
 // Test that we can force the database to be written by calling Flush().
 TEST_F(SQLitePersistentCookieStoreTest, TestFlush) {
   // File timestamps don't work well on all platforms, so we'll determine