Make NSSCertDatabase::ListCerts async and use the worker pool for actual certificate listing.

BUG=340460

TBR=sky@chromium.org (for rename in chrome/browser/certificate_manager_model.cc)

Review URL: https://codereview.chromium.org/144423007

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

9 files changed, 254 insertions, 44 deletions

diff --git a/net/cert/nss_cert_database.cc b/net/cert/nss_cert_database.cc
index 935b271..ed861b2 100644
--- a/net/cert/nss_cert_database.cc
+++ b/net/cert/nss_cert_database.cc
@@ -10,10 +10,14 @@
 #include <pk11pub.h>
 #include <secmod.h>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list_threadsafe.h"
+#include "base/task_runner.h"
+#include "base/threading/worker_pool.h"
 #include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
 #include "crypto/scoped_nss_types.h"
@@ -42,7 +46,6 @@ base::LazyInstance<NSSCertDatabase>::Leaky
 
 }  // namespace
 
-
 NSSCertDatabase::ImportCertFailure::ImportCertFailure(
     const scoped_refptr<X509Certificate>& cert,
     int err)
@@ -71,18 +74,21 @@ NSSCertDatabase::NSSCertDatabase()
 
 NSSCertDatabase::~NSSCertDatabase() {}
 
-void NSSCertDatabase::ListCerts(CertificateList* certs) {
-  certs->clear();
+void NSSCertDatabase::ListCertsSync(CertificateList* certs) {
+  ListCertsImpl(certs);
+}
 
-  CERTCertList* cert_list = PK11_ListCerts(PK11CertListUnique, NULL);
-  CERTCertListNode* node;
-  for (node = CERT_LIST_HEAD(cert_list);
-       !CERT_LIST_END(node, cert_list);
-       node = CERT_LIST_NEXT(node)) {
-    certs->push_back(X509Certificate::CreateFromHandle(
-        node->cert, X509Certificate::OSCertHandles()));
-  }
-  CERT_DestroyCertList(cert_list);
+void NSSCertDatabase::ListCerts(
+    const base::Callback<void(scoped_ptr<CertificateList> certs)>& callback) {
+  scoped_ptr<CertificateList> certs(new CertificateList());
+
+  // base::Pased will NULL out |certs|, so cache the underlying pointer here.
+  CertificateList* raw_certs = certs.get();
+  GetSlowTaskRunner()->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&NSSCertDatabase::ListCertsImpl,
+                 base::Unretained(raw_certs)),
+      base::Bind(callback, base::Passed(&certs)));
 }
 
 crypto::ScopedPK11Slot NSSCertDatabase::GetPublicSlot() const {
@@ -350,6 +356,32 @@ void NSSCertDatabase::RemoveObserver(Observer* observer) {
   observer_list_->RemoveObserver(observer);
 }
 
+void NSSCertDatabase::SetSlowTaskRunnerForTest(
+    const scoped_refptr<base::TaskRunner>& task_runner) {
+  slow_task_runner_for_test_ = task_runner;
+}
+
+// static
+void NSSCertDatabase::ListCertsImpl(CertificateList* certs) {
+  certs->clear();
+
+  CERTCertList* cert_list = PK11_ListCerts(PK11CertListUnique, NULL);
+  CERTCertListNode* node;
+  for (node = CERT_LIST_HEAD(cert_list);
+       !CERT_LIST_END(node, cert_list);
+       node = CERT_LIST_NEXT(node)) {
+    certs->push_back(X509Certificate::CreateFromHandle(
+        node->cert, X509Certificate::OSCertHandles()));
+  }
+  CERT_DestroyCertList(cert_list);
+}
+
+scoped_refptr<base::TaskRunner> NSSCertDatabase::GetSlowTaskRunner() const {
+  if (slow_task_runner_for_test_)
+    return slow_task_runner_for_test_;
+  return base::WorkerPool::GetTaskRunner(true /*task is slow*/);
+}
+
 void NSSCertDatabase::NotifyObserversOfCertAdded(const X509Certificate* cert) {
   observer_list_->Notify(&Observer::OnCertAdded, make_scoped_refptr(cert));
 }
diff --git a/net/cert/nss_cert_database.h b/net/cert/nss_cert_database.h
index 0707b8d..94f1f20 100644
--- a/net/cert/nss_cert_database.h
+++ b/net/cert/nss_cert_database.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback_forward.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
 #include "crypto/scoped_nss_types.h"
@@ -18,6 +19,7 @@
 
 namespace base {
 template <typename T> struct DefaultLazyInstanceTraits;
+class TaskRunner;
 }
 template <class ObserverType> class ObserverListThreadSafe;
 
@@ -91,12 +93,21 @@ class NET_EXPORT NSSCertDatabase {
     DISTRUSTED_OBJ_SIGN   = 1 << 5,
   };
 
+  typedef base::Callback<void(scoped_ptr<CertificateList> certs)>
+      ListCertsCallback;
+
   // DEPRECATED: See http://crbug.com/329735.
   static NSSCertDatabase* GetInstance();
 
   // Get a list of unique certificates in the certificate database (one
   // instance of all certificates).
-  virtual void ListCerts(CertificateList* certs);
+  // DEPRECATED by |ListCerts|. See http://crbug.com/340460.
+  virtual void ListCertsSync(CertificateList* certs);
+
+  // Asynchronously get a list of unique certificates in the certificate
+  // database (one instance of all certificates). Note that the callback may be
+  // run even after the database is deleted.
+  virtual void ListCerts(const ListCertsCallback& callback);
 
   // Get the default slot for public key data.
   virtual crypto::ScopedPK11Slot GetPublicSlot() const;
@@ -209,10 +220,23 @@ class NET_EXPORT NSSCertDatabase {
   // on the same thread on which AddObserver() was called.
   void RemoveObserver(Observer* observer);
 
+  // Overrides task runner that's used for running slow tasks.
+  void SetSlowTaskRunnerForTest(
+      const scoped_refptr<base::TaskRunner>& task_runner);
+
  protected:
   NSSCertDatabase();
   virtual ~NSSCertDatabase();
 
+  // Certificate listing implementation used by |ListCerts| and |ListCertsSync|.
+  // Static so it may safely be used on the worker thread.
+  static void ListCertsImpl(CertificateList* certs);
+
+  // Gets task runner that should be used for slow tasks like certificate
+  // listing. Defaults to a base::WorkerPool runner, but may be overriden
+  // in tests (see SetSlowTaskRunnerForTest).
+  scoped_refptr<base::TaskRunner> GetSlowTaskRunner() const;
+
  private:
   friend struct base::DefaultLazyInstanceTraits<NSSCertDatabase>;
 
@@ -221,6 +245,9 @@ class NET_EXPORT NSSCertDatabase {
   void NotifyObserversOfCertRemoved(const X509Certificate* cert);
   void NotifyObserversOfCACertChanged(const X509Certificate* cert);
 
+  // Task runner that should be used in tests if set.
+  scoped_refptr<base::TaskRunner> slow_task_runner_for_test_;
+
   const scoped_refptr<ObserverListThreadSafe<Observer> > observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(NSSCertDatabase);
diff --git a/net/cert/nss_cert_database_chromeos.cc b/net/cert/nss_cert_database_chromeos.cc
index 936287c..79e5781 100644
--- a/net/cert/nss_cert_database_chromeos.cc
+++ b/net/cert/nss_cert_database_chromeos.cc
@@ -7,6 +7,12 @@
 #include <cert.h>
 #include <pk11pub.h>
 
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/location.h"
+#include "base/task_runner.h"
 #include "net/base/crypto_module.h"
 #include "net/cert/x509_certificate.h"
 
@@ -22,18 +28,22 @@ NSSCertDatabaseChromeOS::NSSCertDatabaseChromeOS(
 
 NSSCertDatabaseChromeOS::~NSSCertDatabaseChromeOS() {}
 
-void NSSCertDatabaseChromeOS::ListCerts(CertificateList* certs) {
-  NSSCertDatabase::ListCerts(certs);
+void NSSCertDatabaseChromeOS::ListCertsSync(CertificateList* certs) {
+  ListCertsImpl(profile_filter_, certs);
+}
 
-  size_t pre_size = certs->size();
-  certs->erase(std::remove_if(
-                   certs->begin(),
-                   certs->end(),
-                   NSSProfileFilterChromeOS::CertNotAllowedForProfilePredicate(
-                       profile_filter_)),
-               certs->end());
-  DVLOG(1) << "filtered " << pre_size - certs->size() << " of " << pre_size
-           << " certs";
+void NSSCertDatabaseChromeOS::ListCerts(
+    const base::Callback<void(scoped_ptr<CertificateList> certs)>& callback) {
+  scoped_ptr<CertificateList> certs(new CertificateList());
+
+  // base::Pased will NULL out |certs|, so cache the underlying pointer here.
+  CertificateList* raw_certs = certs.get();
+  GetSlowTaskRunner()->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&NSSCertDatabaseChromeOS::ListCertsImpl,
+                 profile_filter_,
+                 base::Unretained(raw_certs)),
+      base::Bind(callback, base::Passed(&certs)));
 }
 
 crypto::ScopedPK11Slot NSSCertDatabaseChromeOS::GetPublicSlot() const {
@@ -62,4 +72,20 @@ void NSSCertDatabaseChromeOS::ListModules(CryptoModuleList* modules,
            << " modules";
 }
 
+void NSSCertDatabaseChromeOS::ListCertsImpl(
+    const NSSProfileFilterChromeOS& profile_filter,
+    CertificateList* certs) {
+  NSSCertDatabase::ListCertsImpl(certs);
+
+  size_t pre_size = certs->size();
+  certs->erase(std::remove_if(
+                   certs->begin(),
+                   certs->end(),
+                   NSSProfileFilterChromeOS::CertNotAllowedForProfilePredicate(
+                       profile_filter)),
+               certs->end());
+  DVLOG(1) << "filtered " << pre_size - certs->size() << " of " << pre_size
+           << " certs";
+}
+
 }  // namespace net
diff --git a/net/cert/nss_cert_database_chromeos.h b/net/cert/nss_cert_database_chromeos.h
index 63eab07..07a1e67 100644
--- a/net/cert/nss_cert_database_chromeos.h
+++ b/net/cert/nss_cert_database_chromeos.h
@@ -6,7 +6,9 @@
 #define NET_CERT_NSS_CERT_DATABASE_CHROMEOS_
 
 #include "base/callback.h"
+#include "base/memory/weak_ptr.h"
 #include "crypto/scoped_nss_types.h"
+#include "net/base/net_export.h"
 #include "net/cert/nss_cert_database.h"
 #include "net/cert/nss_profile_filter_chromeos.h"
 
@@ -19,7 +21,9 @@ class NET_EXPORT NSSCertDatabaseChromeOS : public NSSCertDatabase {
   virtual ~NSSCertDatabaseChromeOS();
 
   // NSSCertDatabase implementation.
-  virtual void ListCerts(CertificateList* certs) OVERRIDE;
+  virtual void ListCertsSync(CertificateList* certs) OVERRIDE;
+  virtual void ListCerts(const NSSCertDatabase::ListCertsCallback& callback)
+      OVERRIDE;
   virtual crypto::ScopedPK11Slot GetPublicSlot() const OVERRIDE;
   virtual crypto::ScopedPK11Slot GetPrivateSlot() const OVERRIDE;
   virtual void ListModules(CryptoModuleList* modules, bool need_rw) const
@@ -30,6 +34,13 @@ class NET_EXPORT NSSCertDatabaseChromeOS : public NSSCertDatabase {
   // TODO(mattm): handle trust setting correctly for certs in read-only slots.
 
  private:
+  // Certificate listing implementation used by |ListCerts| and |ListCertsSync|.
+  // The certificate list normally returned by NSSCertDatabase::ListCertsImpl
+  // is additionally filtered by |profile_filter|.
+  // Static so it may safely be used on the worker thread.
+  static void ListCertsImpl(const NSSProfileFilterChromeOS& profile_filter,
+                            CertificateList* certs);
+
   crypto::ScopedPK11Slot public_slot_;
   crypto::ScopedPK11Slot private_slot_;
   NSSProfileFilterChromeOS profile_filter_;
diff --git a/net/cert/nss_cert_database_chromeos_unittest.cc b/net/cert/nss_cert_database_chromeos_unittest.cc
index 465d25d..324575d 100644
--- a/net/cert/nss_cert_database_chromeos_unittest.cc
+++ b/net/cert/nss_cert_database_chromeos_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
 #include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
@@ -30,6 +31,14 @@ bool IsCertInCertificateList(const X509Certificate* cert,
   return false;
 }
 
+void SwapCertLists(CertificateList* destination,
+                   scoped_ptr<CertificateList> source) {
+  ASSERT_TRUE(destination);
+  ASSERT_TRUE(source);
+
+  destination->swap(*source);
+}
+
 }  // namespace
 
 class NSSCertDatabaseChromeOSTest : public testing::Test,
@@ -51,11 +60,13 @@ class NSSCertDatabaseChromeOSTest : public testing::Test,
         crypto::GetPrivateSlotForChromeOSUser(
             user_1_.username_hash(),
             base::Callback<void(crypto::ScopedPK11Slot)>())));
+    db_1_->SetSlowTaskRunnerForTest(base::MessageLoopProxy::current());
     db_2_.reset(new NSSCertDatabaseChromeOS(
         crypto::GetPublicSlotForChromeOSUser(user_2_.username_hash()),
         crypto::GetPrivateSlotForChromeOSUser(
             user_2_.username_hash(),
             base::Callback<void(crypto::ScopedPK11Slot)>())));
+    db_2_->SetSlowTaskRunnerForTest(base::MessageLoopProxy::current());
 
     // Add observer to CertDatabase for checking that notifications from
     // NSSCertDatabaseChromeOS are proxied to the CertDatabase.
@@ -150,8 +161,8 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportCACerts) {
   // Get cert list for each user.
   CertificateList user_1_certlist;
   CertificateList user_2_certlist;
-  db_1_->ListCerts(&user_1_certlist);
-  db_2_->ListCerts(&user_2_certlist);
+  db_1_->ListCertsSync(&user_1_certlist);
+  db_2_->ListCertsSync(&user_2_certlist);
 
   // Check that the imported certs only shows up in the list for the user that
   // imported them.
@@ -170,6 +181,22 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportCACerts) {
   // EXPECT_EQ(certs_1[0]->os_cert_handle(), added_ca_[0]);
   // EXPECT_EQ(certs_2[0]->os_cert_handle(), added_ca_[1]);
   EXPECT_EQ(0U, added_.size());
+
+  // Tests that the new certs are loaded by async ListCerts method.
+  CertificateList user_1_certlist_async;
+  CertificateList user_2_certlist_async;
+  db_1_->ListCerts(
+      base::Bind(&SwapCertLists, base::Unretained(&user_1_certlist_async)));
+  db_2_->ListCerts(
+      base::Bind(&SwapCertLists, base::Unretained(&user_2_certlist_async)));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(IsCertInCertificateList(certs_1[0], user_1_certlist_async));
+  EXPECT_FALSE(IsCertInCertificateList(certs_1[0], user_2_certlist_async));
+
+  EXPECT_TRUE(IsCertInCertificateList(certs_2[0], user_2_certlist_async));
+  EXPECT_FALSE(IsCertInCertificateList(certs_2[0], user_1_certlist_async));
 }
 
 // Test that ImportServerCerts imports the cert to the correct slot, and that
@@ -200,8 +227,8 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportServerCert) {
   // Get cert list for each user.
   CertificateList user_1_certlist;
   CertificateList user_2_certlist;
-  db_1_->ListCerts(&user_1_certlist);
-  db_2_->ListCerts(&user_2_certlist);
+  db_1_->ListCertsSync(&user_1_certlist);
+  db_2_->ListCertsSync(&user_2_certlist);
 
   // Check that the imported certs only shows up in the list for the user that
   // imported them.
@@ -217,6 +244,36 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportServerCert) {
   // fire. Is that correct?
   EXPECT_EQ(0U, added_ca_.size());
   EXPECT_EQ(0U, added_.size());
+
+  // Tests that the new certs are loaded by async ListCerts method.
+  CertificateList user_1_certlist_async;
+  CertificateList user_2_certlist_async;
+  db_1_->ListCerts(
+      base::Bind(&SwapCertLists, base::Unretained(&user_1_certlist_async)));
+  db_2_->ListCerts(
+      base::Bind(&SwapCertLists, base::Unretained(&user_2_certlist_async)));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(IsCertInCertificateList(certs_1[0], user_1_certlist_async));
+  EXPECT_FALSE(IsCertInCertificateList(certs_1[0], user_2_certlist_async));
+
+  EXPECT_TRUE(IsCertInCertificateList(certs_2[0], user_2_certlist_async));
+  EXPECT_FALSE(IsCertInCertificateList(certs_2[0], user_1_certlist_async));
+}
+
+// Tests that There is no crash if the database is deleted while ListCerts
+// is being processed on the worker pool.
+TEST_F(NSSCertDatabaseChromeOSTest, NoCrashIfShutdownBeforeDoneOnWorkerPool) {
+  CertificateList certlist;
+  db_1_->ListCerts(base::Bind(&SwapCertLists, base::Unretained(&certlist)));
+  EXPECT_EQ(0U, certlist.size());
+
+  db_1_.reset();
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_LT(0U, certlist.size());
 }
 
 }  // namespace net
diff --git a/net/cert/nss_cert_database_unittest.cc b/net/cert/nss_cert_database_unittest.cc
index 39e1a33..342e0b9 100644
--- a/net/cert/nss_cert_database_unittest.cc
+++ b/net/cert/nss_cert_database_unittest.cc
@@ -8,11 +8,14 @@
 
 #include <algorithm>
 
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/path_service.h"
+#include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -41,6 +44,16 @@ using base::ASCIIToUTF16;
 
 namespace net {
 
+namespace {
+
+void SwapCertList(CertificateList* destination,
+                  scoped_ptr<CertificateList> source) {
+  ASSERT_TRUE(destination);
+  destination->swap(*source);
+}
+
+}  // namespace
+
 class CertDatabaseNSSTest : public testing::Test {
  public:
   virtual void SetUp() {
@@ -127,11 +140,26 @@ class CertDatabaseNSSTest : public testing::Test {
   crypto::ScopedTestNSSDB test_nssdb_;
 };
 
+TEST_F(CertDatabaseNSSTest, ListCertsSync) {
+  // This test isn't terribly useful, though it will at least let valgrind test
+  // for leaks.
+  CertificateList certs;
+  cert_db_->ListCertsSync(&certs);
+  // The test DB is empty, but let's assume there will always be something in
+  // the other slots.
+  EXPECT_LT(0U, certs.size());
+}
+
 TEST_F(CertDatabaseNSSTest, ListCerts) {
   // This test isn't terribly useful, though it will at least let valgrind test
   // for leaks.
   CertificateList certs;
-  cert_db_->ListCerts(&certs);
+  cert_db_->SetSlowTaskRunnerForTest(base::MessageLoopProxy::current());
+  cert_db_->ListCerts(base::Bind(&SwapCertList, base::Unretained(&certs)));
+  EXPECT_EQ(0U, certs.size());
+
+  base::RunLoop().RunUntilIdle();
+
   // The test DB is empty, but let's assume there will always be something in
   // the other slots.
   EXPECT_LT(0U, certs.size());
diff --git a/net/cert/nss_profile_filter_chromeos.cc b/net/cert/nss_profile_filter_chromeos.cc
index 4871817..906780f 100644
--- a/net/cert/nss_profile_filter_chromeos.cc
+++ b/net/cert/nss_profile_filter_chromeos.cc
@@ -4,9 +4,8 @@
 
 #include "net/cert/nss_profile_filter_chromeos.h"
 
-#include "base/bind.h"
-#include "base/callback.h"
 #include "base/strings/stringprintf.h"
+#include "net/cert/x509_certificate.h"
 
 namespace net {
 
@@ -35,8 +34,29 @@ std::string CertSlotsString(const scoped_refptr<X509Certificate>& cert) {
 
 NSSProfileFilterChromeOS::NSSProfileFilterChromeOS() {}
 
+NSSProfileFilterChromeOS::NSSProfileFilterChromeOS(
+    const NSSProfileFilterChromeOS& other) {
+  public_slot_.reset(other.public_slot_ ?
+      PK11_ReferenceSlot(other.public_slot_.get()) :
+      NULL);
+  private_slot_.reset(other.private_slot_ ?
+      PK11_ReferenceSlot(other.private_slot_.get()) :
+      NULL);
+}
+
 NSSProfileFilterChromeOS::~NSSProfileFilterChromeOS() {}
 
+NSSProfileFilterChromeOS& NSSProfileFilterChromeOS::operator=(
+    const NSSProfileFilterChromeOS& other) {
+  public_slot_.reset(other.public_slot_ ?
+      PK11_ReferenceSlot(other.public_slot_.get()) :
+      NULL);
+  private_slot_.reset(other.private_slot_ ?
+      PK11_ReferenceSlot(other.private_slot_.get()) :
+      NULL);
+  return *this;
+}
+
 void NSSProfileFilterChromeOS::Init(crypto::ScopedPK11Slot public_slot,
                                     crypto::ScopedPK11Slot private_slot) {
   public_slot_ = public_slot.Pass();
diff --git a/net/cert/nss_profile_filter_chromeos.h b/net/cert/nss_profile_filter_chromeos.h
index d5ff818..ae310c6 100644
--- a/net/cert/nss_profile_filter_chromeos.h
+++ b/net/cert/nss_profile_filter_chromeos.h
@@ -2,26 +2,27 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_CERT_NSS_PROFILE_FILTER_CHROMEOS
-#define NET_CERT_NSS_PROFILE_FILTER_CHROMEOS
+#ifndef NET_CERT_NSS_PROFILE_FILTER_CHROMEOS_H_
+#define NET_CERT_NSS_PROFILE_FILTER_CHROMEOS_H_
 
-#include "base/callback_forward.h"
+#include "base/memory/scoped_ptr.h"
 #include "crypto/scoped_nss_types.h"
 #include "net/base/crypto_module.h"
-#include "net/cert/x509_certificate.h"
-
-namespace content {
-class ResourceContext;
-}  // namespace content
+#include "net/base/net_export.h"
 
 namespace net {
 
+class X509Certificate;
+
 class NET_EXPORT NSSProfileFilterChromeOS {
  public:
   NSSProfileFilterChromeOS();
+  NSSProfileFilterChromeOS(const NSSProfileFilterChromeOS& other);
   ~NSSProfileFilterChromeOS();
 
-  // Initialize with slot handles.
+  NSSProfileFilterChromeOS& operator=(const NSSProfileFilterChromeOS& other);
+
+  // Initializes the filter with slot handles.
   void Init(crypto::ScopedPK11Slot public_slot,
             crypto::ScopedPK11Slot private_slot);
 
@@ -51,10 +52,8 @@ class NET_EXPORT NSSProfileFilterChromeOS {
  private:
   crypto::ScopedPK11Slot public_slot_;
   crypto::ScopedPK11Slot private_slot_;
-
-  DISALLOW_COPY_AND_ASSIGN(NSSProfileFilterChromeOS);
 };
 
 }  // namespace net
 
-#endif  // NET_CERT_NSS_PROFILE_FILTER_CHROMEOS
+#endif  // NET_CERT_NSS_PROFILE_FILTER_CHROMEOS_H_
diff --git a/net/cert/nss_profile_filter_chromeos_unittest.cc b/net/cert/nss_profile_filter_chromeos_unittest.cc
index 8f38061..1f6260d 100644
--- a/net/cert/nss_profile_filter_chromeos_unittest.cc
+++ b/net/cert/nss_profile_filter_chromeos_unittest.cc
@@ -76,6 +76,8 @@ class NSSProfileFilterChromeOSTest : public testing::Test {
         crypto::GetPublicSlotForChromeOSUser(user_1_.username_hash()),
         private_slot_1.Pass());
 
+    profile_filter_1_copy_ = profile_filter_1_;
+
     crypto::ScopedPK11Slot private_slot_2(crypto::GetPrivateSlotForChromeOSUser(
         user_2_.username_hash(),
         base::Callback<void(crypto::ScopedPK11Slot)>()));
@@ -97,12 +99,14 @@ class NSSProfileFilterChromeOSTest : public testing::Test {
   NSSProfileFilterChromeOS no_slots_profile_filter_;
   NSSProfileFilterChromeOS profile_filter_1_;
   NSSProfileFilterChromeOS profile_filter_2_;
+  NSSProfileFilterChromeOS profile_filter_1_copy_;
 };
 
 TEST_F(NSSProfileFilterChromeOSTest, TempCertAllowed) {
   EXPECT_EQ(NULL, certs_[0]->os_cert_handle()->slot);
   EXPECT_TRUE(no_slots_profile_filter_.IsCertAllowed(certs_[0]));
   EXPECT_TRUE(profile_filter_1_.IsCertAllowed(certs_[0]));
+  EXPECT_TRUE(profile_filter_1_copy_.IsCertAllowed(certs_[0]));
   EXPECT_TRUE(profile_filter_2_.IsCertAllowed(certs_[0]));
 }
 
@@ -111,6 +115,7 @@ TEST_F(NSSProfileFilterChromeOSTest, InternalSlotAllowed) {
   ASSERT_TRUE(internal_slot.get());
   EXPECT_TRUE(no_slots_profile_filter_.IsModuleAllowed(internal_slot.get()));
   EXPECT_TRUE(profile_filter_1_.IsModuleAllowed(internal_slot.get()));
+  EXPECT_TRUE(profile_filter_1_copy_.IsModuleAllowed(internal_slot.get()));
   EXPECT_TRUE(profile_filter_2_.IsModuleAllowed(internal_slot.get()));
 
   crypto::ScopedPK11Slot internal_key_slot(PK11_GetInternalKeySlot());
@@ -118,6 +123,7 @@ TEST_F(NSSProfileFilterChromeOSTest, InternalSlotAllowed) {
   EXPECT_TRUE(
       no_slots_profile_filter_.IsModuleAllowed(internal_key_slot.get()));
   EXPECT_TRUE(profile_filter_1_.IsModuleAllowed(internal_key_slot.get()));
+  EXPECT_TRUE(profile_filter_1_copy_.IsModuleAllowed(internal_key_slot.get()));
   EXPECT_TRUE(profile_filter_2_.IsModuleAllowed(internal_key_slot.get()));
 }
 
@@ -126,12 +132,14 @@ TEST_F(NSSProfileFilterChromeOSTest, RootCertsAllowed) {
   ASSERT_TRUE(root_certs_slot.get());
   EXPECT_TRUE(no_slots_profile_filter_.IsModuleAllowed(root_certs_slot.get()));
   EXPECT_TRUE(profile_filter_1_.IsModuleAllowed(root_certs_slot.get()));
+  EXPECT_TRUE(profile_filter_1_copy_.IsModuleAllowed(root_certs_slot.get()));
   EXPECT_TRUE(profile_filter_2_.IsModuleAllowed(root_certs_slot.get()));
 
   CertificateList root_certs(ListCertsInSlot(root_certs_slot.get()));
   ASSERT_FALSE(root_certs.empty());
   EXPECT_TRUE(no_slots_profile_filter_.IsCertAllowed(root_certs[0]));
   EXPECT_TRUE(profile_filter_1_.IsCertAllowed(root_certs[0]));
+  EXPECT_TRUE(profile_filter_1_copy_.IsCertAllowed(root_certs[0]));
   EXPECT_TRUE(profile_filter_2_.IsCertAllowed(root_certs[0]));
 }
 
@@ -167,7 +175,9 @@ TEST_F(NSSProfileFilterChromeOSTest, SoftwareSlots) {
   EXPECT_FALSE(no_slots_profile_filter_.IsCertAllowed(cert_2));
 
   EXPECT_TRUE(profile_filter_1_.IsCertAllowed(cert_1));
+  EXPECT_TRUE(profile_filter_1_copy_.IsCertAllowed(cert_1));
   EXPECT_FALSE(profile_filter_1_.IsCertAllowed(cert_2));
+  EXPECT_FALSE(profile_filter_1_copy_.IsCertAllowed(cert_2));
 
   EXPECT_FALSE(profile_filter_2_.IsCertAllowed(cert_1));
   EXPECT_TRUE(profile_filter_2_.IsCertAllowed(cert_2));