Skip to content

GitLab

T
tangled
Commit cffec8fb authored by tbarzic@chromium.org's avatar tbarzic@chromium.org
Browse files
Options

Cleanup tests using SetSkipCertificateImporterCreationForTest 

UserNetworkConfigurationUpdater::SetSkipCertificateImporterCreationForTest
is not needed anymore, so it's safe to remove it and cleanup tests
using it.

BUG=None

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283868 0039d316-1c4b-4281-b951-d872f2087c98
parent 19de5712
Hide whitespace changes
Inline Side-by-side
Showing with 4 additions and 159 deletions
chrome/browser/chromeos/policy/user_network_configuration_updater.cc
View file @ cffec8fb
......@@ -19,12 +19,6 @@
#include "net/cert/x509_certificate.h"
#include "policy/policy_constants.h"
namespace {
bool skip_certificate_importer_creation_for_test = false;
} // namespace
namespace policy {
UserNetworkConfigurationUpdater::~UserNetworkConfigurationUpdater() {}
......@@ -74,12 +68,10 @@ UserNetworkConfigurationUpdater::UserNetworkConfigurationUpdater(
// responsible for creating it. This requires |GetNSSCertDatabaseForProfile|
// call, which is not safe before the profile initialization is finalized.
// Thus, listen for PROFILE_ADDED notification, on which |cert_importer_|
// creation should start. This behaviour can be disabled in tests.
if (!skip_certificate_importer_creation_for_test) {
registrar_.Add(this,
chrome::NOTIFICATION_PROFILE_ADDED,
content::Source<Profile>(profile));
}
// creation should start.
registrar_.Add(this,
chrome::NOTIFICATION_PROFILE_ADDED,
content::Source<Profile>(profile));
}
void UserNetworkConfigurationUpdater::SetCertificateImporterForTest(
......@@ -87,12 +79,6 @@ void UserNetworkConfigurationUpdater::SetCertificateImporterForTest(
SetCertificateImporter(certificate_importer.Pass());
}
// static
void UserNetworkConfigurationUpdater::
SetSkipCertificateImporterCreationForTest(bool skip) {
skip_certificate_importer_creation_for_test = skip;
}
void UserNetworkConfigurationUpdater::GetWebTrustedCertificates(
net::CertificateList* certs) const {
*certs = web_trust_certs_;
......@@ -135,9 +121,6 @@ void UserNetworkConfigurationUpdater::Observe(
DCHECK_EQ(type, chrome::NOTIFICATION_PROFILE_ADDED);
Profile* profile = content::Source<Profile>(source).ptr();
if (skip_certificate_importer_creation_for_test)
return;
GetNSSCertDatabaseForProfile(
profile,
base::Bind(
......
chrome/browser/chromeos/policy/user_network_configuration_updater.h
View file @ cffec8fb
......@@ -82,12 +82,6 @@ class UserNetworkConfigurationUpdater : public NetworkConfigurationUpdater,
void SetCertificateImporterForTest(
scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer);
// Used in test to delay CertificateImporter creation until the NSSDatabase is
// ready. This is needed in some tests as the user's certificate database may
// not get initialized in time.
// TODO(tbarzic): Remove this when it's not needed.
static void SetSkipCertificateImporterCreationForTest(bool skip);
private:
class CrosTrustAnchorProvider;
......
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc
View file @ cffec8fb
......@@ -7,7 +7,6 @@
#include "base/macros.h"
#include "base/strings/stringprintf.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/policy/user_network_configuration_updater.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/net/nss_context.h"
......@@ -18,9 +17,7 @@
#include "content/public/browser/notification_service.h"
#include "content/public/common/content_switches.h"
#include "content/public/test/test_utils.h"
#include "content/public/test/test_utils.h"
#include "content/test/net/url_request_mock_http_job.h"
#include "crypto/nss_util.h"
#include "net/base/net_errors.h"
#include "net/cert/nss_cert_database.h"
#include "policy/policy_constants.h"
......@@ -101,12 +98,6 @@ void DidGetCertDatabase(base::RunLoop* loop, net::NSSCertDatabase* cert_db) {
class EnterprisePlatformKeysTest : public ExtensionApiTest {
public:
virtual void SetUp() {
policy::UserNetworkConfigurationUpdater::
SetSkipCertificateImporterCreationForTest(true /*skip*/);
ExtensionApiTest::SetUp();
}
virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
ExtensionApiTest::SetUpCommandLine(command_line);
......@@ -135,16 +126,6 @@ class EnterprisePlatformKeysTest : public ExtensionApiTest {
FROM_HERE,
base::Bind(chrome_browser_net::SetUrlRequestMocksEnabled, true));
{
base::RunLoop loop;
content::BrowserThread::PostTask(
content::BrowserThread::IO,
FROM_HERE,
base::Bind(&EnterprisePlatformKeysTest::SetupTestNSSDBOnIOThread,
base::Unretained(this),
&loop));
loop.Run();
}
{
base::RunLoop loop;
......@@ -156,31 +137,7 @@ class EnterprisePlatformKeysTest : public ExtensionApiTest {
SetPolicy();
}
virtual void CleanUpOnMainThread() OVERRIDE {
base::RunLoop loop;
content::BrowserThread::PostTask(
content::BrowserThread::IO,
FROM_HERE,
base::Bind(&EnterprisePlatformKeysTest::DeleteTestNSSDBOnIOThread,
base::Unretained(this),
&loop));
loop.Run();
ExtensionApiTest::CleanUpOnMainThread();
}
private:
void SetupTestNSSDBOnIOThread(base::RunLoop* loop) {
test_nssdb_.reset(new crypto::ScopedTestNSSDB);
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE, loop->QuitClosure());
}
void DeleteTestNSSDBOnIOThread(base::RunLoop* loop) {
test_nssdb_.reset();
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE, loop->QuitClosure());
}
void SetPolicy() {
// Extensions that are force-installed come from an update URL, which
// defaults to the webstore. Use a mock URL for this test with an update
......@@ -209,7 +166,6 @@ class EnterprisePlatformKeysTest : public ExtensionApiTest {
observer.Wait();
}
scoped_ptr<crypto::ScopedTestNSSDB> test_nssdb_;
policy::MockConfigurationPolicyProvider policy_provider_;
};
......
chrome/browser/ui/webui/options/certificate_manager_browsertest.cc
View file @ cffec8fb
......@@ -2,13 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/callback.h"
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/webui/options/options_ui_browsertest.h"
#include "chrome/common/url_constants.h"
#include "components/policy/core/browser/browser_policy_connector.h"
#include "components/policy/core/common/external_data_fetcher.h"
#include "components/policy/core/common/mock_configuration_policy_provider.h"
......@@ -24,12 +20,7 @@
#if defined(OS_CHROMEOS)
#include "chrome/browser/chromeos/policy/device_policy_cros_browser_test.h"
#include "chrome/browser/chromeos/policy/user_network_configuration_updater.h"
#include "chrome/browser/chromeos/policy/user_network_configuration_updater_factory.h"
#include "chrome/browser/net/nss_context.h"
#include "chromeos/network/onc/onc_certificate_importer_impl.h"
#include "chromeos/network/onc/onc_test_utils.h"
#include "crypto/nss_util.h"
#endif
using testing::Return;
......@@ -41,22 +32,6 @@ class CertificateManagerBrowserTest : public options::OptionsUIBrowserTest {
virtual ~CertificateManagerBrowserTest() {}
protected:
virtual void SetUp() OVERRIDE {
#if defined(OS_CHROMEOS)
policy::UserNetworkConfigurationUpdater::
SetSkipCertificateImporterCreationForTest(true);
#endif
options::OptionsUIBrowserTest::SetUp();
}
virtual void TearDown() OVERRIDE {
#if defined(OS_CHROMEOS)
policy::UserNetworkConfigurationUpdater::
SetSkipCertificateImporterCreationForTest(false);
#endif
options::OptionsUIBrowserTest::TearDown();
}
virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
#if defined(OS_CHROMEOS)
device_policy_test_helper_.MarkAsEnterpriseOwned();
......@@ -67,69 +42,7 @@ class CertificateManagerBrowserTest : public options::OptionsUIBrowserTest {
policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
}
void SetUpOnIOThread() {
#if defined(OS_CHROMEOS)
test_nssdb_.reset(new crypto::ScopedTestNSSDB());
#endif
}
void TearDownOnIOThread() {
#if defined(OS_CHROMEOS)
test_nssdb_.reset();
#endif
}
virtual void SetUpOnMainThread() OVERRIDE {
content::BrowserThread::PostTask(
content::BrowserThread::IO,
FROM_HERE,
base::Bind(&CertificateManagerBrowserTest::SetUpOnIOThread, this));
content::RunAllPendingInMessageLoop(content::BrowserThread::IO);
content::RunAllPendingInMessageLoop();
#if defined(OS_CHROMEOS)
// UserNetworkConfigurationUpdater's onc::CertificateImporter is usually
// passed the NSSCertDatabase fetched during testing profile
// constrution. Unfortunately, test database gets setup after that, so we
// would end up with |PK11_GetInternalKeySlot|. The cause of this is in
// |crypto::InitializeNSSForChromeOSUser|, which does not open new
// database slot for primary user, but it just uses the singleton one (which
// is not set in tests before |test_nssdb_| is created). To handle this,
// creating certificate importer during the UserNetworkConfiguirationUpdater
// service creation is set to be skipped (see |SetUp|), and cert importer
// is set up here.
// Note that creating |test_nssdb_| sooner (in SetUp) would break thread
// restrictions, which require it to be used on IO thread only.
// TODO(tbarzic): Update InitializeNSSForChromeOSUser not to special case
// the primary user.
GetNSSCertDatabaseForProfile(
browser()->profile(),
base::Bind(
&CertificateManagerBrowserTest::UpdateNetworkConfigurationUpdater,
base::Unretained(this)));
content::RunAllPendingInMessageLoop(content::BrowserThread::IO);
content::RunAllPendingInMessageLoop();
#endif
}
virtual void CleanUpOnMainThread() OVERRIDE {
content::BrowserThread::PostTask(
content::BrowserThread::IO,
FROM_HERE,
base::Bind(&CertificateManagerBrowserTest::TearDownOnIOThread, this));
content::RunAllPendingInMessageLoop(content::BrowserThread::IO);
}
#if defined(OS_CHROMEOS)
void UpdateNetworkConfigurationUpdater(net::NSSCertDatabase* database) {
policy::UserNetworkConfigurationUpdaterFactory::GetForProfile(
browser()->profile())->SetCertificateImporterForTest(
scoped_ptr<chromeos::onc::CertificateImporter>(
new chromeos::onc::CertificateImporterImpl(database)));
}
void LoadONCPolicy(const std::string& filename) {
const std::string& user_policy_blob =
chromeos::onc::test_utils::ReadTestData(filename);
......@@ -163,7 +76,6 @@ class CertificateManagerBrowserTest : public options::OptionsUIBrowserTest {
policy::MockConfigurationPolicyProvider provider_;
#if defined(OS_CHROMEOS)
policy::DevicePolicyCrosTestHelper device_policy_test_helper_;
scoped_ptr<crypto::ScopedTestNSSDB> test_nssdb_;
#endif
};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment