Remove the old synchronous CookieMonster API.

chrome/browser/net/sqlite_persistent_cookie_store.cc

Index: chrome/browser/net/sqlite_persistent_cookie_store.cc
===================================================================
--- chrome/browser/net/sqlite_persistent_cookie_store.cc	(revision 95998)
+++ chrome/browser/net/sqlite_persistent_cookie_store.cc	(working copy)
@@ -7,6 +7,7 @@
 #include <list>
 
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ -38,7 +39,7 @@
   }
 
   // Creates or load the SQLite database.
-  bool Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies);
+  bool Load(const LoadedCallback& loaded_callback);
 
   // Batch a cookie addition.
   void AddCookie(const net::CookieMonster::CanonicalCookie& cc);
@@ -91,6 +92,11 @@
   };
 
  private:
+  // Creates or load the SQLite database on DB thread.
+  void LoadOnDBThread();
+  // Notify the CookieMonster when loading complete.
+  void Notify(bool load_success_);
+
   // Batch a cookie operation (add or delete)
   void BatchOperation(PendingOperation::OperationType op,
                       const net::CookieMonster::CanonicalCookie& cc);
@@ -103,6 +109,9 @@
   scoped_ptr<sql::Connection> db_;
   sql::MetaTable meta_table_;
 
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies_;
+  LoadedCallback loaded_callback_;
+
   typedef std::list<PendingOperation*> PendingOperationsList;
   PendingOperationsList pending_;
   PendingOperationsList::size_type num_pending_;
@@ -154,26 +163,37 @@
 }  // namespace
 
 bool SQLitePersistentCookieStore::Backend::Load(
-    std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
+    const LoadedCallback& loaded_callback) {
   // This function should be called only once per instance.
   DCHECK(!db_.get());
+  loaded_callback_ = loaded_callback;

[erikwright (departed), 2011/08/11 15:13:52]

DCHECK(loaded_callback_.is_null());

[ycxiao, 2011/08/12 02:35:24]

Done.

+  BrowserThread::PostTask(
+      BrowserThread::DB, FROM_HERE,
+      base::Bind(&Backend::LoadOnDBThread, base::Unretained(this)));
+  return true;
+}
 
+void SQLitePersistentCookieStore::Backend::LoadOnDBThread() {

[erikwright (departed), 2011/08/11 15:13:52]

Write LoadAndNotifyOnDBThread that does Notify(LoadInternal());

Then you can just return true/false from LoadInternal.

[ycxiao, 2011/08/12 02:35:24]

Done.

   // Ensure the parent directory for storing cookies is created before reading

[erikwright (departed), 2011/08/11 15:13:52]

Comment is now out of date.

[ycxiao, 2011/08/12 02:35:24]

Done.

   // from it.  We make an exception to allow IO on the UI thread here because
   // we are going to disk anyway in db_->Open.  (This code will be moved to the
   // DB thread as part of http://crbug.com/52909.)
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   {
     base::ThreadRestrictions::ScopedAllowIO allow_io;

[erikwright (departed), 2011/08/11 15:13:52]

ScopedAllowIO, no longer required. Move other code to outer scope.

[ycxiao, 2011/08/12 02:35:24]

Done.

     const FilePath dir = path_.DirName();
-    if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir))
-      return false;
+    if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir)) {
+      Notify(false);
+      return;
+    }
   }
 
   db_.reset(new sql::Connection);
   if (!db_->Open(path_)) {
     NOTREACHED() << "Unable to open cookie DB.";
     db_.reset();
-    return false;
+    Notify(false);
+    return;
   }
 
   db_->set_error_delegate(GetErrorHandlerForCookieDb());
@@ -181,7 +201,8 @@
   if (!EnsureDatabaseVersion() || !InitTable(db_.get())) {
     NOTREACHED() << "Unable to open cookie DB.";
     db_.reset();
-    return false;
+    Notify(false);
+    return;
   }
 
   db_->Preload();

[erikwright (departed), 2011/08/11 15:13:52]

Lines 184-208 should be a helper method like 'InitializeDatabase' or similar.

[ycxiao, 2011/08/12 02:35:24]

Done.

@@ -193,9 +214,15 @@
   if (!smt) {
     NOTREACHED() << "select statement prep failed";
     db_.reset();
-    return false;
+    Notify(false);
+    return;
   }
 
+  // Reserve space for the maximum amount of cookies a database should have.
+  // This prevents multiple vector growth / copies as we append cookies.
+  const size_t kMaxCookies = 3300;
+  cookies_.reserve(kMaxCookies);
+
   while (smt.Step()) {
     scoped_ptr<net::CookieMonster::CanonicalCookie> cc(
         new net::CookieMonster::CanonicalCookie(
@@ -215,12 +242,24 @@
             true));                                         // has_expires
     DLOG_IF(WARNING,
             cc->CreationDate() > Time::Now()) << L"CreationDate too recent";
-    cookies->push_back(cc.release());
+    cookies_.push_back(cc.release());
   }
 
-  return true;
+  Notify(true);
 }
 
+void SQLitePersistentCookieStore::Backend::Notify(bool load_success) {

[erikwright (departed), 2011/08/11 15:13:52]

It seems the success value is not used? Remove it?

[ycxiao, 2011/08/12 02:35:24]

I also notice that it is not used. But this flag passes useful information which
maybe wanted in the future. So I am not sure how to deal with it. 

+  if (BrowserThread::CurrentlyOn(BrowserThread::DB)) {

[erikwright (departed), 2011/08/11 15:13:52]

Instead of having a thread switch here, you can simply do the PostTask to IO
thread in LoadAndNotifyOnDBThread.

So rename this to NotifyOnIOThread and remove lines 252-258.

[ycxiao, 2011/08/12 02:35:24]

Done.

+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&SQLitePersistentCookieStore::Backend::Notify,
+                   base::Unretained(this), load_success));
+    return;
+  }
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  loaded_callback_.Run(cookies_);
+}
+
 bool SQLitePersistentCookieStore::Backend::EnsureDatabaseVersion() {
   // Version check.
   if (!meta_table_.Init(
@@ -483,9 +522,8 @@
   }
 }
 
-bool SQLitePersistentCookieStore::Load(
-    std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
-  return backend_->Load(cookies);
+bool SQLitePersistentCookieStore::Load(const LoadedCallback& loaded_callback) {
+  return backend_->Load(loaded_callback);
 }
 
 void SQLitePersistentCookieStore::AddCookie(