Make ServerBoundCertStore NonThreadSafe.

BUG=89665


Review URL: https://chromiumcodereview.appspot.com/11745007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175056 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 15 insertions, 22 deletions

diff --git a/net/base/default_server_bound_cert_store.cc b/net/base/default_server_bound_cert_store.cc
index 5f0cf92..05cd826 100644
--- a/net/base/default_server_bound_cert_store.cc
+++ b/net/base/default_server_bound_cert_store.cc
@@ -19,7 +19,7 @@ DefaultServerBoundCertStore::DefaultServerBoundCertStore(
 
 void DefaultServerBoundCertStore::FlushStore(
     const base::Closure& completion_task) {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
 
   if (initialized_ && store_)
     store_->Flush(completion_task);
@@ -34,7 +34,7 @@ bool DefaultServerBoundCertStore::GetServerBoundCert(
     base::Time* expiration_time,
     std::string* private_key_result,
     std::string* cert_result) {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
   InitIfNecessary();
 
   ServerBoundCertMap::iterator it = server_bound_certs_.find(server_identifier);
@@ -59,7 +59,7 @@ void DefaultServerBoundCertStore::SetServerBoundCert(
     base::Time expiration_time,
     const std::string& private_key,
     const std::string& cert) {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
   InitIfNecessary();
 
   InternalDeleteServerBoundCert(server_identifier);
@@ -72,7 +72,7 @@ void DefaultServerBoundCertStore::SetServerBoundCert(
 
 void DefaultServerBoundCertStore::DeleteServerBoundCert(
     const std::string& server_identifier) {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
   InitIfNecessary();
   InternalDeleteServerBoundCert(server_identifier);
 }
@@ -80,7 +80,7 @@ void DefaultServerBoundCertStore::DeleteServerBoundCert(
 void DefaultServerBoundCertStore::DeleteAllCreatedBetween(
     base::Time delete_begin,
     base::Time delete_end) {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
   InitIfNecessary();
   for (ServerBoundCertMap::iterator it = server_bound_certs_.begin();
        it != server_bound_certs_.end();) {
@@ -103,7 +103,7 @@ void DefaultServerBoundCertStore::DeleteAll() {
 
 void DefaultServerBoundCertStore::GetAllServerBoundCerts(
     ServerBoundCertList* server_bound_certs) {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
   InitIfNecessary();
   for (ServerBoundCertMap::iterator it = server_bound_certs_.begin();
        it != server_bound_certs_.end(); ++it) {
@@ -112,14 +112,14 @@ void DefaultServerBoundCertStore::GetAllServerBoundCerts(
 }
 
 int DefaultServerBoundCertStore::GetCertCount() {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
   InitIfNecessary();
 
   return server_bound_certs_.size();
 }
 
 void DefaultServerBoundCertStore::SetForceKeepSessionState() {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
   InitIfNecessary();
 
   if (store_)
@@ -131,7 +131,7 @@ DefaultServerBoundCertStore::~DefaultServerBoundCertStore() {
 }
 
 void DefaultServerBoundCertStore::DeleteAllInMemory() {
-  base::AutoLock autolock(lock_);
+  DCHECK(CalledOnValidThread());
 
   for (ServerBoundCertMap::iterator it = server_bound_certs_.begin();
        it != server_bound_certs_.end(); ++it) {
@@ -141,8 +141,7 @@ void DefaultServerBoundCertStore::DeleteAllInMemory() {
 }
 
 void DefaultServerBoundCertStore::InitStore() {
-  lock_.AssertAcquired();
-
+  DCHECK(CalledOnValidThread());
   DCHECK(store_) << "Store must exist to initialize";
 
   // Initialize the store and sync in any saved persistent certs.
@@ -160,7 +159,7 @@ void DefaultServerBoundCertStore::InitStore() {
 
 void DefaultServerBoundCertStore::InternalDeleteServerBoundCert(
     const std::string& server_identifier) {
-  lock_.AssertAcquired();
+  DCHECK(CalledOnValidThread());
 
   ServerBoundCertMap::iterator it = server_bound_certs_.find(server_identifier);
   if (it == server_bound_certs_.end())
@@ -176,7 +175,7 @@ void DefaultServerBoundCertStore::InternalDeleteServerBoundCert(
 void DefaultServerBoundCertStore::InternalInsertServerBoundCert(
     const std::string& server_identifier,
     ServerBoundCert* cert) {
-  lock_.AssertAcquired();
+  DCHECK(CalledOnValidThread());
 
   if (store_)
     store_->AddServerBoundCert(*cert);
diff --git a/net/base/default_server_bound_cert_store.h b/net/base/default_server_bound_cert_store.h
index 94571aa..48bf784 100644
--- a/net/base/default_server_bound_cert_store.h
+++ b/net/base/default_server_bound_cert_store.h
@@ -12,7 +12,6 @@
 #include "base/callback_forward.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
-#include "base/synchronization/lock.h"
 #include "net/base/net_export.h"
 #include "net/base/server_bound_cert_store.h"
 
@@ -25,9 +24,6 @@ namespace net {
 // and synchronizes server bound certs to an optional permanent storage that
 // implements the PersistentStore interface. The use case is described in
 // http://balfanz.github.com/tls-obc-spec/draft-balfanz-tls-obc-00.html
-//
-// This class can be accessed by multiple threads. For example, it can be used
-// by IO and server bound cert management UI.
 class NET_EXPORT DefaultServerBoundCertStore : public ServerBoundCertStore {
  public:
   class PersistentStore;
@@ -86,7 +82,6 @@ class NET_EXPORT DefaultServerBoundCertStore : public ServerBoundCertStore {
   // Called by all non-static functions to ensure that the cert store has
   // been initialized. This is not done during creating so it doesn't block
   // the window showing.
-  // Note: this method should always be called with lock_ held.
   void InitIfNecessary() {
     if (!initialized_) {
       if (store_)
@@ -117,9 +112,6 @@ class NET_EXPORT DefaultServerBoundCertStore : public ServerBoundCertStore {
 
   ServerBoundCertMap server_bound_certs_;
 
-  // Lock for thread-safety
-  base::Lock lock_;
-
   DISALLOW_COPY_AND_ASSIGN(DefaultServerBoundCertStore);
 };
 
diff --git a/net/base/server_bound_cert_store.h b/net/base/server_bound_cert_store.h
index abd1b64..7a6f866 100644
--- a/net/base/server_bound_cert_store.h
+++ b/net/base/server_bound_cert_store.h
@@ -8,6 +8,7 @@
 #include <list>
 #include <string>
 
+#include "base/threading/non_thread_safe.h"
 #include "base/time.h"
 #include "net/base/net_export.h"
 #include "net/base/ssl_client_cert_type.h"
@@ -21,7 +22,8 @@ namespace net {
 
 // Owned only by a single ServerBoundCertService object, which is responsible
 // for deleting it.
-class NET_EXPORT ServerBoundCertStore {
+class NET_EXPORT ServerBoundCertStore
+    : NON_EXPORTED_BASE(public base::NonThreadSafe) {
  public:
   // The ServerBoundCert class contains a private key in addition to the server
   // cert, and cert type.