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

net/base/cookie_monster_perftest.cc

 // Copyright (c) 2011 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 <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"
 #include "googleurl/src/gurl.h"
 #include "net/base/cookie_monster.h"
 #include "net/base/cookie_monster_store_test.h"
 #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;
 static const char kCookieLine[] = "A  = \"b=;\\\"\"  ;secure;;;";
 
 namespace net {
 
 TEST(ParsedCookieTest, TestParseCookies) {
   std::string cookie(kCookieLine);
   PerfTimeLogger timer("Parsed_cookie_parse_cookies");
@@ skipping 10 matching lines @@
   PerfTimeLogger timer("Parsed_cookie_parse_big_cookies");
   for (int i = 0; i < kNumCookies; ++i) {
     CookieMonster::ParsedCookie pc(cookie);
     EXPECT_TRUE(pc.IsValid());
   }
   timer.Done();
 }
 
 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)));
+
+  SetCookieCallback setCookieCallback;
 
   for (std::vector<GURL>::const_iterator it = gurls.begin();
        it != gurls.end(); ++it) {
-    EXPECT_TRUE(cm->SetCookie(*it, cookie));
+    setCookieCallback.SetCookie(cm, *it, kCookieLine);
   }
 
+  GetCookiesWithInfoCallback getCookiesCallback;
+
   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);
+    getCookiesCallback.GetCookiesWithInfo(cm, *it);
   }
   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)));
-
-  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);
-  }
-  timer.Done();
-}
 
 static int CountInString(const std::string& str, char c) {
   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");
 
   std::vector<std::string> domain_list;
 
   // Create a balanced binary tree of domains on which the cookie is set.
   domain_list.push_back(domain_base);
   for (int i1 = 0; i1 < 2; i1++) {
     std::string domain_base_1((i1 ? "a." : "b.") + domain_base);
     EXPECT_EQ("top.com", cm->GetKey(domain_base_1));
@@ skipping 15 matching lines @@
     }
   }
 
 
   EXPECT_EQ(31u, domain_list.size());
   for (std::vector<std::string>::const_iterator it = domain_list.begin();
        it != domain_list.end(); it++) {
     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;
 
   // Create a line of 32 domain cookies such that all cookies stored
   // by effective TLD+1 will apply to probe GURL.
   // (TLD + 1 is the level above .com/org/net/etc, e.g. "top.com"
   // or "google.com".  "Effective" is added to include sites like
   // bbc.co.uk, where the effetive TLD+1 is more than one level
   // below the top level.)
   domain_list.push_back("a.top.com");
   domain_list.push_back("b.a.top.com");
   domain_list.push_back("a.b.a.top.com");
   domain_list.push_back("b.a.b.a.top.com");
   EXPECT_EQ(4u, domain_list.size());
 
   const char* domain_cookie_format_line = "a%03d=b; domain=%s";
   for (int i = 0; i < 8; i++) {
     for (std::vector<std::string>::const_iterator it = domain_list.begin();
          it != domain_list.end(); it++) {
       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.
   int64 time_tick(base::Time::Now().ToInternalValue());
 
   for (int domain_num = 0; domain_num < 300; domain_num++) {
     std::string domain_name(base::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(base::StringPrintf("Cookie_%d=1; Path=/",
                                                  cookie_num));
       AddCookieToList(gurl, cookie_line,
                       base::Time::FromInternalValue(time_tick++),
                       &initial_cookies);
     }
   }
 
   store->SetLoadExpectation(true, initial_cookies);
 
   scoped_refptr<CookieMonster> cm(new CookieMonster(store, NULL));
 
   // Import will happen on first access.
   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++)
     cm->GetKey("www.google.com");
   timer.Done();
 }
 
 // This test is probing for whether garbage collection happens when it
 // shouldn't.  This will not in general be visible functionally, since
 // if GC runs twice in a row without any change to the store, the second
 // GC run will not do anything the first one didn't.  That's why this is
 // 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;
     int num_old_cookies;
   } test_cases[] = {
     {
       // A whole lot of recent cookies; gc shouldn't happen.
       "all_recent",
       CookieMonster::kMaxCookies * 2,
       0,
@@ skipping 23 matching lines @@
   for (int ci = 0; ci < static_cast<int>(ARRAYSIZE_UNSAFE(test_cases)); ++ci) {
     const TestCase& test_case(test_cases[ci]);
     scoped_refptr<CookieMonster> cm(
         CreateMonsterFromStoreForGC(
             test_case.num_cookies, test_case.num_old_cookies,
             CookieMonster::kSafeFromGlobalPurgeDays * 2));
 
     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