Finalize a CL originally by departed intern ycxiao@ that detaches the loading of cookies from the...

net/base/cookie_monster_perftest.cc

Index: net/base/cookie_monster_perftest.cc
===================================================================
--- net/base/cookie_monster_perftest.cc	(revision 99705)
+++ net/base/cookie_monster_perftest.cc	(working copy)
@@ -4,8 +4,8 @@
 
 #include <algorithm>
 
-#include "net/base/cookie_monster.h"
-
+#include "base/bind.h"
+#include "base/message_loop.h"
 #include "base/perftimer.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
@@ -15,8 +15,13 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
-  class ParsedCookieTest : public testing::Test { };

[Randy Smith (Not in Mondays), 2011/09/07 19:04:07]

Why is it ok to get rid of this?  (I'm assuming it is--question is for my own
education.)

[erikwright (departed), 2011/09/08 02:48:58]

No one actually used it. The TEST definitions do not require a pre-defined
class. TEST_F does.

-  class CookieMonsterTest : public testing::Test { };
+class CookieMonsterTest : public testing::Test {
+ public:
+  CookieMonsterTest() : message_loop_(new MessageLoopForIO()) {}
+
+ private:
+  scoped_ptr<MessageLoop> message_loop_;
+};
 }
 
 static const int kNumCookies = 20000;
@@ -47,102 +52,165 @@
 
 static const GURL kUrlGoogle("http://www.google.izzle");
 
-TEST(CookieMonsterTest, TestAddCookiesOnSingleHost) {
+class BaseCallback {
+ public:
+  BaseCallback() : run_(false) {}
+  bool run() { return run_; }

[Randy Smith (Not in Mondays), 2011/09/07 19:04:07]

nit: const?

[erikwright (departed), 2011/09/08 02:48:58]

Done.

+  void reset() { run_ = false; }

[Randy Smith (Not in Mondays), 2011/09/07 19:04:07]

nit: The semantic difference between BaseCallback::run() and <Child of
BaseCallback>::Run() is great enough (state probe vs. take action) that I'd be
inclined to make the names more different.  Maybe has_run_, has_run() and
reset_has_run()?

[erikwright (departed), 2011/09/08 02:48:58]

Done.

+ protected:
+  bool run_;
+};
+
+
+class SetCookieCallback  : public BaseCallback {
+ public:
+  void SetCookie(
+      CookieMonster* cm, const GURL& gurl, const std::string& cookie) {
+    cm->SetCookieWithOptionsAsync(gurl, cookie, options_, base::Bind(
+        &SetCookieCallback::Run, base::Unretained(this)));
+    MessageLoop::current()->RunAllPending();
+    EXPECT_TRUE(run());
+    reset();
+  }
+ private:
+  void Run(bool success) {
+    EXPECT_TRUE(success);
+    run_ = true;
+  }
+  net::CookieOptions options_;
+};
+
+class GetCookiesCallback : public BaseCallback {
+ public:
+  const std::string& GetCookies(CookieMonster* cm, const GURL& gurl) {
+    cm->GetCookiesWithOptionsAsync(gurl, options_, base::Bind(
+        &GetCookiesCallback::Run, base::Unretained(this)));
+    MessageLoop::current()->RunAllPending();
+    EXPECT_TRUE(run());
+    reset();
+    return cookies_;
+  }
+
+ private:
+  void Run(const std::string& cookies) {
+    run_ = true;
+    cookies_ = cookies;
+  }
+  std::string cookies_;
+  net::CookieOptions options_;
+};
+
+class GetCookiesWithInfoCallback : public BaseCallback {
+ public:
+  const std::string& GetCookiesWithInfo(CookieMonster* cm, const GURL& gurl) {
+    cm->GetCookiesWithInfoAsync(gurl, options_, base::Bind(
+        &GetCookiesWithInfoCallback::Run,
+        base::Unretained(this)));
+    MessageLoop::current()->RunAllPending();

[Randy Smith (Not in Mondays), 2011/09/07 19:04:07]

Does this guarantee that we've received the callback?  I guess if everything's
on the same message loop, it does, because by the time the first task has been
run the return one will have been posted.  Still, that reasoning probably
deserves a comment.

[erikwright (departed), 2011/09/08 02:48:58]

Done.

+    EXPECT_TRUE(run());
+    reset();
+    return cookies_;
+  }
+
+ private:
+  void Run(
+    const std::string& cookie_line,
+    const std::vector<CookieStore::CookieInfo>& cookie_infos) {
+    cookies_ = cookie_line;
+    run_ = true;
+  }
+
+  std::string cookies_;
+  net::CookieOptions options_;
+};
+
+
+TEST_F(CookieMonsterTest, TestAddCookiesOnSingleHost) {
   scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
   std::vector<std::string> cookies;
   for (int i = 0; i < kNumCookies; i++) {
     cookies.push_back(base::StringPrintf("a%03d=b", i));
   }
 
+  SetCookieCallback setCookieCallback;
+
   // Add a bunch of cookies on a single host
   PerfTimeLogger timer("Cookie_monster_add_single_host");
+
   for (std::vector<std::string>::const_iterator it = cookies.begin();
        it != cookies.end(); ++it) {
-    EXPECT_TRUE(cm->SetCookie(kUrlGoogle, *it));
+    setCookieCallback.SetCookie(cm, kUrlGoogle, *it);
   }
   timer.Done();
 
+  GetCookiesCallback getCookiesCallback;
+
   PerfTimeLogger timer2("Cookie_monster_query_single_host");
   for (std::vector<std::string>::const_iterator it = cookies.begin();
        it != cookies.end(); ++it) {
-    cm->GetCookies(kUrlGoogle);
+    getCookiesCallback.GetCookies(cm, kUrlGoogle);
   }
   timer2.Done();
 
   PerfTimeLogger timer3("Cookie_monster_deleteall_single_host");
-  cm->DeleteAll(false);
+  cm->DeleteAllAsync(CookieMonster::DeleteCallback());
+  MessageLoop::current()->RunAllPending();
   timer3.Done();
 }
 
-TEST(CookieMonsterTest, TestAddCookieOnManyHosts) {
+TEST_F(CookieMonsterTest, TestAddCookieOnManyHosts) {
   scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
   std::string cookie(kCookieLine);
   std::vector<GURL> gurls;  // just wanna have ffffuunnn
   for (int i = 0; i < kNumCookies; ++i) {
-    gurls.push_back(GURL(base::StringPrintf("http://a%04d.izzle", i)));
+    gurls.push_back(GURL(base::StringPrintf("https://a%04d.izzle", i)));
   }
 
+  SetCookieCallback setCookieCallback;
+
   // Add a cookie on a bunch of host
   PerfTimeLogger timer("Cookie_monster_add_many_hosts");
   for (std::vector<GURL>::const_iterator it = gurls.begin();
        it != gurls.end(); ++it) {
-    EXPECT_TRUE(cm->SetCookie(*it, cookie));
+    setCookieCallback.SetCookie(cm, *it, cookie);
   }
   timer.Done();
 
+  GetCookiesCallback getCookiesCallback;
+
   PerfTimeLogger timer2("Cookie_monster_query_many_hosts");
   for (std::vector<GURL>::const_iterator it = gurls.begin();
        it != gurls.end(); ++it) {
-    cm->GetCookies(*it);
+    getCookiesCallback.GetCookies(cm, *it);
   }
   timer2.Done();
 
   PerfTimeLogger timer3("Cookie_monster_deleteall_many_hosts");
-  cm->DeleteAll(false);
+  cm->DeleteAllAsync(CookieMonster::DeleteCallback());
+  MessageLoop::current()->RunAllPending();
   timer3.Done();
 }
 
-TEST(CookieMonsterTest, TestGetCookiesWithOptions) {
+TEST_F(CookieMonsterTest, TestGetCookiesWithInfo) {
   scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
-  std::string cookie(kCookieLine);
+
   std::vector<GURL> gurls;
   for (int i = 0; i < kNumCookies; ++i)
-    gurls.push_back(GURL(base::StringPrintf("http://a%04d.izzle", i)));
+    gurls.push_back(GURL(base::StringPrintf("https://a%04d.izzle", i)));
 
-  for (std::vector<GURL>::const_iterator it = gurls.begin();
-       it != gurls.end(); ++it) {
-    EXPECT_TRUE(cm->SetCookie(*it, cookie));
-  }
+  SetCookieCallback setCookieCallback;
 
-  PerfTimeLogger timer("Cookie_monster_get_cookie_info");
   for (std::vector<GURL>::const_iterator it = gurls.begin();
        it != gurls.end(); ++it) {
-    CookieOptions options;
-    std::string cookie_line;
-    cookie_line = cm->GetCookiesWithOptions(*it, options);
+    setCookieCallback.SetCookie(cm, *it, kCookieLine);
   }
-  timer.Done();
-}
 
-TEST(CookieMonsterTest, TestGetCookiesWithInfo) {
-  scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
-  std::string cookie(kCookieLine);
-  std::vector<GURL> gurls;
-  for (int i = 0; i < kNumCookies; ++i)
-    gurls.push_back(GURL(base::StringPrintf("http://a%04d.izzle", i)));
+  GetCookiesWithInfoCallback getCookiesCallback;
 
-  for (std::vector<GURL>::const_iterator it = gurls.begin();
-       it != gurls.end(); ++it) {
-    EXPECT_TRUE(cm->SetCookie(*it, cookie));
-  }
-
   PerfTimeLogger timer("Cookie_monster_get_cookie_info");
   for (std::vector<GURL>::const_iterator it = gurls.begin();
        it != gurls.end(); ++it) {
-    CookieOptions options;
-    std::string cookie_line;
-    std::vector<CookieStore::CookieInfo> cookie_infos;
-    cm->GetCookiesWithInfo(*it, options, &cookie_line, &cookie_infos);
+    getCookiesCallback.GetCookiesWithInfo(cm, *it);
   }
   timer.Done();
 }
@@ -151,8 +219,10 @@
   return std::count(str.begin(), str.end(), c);
 }
 
-TEST(CookieMonsterTest, TestDomainTree) {
+TEST_F(CookieMonsterTest, TestDomainTree) {
   scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
+  GetCookiesCallback getCookiesCallback;
+  SetCookieCallback setCookieCallback;
   const char* domain_cookie_format_tree = "a=b; domain=%s";
   const std::string domain_base("top.com");
 
@@ -188,25 +258,25 @@
     GURL gurl("https://" + *it + "/");
     const std::string cookie = base::StringPrintf(domain_cookie_format_tree,
                                                   it->c_str());
-    EXPECT_TRUE(cm->SetCookie(gurl, cookie));
+    setCookieCallback.SetCookie(cm, gurl, cookie);
   }
   EXPECT_EQ(31u, cm->GetAllCookies().size());
 
   GURL probe_gurl("https://b.a.b.a.top.com/");
-  std::string cookie_line;
-  cookie_line = cm->GetCookies(probe_gurl);
-  EXPECT_EQ(5, CountInString(cookie_line, '=')) << "Cookie line: "
-                                                << cookie_line;
+  std::string cookie_line = getCookiesCallback.GetCookies(cm, probe_gurl);
+  EXPECT_EQ(5, CountInString(cookie_line, '=')) << "Cookie line: " <<
+      cookie_line;
   PerfTimeLogger timer("Cookie_monster_query_domain_tree");
   for (int i = 0; i < kNumCookies; i++) {
-    cm->GetCookies(probe_gurl);
+    getCookiesCallback.GetCookies(cm, probe_gurl);
   }
   timer.Done();
-
 }
 
-TEST(CookieMonsterTest, TestDomainLine) {
+TEST_F(CookieMonsterTest, TestDomainLine) {
   scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
+  SetCookieCallback setCookieCallback;
+  GetCookiesCallback getCookiesCallback;
   std::vector<std::string> domain_list;
   GURL probe_gurl("https://b.a.b.a.top.com/");
   std::string cookie_line;
@@ -230,23 +300,23 @@
       GURL gurl("https://" + *it + "/");
       const std::string cookie = base::StringPrintf(domain_cookie_format_line,
                                                     i, it->c_str());
-      EXPECT_TRUE(cm->SetCookie(gurl, cookie));
+      setCookieCallback.SetCookie(cm, gurl, cookie);
     }
   }
-  EXPECT_EQ(32u, cm->GetAllCookies().size());
 
-  cookie_line = cm->GetCookies(probe_gurl);
+  cookie_line = getCookiesCallback.GetCookies(cm, probe_gurl);
   EXPECT_EQ(32, CountInString(cookie_line, '='));
   PerfTimeLogger timer2("Cookie_monster_query_domain_line");
   for (int i = 0; i < kNumCookies; i++) {
-    cm->GetCookies(probe_gurl);
+    getCookiesCallback.GetCookies(cm, probe_gurl);
   }
   timer2.Done();
 }
 
-TEST(CookieMonsterTest, TestImport) {
+TEST_F(CookieMonsterTest, TestImport) {
   scoped_refptr<MockPersistentCookieStore> store(new MockPersistentCookieStore);
   std::vector<CookieMonster::CanonicalCookie*> initial_cookies;
+  GetCookiesCallback getCookiesCallback;
 
   // We want to setup a fairly large backing store, with 300 domains of 50
   // cookies each.  Creation times must be unique.
@@ -272,14 +342,14 @@
   GURL gurl("www.google.com");
   CookieOptions options;
   PerfTimeLogger timer("Cookie_monster_import_from_store");
-  cm->GetCookiesWithOptions(gurl, options);
+  getCookiesCallback.GetCookies(cm, gurl);
   timer.Done();
 
   // Just confirm keys were set as expected.
   EXPECT_EQ("domain_1.com", cm->GetKey("www.Domain_1.com"));
 }
 
-TEST(CookieMonsterTest, TestGetKey) {
+TEST_F(CookieMonsterTest, TestGetKey) {
   scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
   PerfTimeLogger timer("Cookie_monster_get_key");
   for (int i = 0; i < kNumCookies; i++)
@@ -294,7 +364,9 @@
 // a performance test.  The test should be considered to pass if all the
 // times reported are approximately the same--this indicates that no GC
 // happened repeatedly for any case.
-TEST(CookieMonsterTest, TestGCTimes) {
+TEST_F(CookieMonsterTest, TestGCTimes) {
+  SetCookieCallback setCookieCallback;
+
   const struct TestCase {
     const char* name;
     int num_cookies;
@@ -338,13 +410,13 @@
     GURL gurl("http://google.com");
     std::string cookie_line("z=3");
     // Trigger the Garbage collection we're allowed.
-    EXPECT_TRUE(cm->SetCookie(gurl, cookie_line));
+    setCookieCallback.SetCookie(cm, gurl, cookie_line);
 
     PerfTimeLogger timer((std::string("GC_") + test_case.name).c_str());
     for (int i = 0; i < kNumCookies; i++)
-      EXPECT_TRUE(cm->SetCookie(gurl, cookie_line));
+      setCookieCallback.SetCookie(cm, gurl, cookie_line);
     timer.Done();
   }
 }
 
-} // namespace
+}  // namespace