Extract ScopedTestNSSDB from nss_util.

Before ScopedTestNSSDB affected several slot getters from nss_util.h . This change reduces ScopedTestNSSDB to solely setup a temporary test DB and not influencing the global state in nss_util anymore. As a replacement for some of its old behavior, a new ScopedTestSystemNSSKeySlot is added, which allows to override the slot returned by GetSystemNSSKeySlot(). With this change it's now possible to write tests that need both a user and system NSS DB by using ScopedTestSystemNSSKeySlot. As a side-effect, GetPersistentNSSKeySlot() is now compiled on !OS_CHROMEOS only. BUG=210525 (For include changes:) R=rsleevi@chromium.org TBR=nkostylev@chromium.org, stevenjb@chromium.org Review URL: https://codereview.chromium.org/401623006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285881 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Jul 28, 2014 · 190933f · 190933f
1 parent a017c66
commit 190933f
Show file tree

Hide file tree

Showing 32 changed files with 424 additions and 208 deletions.
diff --git a/chrome/browser/chromeos/login/auth/parallel_authenticator_unittest.cc b/chrome/browser/chromeos/login/auth/parallel_authenticator_unittest.cc
@@ -37,8 +37,8 @@
 #include "chromeos/login/auth/user_context.h"
 #include "components/user_manager/user.h"
 #include "content/public/test/test_browser_thread_bundle.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "google_apis/gaia/mock_url_fetcher_factory.h"
 #include "net/base/net_errors.h"
 #include "net/url_request/url_request_status.h"

diff --git a/chrome/browser/chromeos/net/cert_verify_proc_chromeos_unittest.cc b/chrome/browser/chromeos/net/cert_verify_proc_chromeos_unittest.cc
@@ -4,8 +4,8 @@
 
 #include "chrome/browser/chromeos/net/cert_verify_proc_chromeos.h"
 
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/cert_verify_proc.h"

diff --git a/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc b/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
@@ -13,8 +13,8 @@
 #include "chrome/browser/chromeos/net/cert_verify_proc_chromeos.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "net/base/net_log.h"
 #include "net/base/test_completion_callback.h"
 #include "net/base/test_data_directory.h"

diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
@@ -1835,8 +1835,8 @@
       'browser/certificate_manager_model.cc',
       'browser/certificate_manager_model.h',
       'browser/net/nss_context.cc',
-      'browser/net/nss_context_chromeos.cc',
       'browser/net/nss_context.h',
+      'browser/net/nss_context_chromeos.cc',
       'browser/net/nss_context_linux.cc',
       'third_party/mozilla_security_manager/nsNSSCertHelper.cpp',
       'third_party/mozilla_security_manager/nsNSSCertHelper.h',
@@ -3234,6 +3234,13 @@
         }],
         ['use_nss==1', {
           'sources': [ '<@(chrome_browser_nss_sources)' ],
+          'conditions': [
+            ['chromeos==1', {
+               'sources!': [ 'browser/net/nss_context_linux.cc' ],
+              }, { # chromeos==0
+               'sources!': [ 'browser/net/nss_context_chromeos.cc' ],
+              }],
+          ],
         }],
         ['notifications==1', {
           'sources': [ '<@(chrome_browser_notifications_sources)' ],

diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
@@ -796,6 +796,7 @@
         '../components/components.gyp:translate_core_common',
         '../components/components_resources.gyp:components_resources',
         '../components/components_strings.gyp:components_strings',
+        '../crypto/crypto.gyp:crypto_test_support',
         '../device/bluetooth/bluetooth.gyp:device_bluetooth_mocks',
         '../device/serial/serial.gyp:device_serial_test_util',
         '../extensions/common/api/api.gyp:extensions_api',

diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
@@ -490,6 +490,7 @@
         '../components/components_resources.gyp:components_resources',
         '../content/content_shell_and_tests.gyp:test_support_content',
         '../content/content.gyp:content_app_both',
+        '../crypto/crypto.gyp:crypto_test_support',
         '../net/net.gyp:net',
         '../net/net.gyp:net_test_support',
         '../sync/sync.gyp:test_support_sync_api',

diff --git a/chrome/common/net/x509_certificate_model_unittest.cc b/chrome/common/net/x509_certificate_model_unittest.cc
@@ -11,7 +11,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(USE_NSS)
-#include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_db.h"
 #include "net/cert/nss_cert_database.h"
 #endif
 
@@ -224,12 +224,11 @@ TEST(X509CertificateModelTest, GetTypeCA) {
   EXPECT_EQ(net::CA_CERT,
             x509_certificate_model::GetType(cert->os_cert_handle()));
 
-  // Additional parantheses required to disambiguate from function declaration.
-  net::NSSCertDatabase db(
-      (crypto::ScopedPK11Slot(
-          crypto::GetPersistentNSSKeySlot())) /* public slot */,
-      crypto::ScopedPK11Slot(
-          crypto::GetPersistentNSSKeySlot()) /* private lot */);
+  crypto::ScopedTestNSSDB test_nssdb;
+  net::NSSCertDatabase db(crypto::ScopedPK11Slot(PK11_ReferenceSlot(
+                              test_nssdb.slot())) /* public slot */,
+                          crypto::ScopedPK11Slot(PK11_ReferenceSlot(
+                              test_nssdb.slot())) /* private slot */);
 
   // Test that explicitly distrusted CA certs are still returned as CA_CERT
   // type. See http://crbug.com/96654.
@@ -259,12 +258,11 @@ TEST(X509CertificateModelTest, GetTypeServer) {
   EXPECT_EQ(net::OTHER_CERT,
             x509_certificate_model::GetType(cert->os_cert_handle()));
 
-  // Additional parantheses required to disambiguate from function declaration.
-  net::NSSCertDatabase db(
-      (crypto::ScopedPK11Slot(
-          crypto::GetPersistentNSSKeySlot())) /* public slot */,
-      crypto::ScopedPK11Slot(
-          crypto::GetPersistentNSSKeySlot()) /* private lot */);
+  crypto::ScopedTestNSSDB test_nssdb;
+  net::NSSCertDatabase db(crypto::ScopedPK11Slot(PK11_ReferenceSlot(
+                              test_nssdb.slot())) /* public slot */,
+                          crypto::ScopedPK11Slot(PK11_ReferenceSlot(
+                              test_nssdb.slot())) /* private slot */);
 
   // Test GetCertType with server certs and explicit trust.
   EXPECT_TRUE(db.SetCertTrust(

diff --git a/chromeos/cert_loader_unittest.cc b/chromeos/cert_loader_unittest.cc
@@ -9,9 +9,9 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
 #include "crypto/scoped_nss_types.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/nss_cert_database_chromeos.h"

diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp
@@ -511,6 +511,7 @@
         '../build/linux/system.gyp:ssl',
         '../components/components.gyp:onc_component',
         '../crypto/crypto.gyp:crypto',
+        '../crypto/crypto.gyp:crypto_test_support',
         '../dbus/dbus.gyp:dbus_test_support',
         '../net/net.gyp:net',
         '../net/net.gyp:net_test_support',

diff --git a/chromeos/network/client_cert_resolver_unittest.cc b/chromeos/network/client_cert_resolver_unittest.cc
@@ -23,8 +23,8 @@
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/tpm_token_loader.h"
 #include "components/onc/onc_constants.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "net/base/crypto_module.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"

diff --git a/chromeos/network/network_cert_migrator_unittest.cc b/chromeos/network/network_cert_migrator_unittest.cc
@@ -14,8 +14,8 @@
 #include "chromeos/dbus/shill_service_client.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/tpm_token_loader.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "net/base/crypto_module.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"

diff --git a/chromeos/network/network_connection_handler_unittest.cc b/chromeos/network/network_connection_handler_unittest.cc
@@ -25,8 +25,8 @@
 #include "chromeos/network/onc/onc_utils.h"
 #include "chromeos/tpm_token_loader.h"
 #include "components/onc/onc_constants.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/nss_cert_database_chromeos.h"

diff --git a/chromeos/network/onc/onc_certificate_importer_impl_unittest.cc b/chromeos/network/onc/onc_certificate_importer_impl_unittest.cc
@@ -16,8 +16,8 @@
 #include "base/values.h"
 #include "chromeos/network/onc/onc_test_utils.h"
 #include "components/onc/onc_constants.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
+#include "crypto/scoped_test_nss_chromeos_user.h"
 #include "net/base/crypto_module.h"
 #include "net/cert/cert_type.h"
 #include "net/cert/nss_cert_database_chromeos.h"

diff --git a/crypto/BUILD.gn b/crypto/BUILD.gn
@@ -131,6 +131,7 @@ component("crypto") {
       "hmac_nss.cc",
       "nss_util.cc",
       "nss_util.h",
+      "nss_util_internal.h",
       "rsa_private_key_nss.cc",
       "secure_hash_default.cc",
       "signature_creator_nss.cc",
@@ -229,6 +230,7 @@ test("crypto_unittests") {
   deps = [
     ":crypto",
     ":platform",
+    ":test_support",
     "//base",
     "//base/test:run_all_unittests",
     "//base/test:test_support",
@@ -237,6 +239,38 @@ test("crypto_unittests") {
   ]
 }
 
+source_set("test_support") {
+  sources = [
+    "scoped_test_nss_db.cc",
+    "scoped_test_nss_db.h",
+    "scoped_test_nss_chromeos_user.cc",
+    "scoped_test_nss_chromeos_user.h",
+    "scoped_test_system_nss_key_slot.cc",
+    "scoped_test_system_nss_key_slot.h",
+  ]
+  deps = [
+    ":crypto",
+    ":platform",
+    "//base",
+  ]
+
+  if (!use_nss_certs) {
+    sources -= [
+      "scoped_test_nss_db.cc",
+      "scoped_test_nss_db.h",
+    ]
+  }
+
+  if (!is_chromeos) {
+    sources -= [
+      "scoped_test_nss_chromeos_user.cc",
+      "scoped_test_nss_chromeos_user.h",
+      "scoped_test_system_nss_key_slot.cc",
+      "scoped_test_system_nss_key_slot.h",
+    ]
+  }
+}
+
 # This is a meta-target that forwards to NSS's SSL library or OpenSSL,
 # according to the state of the crypto flags. A target just wanting to depend
 # on the current SSL library should just depend on this.

diff --git a/crypto/crypto.gyp b/crypto/crypto.gyp
@@ -111,6 +111,7 @@
               'hmac_nss.cc',
               'nss_util.cc',
               'nss_util.h',
+              'nss_util_internal.h',
               'rsa_private_key_nss.cc',
               'secure_hash_default.cc',
               'signature_creator_nss.cc',
@@ -174,6 +175,7 @@
       ],
       'dependencies': [
         'crypto',
+        'crypto_test_support',
         '../base/base.gyp:base',
         '../base/base.gyp:run_all_unittests',
         '../base/base.gyp:test_support_base',
@@ -254,5 +256,47 @@
         },
       ],
     }],
+    ['use_nss==1', {
+      'targets': [
+        {
+          'target_name': 'crypto_test_support',
+          'type': 'static_library',
+          'dependencies': [
+            '../base/base.gyp:base',
+            'crypto',
+          ],
+          'sources': [
+            'scoped_test_nss_db.cc',
+            'scoped_test_nss_db.h',
+            'scoped_test_nss_chromeos_user.cc',
+            'scoped_test_nss_chromeos_user.h',
+            'scoped_test_system_nss_key_slot.cc',
+            'scoped_test_system_nss_key_slot.h',
+          ],
+          'conditions': [
+            ['use_nss==0', {
+              'sources!': [
+                'scoped_test_nss_db.cc',
+                'scoped_test_nss_db.h',
+              ],
+            }],
+            [ 'chromeos==0', {
+              'sources!': [
+                'scoped_test_nss_chromeos_user.cc',
+                'scoped_test_nss_chromeos_user.h',
+                'scoped_test_system_nss_key_slot.cc',
+                'scoped_test_system_nss_key_slot.h',
+              ],
+            }],
+          ],
+        }
+      ]}, {  # use_nss==0
+      'targets': [
+        {
+          'target_name': 'crypto_test_support',
+          'type': 'none',
+          'sources': [],
+        }
+    ]}],
   ],
 }