Make ServerBoundCertStore interface async, move SQLiteServerBoundCertStore load onto DB thread.

chrome/browser/net/sqlite_server_bound_cert_store.cc

 // Copyright (c) 2012 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 "chrome/browser/net/sqlite_server_bound_cert_store.h"
 
 #include <list>
 #include <set>
 
 #include "base/basictypes.h"
@@ skipping 23 matching lines @@
     : public base::RefCountedThreadSafe<SQLiteServerBoundCertStore::Backend> {
  public:
   Backend(const FilePath& path, ClearOnExitPolicy* clear_on_exit_policy)
       : path_(path),
         db_(NULL),
         num_pending_(0),
         force_keep_session_state_(false),
         clear_on_exit_policy_(clear_on_exit_policy) {
   }
 
-  // Creates or load the SQLite database.
-  bool Load(
-      std::vector<net::DefaultServerBoundCertStore::ServerBoundCert*>* certs);
+  // Creates or loads the SQLite database.
+  void Load(const LoadedCallback& loaded_callback);
 
   // Batch a server bound cert addition.
   void AddServerBoundCert(
       const net::DefaultServerBoundCertStore::ServerBoundCert& cert);
 
   // Batch a server bound cert deletion.
   void DeleteServerBoundCert(
       const net::DefaultServerBoundCertStore::ServerBoundCert& cert);
 
   // Commit pending operations as soon as possible.
   void Flush(const base::Closure& completion_task);
 
   // Commit any pending operations and close the database.  This must be called
   // before the object is destructed.
   void Close();
 
   void SetForceKeepSessionState();
 
  private:
+  bool LoadOnDBThread(
+      std::vector<net::DefaultServerBoundCertStore::ServerBoundCert*>* certs);
+
   friend class base::RefCountedThreadSafe<SQLiteServerBoundCertStore::Backend>;
 
   // You should call Close() before destructing this object.
   ~Backend() {
     DCHECK(!db_.get()) << "Close should have already been called.";
     DCHECK(num_pending_ == 0 && pending_.empty());
   }
 
   // Database upgrade statements.
   bool EnsureDatabaseVersion();
@@ skipping 72 matching lines @@
                      "expiration_time INTEGER,"
                      "creation_time INTEGER)"))
       return false;
   }
 
   return true;
 }
 
 }  // namespace
 
-bool SQLiteServerBoundCertStore::Backend::Load(
-    std::vector<net::DefaultServerBoundCertStore::ServerBoundCert*>* certs) {
+void SQLiteServerBoundCertStore::Backend::Load(
+    const LoadedCallback& loaded_callback) {
   // This function should be called only once per instance.
   DCHECK(!db_.get());
 
-  // TODO(paivanof@gmail.com): We do a lot of disk access in this function,
-  // thus we do an exception to allow IO on the UI thread. This code will be
-  // moved to the DB thread as part of http://crbug.com/89665.
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
+  scoped_ptr<ScopedVector<net::DefaultServerBoundCertStore::ServerBoundCert> >
+      certs(new ScopedVector<net::DefaultServerBoundCertStore::ServerBoundCert>(
+          ));
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::DB, FROM_HERE,
+      base::Bind(base::IgnoreResult(&Backend::LoadOnDBThread), this,
+                 &certs->get()),
+      base::Bind(loaded_callback, base::Passed(&certs)));
+}
+
+bool SQLiteServerBoundCertStore::Backend::LoadOnDBThread(

[erikwright (departed), 2013/01/04 19:20:10]

why keep the bool exit code?

Also, consider simplifying by making the callback a parameter of LoadOnDBThread.

Then you just bind in the callback, and LoadOnDBThread, upon completion, just
re-binds the callback to the result ScopedVector and posts it back to the IO
thread.

[mattm, 2013/01/08 04:53:21]

removed.
Well, because of all the early exits in LoadOnDBThread, I'd want to make a
wrapper method to do the actual posting instead of directly from LoadOnDBThread.
In that case I'm not sure it's really simpler, but I changed it.

[erikwright (departed), 2013/01/10 22:03:30]

I see your point. I don't mind either way.

+    std::vector<net::DefaultServerBoundCertStore::ServerBoundCert*>* certs) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+
+  // This method should be called only once per instance.
+  DCHECK(!db_.get());
 
   base::TimeTicks start = base::TimeTicks::Now();
 
   // Ensure the parent directory for storing certs is created before reading
   // from it.
   const FilePath dir = path_.DirName();
   if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir))
     return false;
 
   int64 db_size = 0;
@@ skipping 34 matching lines @@
             static_cast<net::SSLClientCertType>(smt.ColumnInt(3)),
             base::Time::FromInternalValue(smt.ColumnInt64(5)),
             base::Time::FromInternalValue(smt.ColumnInt64(4)),
             private_key_from_db,
             cert_from_db));
     cert_origins_.insert(cert->server_identifier());
     certs->push_back(cert.release());
   }
 
   UMA_HISTOGRAM_COUNTS_10000("DomainBoundCerts.DBLoadedCount", certs->size());
+  base::TimeDelta load_time = base::TimeTicks::Now() - start;
   UMA_HISTOGRAM_CUSTOM_TIMES("DomainBoundCerts.DBLoadTime",
-                             base::TimeTicks::Now() - start,
+                             load_time,
                              base::TimeDelta::FromMilliseconds(1),
                              base::TimeDelta::FromMinutes(1),
                              50);
+  DVLOG(1) << "loaded " << certs->size() << " in " << load_time.InMilliseconds()
+           << " ms";
   return true;
 }
 
 bool SQLiteServerBoundCertStore::Backend::EnsureDatabaseVersion() {
   // Version check.
   if (!meta_table_.Init(
       db_.get(), kCurrentVersionNumber, kCompatibleVersionNumber)) {
     return false;
   }
 
@@ skipping 301 matching lines @@
   base::AutoLock locked(lock_);
   force_keep_session_state_ = true;
 }
 
 SQLiteServerBoundCertStore::SQLiteServerBoundCertStore(
     const FilePath& path,
     ClearOnExitPolicy* clear_on_exit_policy)
     : backend_(new Backend(path, clear_on_exit_policy)) {
 }
 
-bool SQLiteServerBoundCertStore::Load(
-    std::vector<net::DefaultServerBoundCertStore::ServerBoundCert*>* certs) {
-  return backend_->Load(certs);
+void SQLiteServerBoundCertStore::Load(
+    const LoadedCallback& loaded_callback) {
+  backend_->Load(loaded_callback);
 }
 
 void SQLiteServerBoundCertStore::AddServerBoundCert(
     const net::DefaultServerBoundCertStore::ServerBoundCert& cert) {
   if (backend_.get())
     backend_->AddServerBoundCert(cert);
 }
 
 void SQLiteServerBoundCertStore::DeleteServerBoundCert(
     const net::DefaultServerBoundCertStore::ServerBoundCert& cert) {
@@ skipping 14 matching lines @@
 }
 
 SQLiteServerBoundCertStore::~SQLiteServerBoundCertStore() {
   if (backend_.get()) {
     backend_->Close();
     // Release our reference, it will probably still have a reference if the
     // background thread has not run Close() yet.
     backend_ = NULL;
   }
 }