ScopedTestSystemNSSKeySlotMixin: Preserve slot state after pre-tests

Before this CL, ScopedTestSystemNSSKeySlotMixin used ScopedTestNSSDB
which used a temp. directory to save the database data. This temp.
directory will be deleted after browser pre-tests (tests that start with
PRE_).

This CL changes ScopedTestSystemNSSKeySlotMixin such that it opens a
software slot in the user data directory which is not deleted after
browser pre-tests so to allow tests using system slot to simulate
browser restarts.

Bug: 1136437, 1127284, 1113115
Change-Id: Ie61a6a7f594b996236b709e2db765a240e7b8800
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2460730Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Omar Morsi <omorsi@google.com>
Cr-Commit-Position: refs/heads/master@{#817994}

chrome/browser/chromeos/scoped_test_system_nss_key_slot_mixin.cc

@@ -4,25 +4,44 @@
 
 #include "chrome/browser/chromeos/scoped_test_system_nss_key_slot_mixin.h"
 
+#include <memory>
+
 #include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/logging.h"
+#include "base/path_service.h"
 #include "base/run_loop.h"
+#include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "crypto/nss_util.h"
+#include "crypto/nss_util_internal.h"
+#include "crypto/scoped_nss_types.h"
 #include "crypto/scoped_test_system_nss_key_slot.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
 
+namespace {
+
+// Returns a subdirectory under the user data directory (which is not cleared
+// after pre-tests).
+base::FilePath GetNssDbTestDir() {
+  base::FilePath nss_db_subdir;
+  base::PathService::Get(chrome::DIR_USER_DATA, &nss_db_subdir);
+  nss_db_subdir = nss_db_subdir.AppendASCII("nss_db_subdir");
+  return nss_db_subdir;
+}
+
+}  // namespace
+
 ScopedTestSystemNSSKeySlotMixin::ScopedTestSystemNSSKeySlotMixin(
     InProcessBrowserTestMixinHost* host)
     : InProcessBrowserTestMixin(host) {}
 
 ScopedTestSystemNSSKeySlotMixin::~ScopedTestSystemNSSKeySlotMixin() = default;
 
-PK11SlotInfo* ScopedTestSystemNSSKeySlotMixin::slot() {
-  return scoped_test_system_nss_key_slot_->slot();
-}
-
 void ScopedTestSystemNSSKeySlotMixin::SetUpOnMainThread() {
   bool system_slot_initialized_successfully = false;
   base::RunLoop loop;
@@ -48,14 +67,36 @@ void ScopedTestSystemNSSKeySlotMixin::TearDownOnMainThread() {
 
 void ScopedTestSystemNSSKeySlotMixin::InitializeOnIo(bool* out_success) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  scoped_test_system_nss_key_slot_ =
-      std::make_unique<crypto::ScopedTestSystemNSSKeySlot>();
-  *out_success = scoped_test_system_nss_key_slot_->ConstructedSuccessfully();
+
+  crypto::EnsureNSSInit();
+  // NSS is allowed to do IO on the current thread since dispatching
+  // to a dedicated thread would still have the affect of blocking
+  // the current thread, due to NSS's internal locking requirements
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  base::FilePath nss_db_subdir = GetNssDbTestDir();
+  ASSERT_TRUE(base::CreateDirectory(nss_db_subdir));
+
+  const char kTestDescription[] = "Test DB";
+  slot_ = crypto::OpenSoftwareNSSDB(nss_db_subdir, kTestDescription);
+  *out_success = !!slot_;
+
+  if (slot_) {
+    crypto::SetSystemKeySlotForTesting(
+        crypto::ScopedPK11Slot(PK11_ReferenceSlot(slot_.get())));
+  }
 }
 
 void ScopedTestSystemNSSKeySlotMixin::DestroyOnIo() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  scoped_test_system_nss_key_slot_.reset();
+
+  crypto::SetSystemKeySlotForTesting(nullptr);
+
+  if (slot_) {
+    SECStatus status = SECMOD_CloseUserDB(slot_.get());
+    if (status != SECSuccess)
+      PLOG(ERROR) << "SECMOD_CloseUserDB failed: " << PORT_GetError();
+  }
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/scoped_test_system_nss_key_slot_mixin.h

@@ -6,18 +6,20 @@
 #define CHROME_BROWSER_CHROMEOS_SCOPED_TEST_SYSTEM_NSS_KEY_SLOT_MIXIN_H_
 
 #include <pk11pub.h>
+
 #include <memory>
 
 #include "chrome/test/base/mixin_based_in_process_browser_test.h"
-
-namespace crypto {
-class ScopedTestSystemNSSKeySlot;
-}
+#include "crypto/scoped_nss_types.h"
 
 namespace chromeos {
 
-// Owns a persistent NSS software database in a temporary directory and the
+// Owns a persistent NSS software database in the user directory and the
 // association of the system slot with this database.
+// Note: The database is persisted in the user data directory
+// (chrome::DIR_USER_DATA) so it persists between PRE_ and non-PRE_ tests. This
+// allows simulating browser restarts after doing some operations on the
+// database without losing its state.
 //
 // This mixin performs the blocking initialization/destruction in the
 // {SetUp|TearDown}OnMainThread methods.
@@ -30,14 +32,7 @@ class ScopedTestSystemNSSKeySlotMixin final : public InProcessBrowserTestMixin {
       const ScopedTestSystemNSSKeySlotMixin&) = delete;
   ~ScopedTestSystemNSSKeySlotMixin() override;
 
-  crypto::ScopedTestSystemNSSKeySlot* scoped_test_system_nss_key_slot() {
-    return scoped_test_system_nss_key_slot_.get();
-  }
-  const crypto::ScopedTestSystemNSSKeySlot* scoped_test_system_nss_key_slot()
-      const {
-    return scoped_test_system_nss_key_slot_.get();
-  }
-  PK11SlotInfo* slot();
+  PK11SlotInfo* slot() { return slot_.get(); }
 
   void SetUpOnMainThread() override;
   void TearDownOnMainThread() override;
@@ -46,8 +41,7 @@ class ScopedTestSystemNSSKeySlotMixin final : public InProcessBrowserTestMixin {
   void InitializeOnIo(bool* out_success);
   void DestroyOnIo();
 
-  std::unique_ptr<crypto::ScopedTestSystemNSSKeySlot>
-      scoped_test_system_nss_key_slot_;
+  crypto::ScopedPK11Slot slot_;
 };
 
 }  // namespace chromeos