Commit 88d6bfbe authored by Rakina Zata Amni's avatar Rakina Zata Amni Committed by Commit Bot

Revert "Fix CertVerifierService initialization in some unittests"

This reverts commit a79726df.

Reason for revert: This is suspected to break some network-related tests

Original change's description:
> Fix CertVerifierService initialization in some unittests
>
> The manual resetting of the CertVerifierServiceFactory in between
> unit_tests seems not to work on Android. So, refactor the
> initialization code to look like the Storage Service's, using
> SequenceLocalStorage to reset the CertVerifierServiceFactory
> between unit tests.
>
> Bug: 1015134, 1094483
> Change-Id: Ic13f67274cecd49450279da401fb4cd29c9b1e94
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2520377
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Matthew Denton <mpdenton@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825141}

TBR=jam@chromium.org,mpdenton@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1015134, 1094483, 1146899
Change-Id: I4903c4c8fa157f0e47cefd32cfb679fe144d050e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2524812
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: default avatarRakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825251}
parent 5f273f29
......@@ -520,22 +520,12 @@ GetNewCertVerifierServiceRemote(
return cert_verifier_remote;
}
void RunInProcessCertVerifierServiceFactory(
void CreateInProcessCertVerifierServiceOnThread(
mojo::PendingReceiver<cert_verifier::mojom::CertVerifierServiceFactory>
receiver) {
#if defined(OS_CHROMEOS)
DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::IO) ||
BrowserThread::CurrentlyOn(BrowserThread::IO));
#else
DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::UI) ||
BrowserThread::CurrentlyOn(BrowserThread::UI));
#endif
static base::NoDestructor<base::SequenceLocalStorageSlot<
std::unique_ptr<cert_verifier::CertVerifierServiceFactoryImpl>>>
service_factory_slot;
service_factory_slot->GetOrCreateValue() =
std::make_unique<cert_verifier::CertVerifierServiceFactoryImpl>(
std::move(receiver));
// Except in tests, our CertVerifierServiceFactoryImpl is a singleton.
static base::NoDestructor<cert_verifier::CertVerifierServiceFactoryImpl>
cv_service_factory(std::move(receiver));
}
// Owns the CertVerifierServiceFactory used by the browser.
......@@ -543,19 +533,16 @@ void RunInProcessCertVerifierServiceFactory(
class CertVerifierServiceFactoryOwner {
public:
CertVerifierServiceFactoryOwner() = default;
CertVerifierServiceFactoryOwner(const CertVerifierServiceFactoryOwner&) =
delete;
CertVerifierServiceFactoryOwner& operator=(
const CertVerifierServiceFactoryOwner&) = delete;
~CertVerifierServiceFactoryOwner() = default;
~CertVerifierServiceFactoryOwner() = delete;
static CertVerifierServiceFactoryOwner* Get() {
static base::NoDestructor<
base::SequenceLocalStorageSlot<CertVerifierServiceFactoryOwner>>
static base::NoDestructor<CertVerifierServiceFactoryOwner>
cert_verifier_service_factory_owner;
return &cert_verifier_service_factory_owner->GetOrCreateValue();
return &*cert_verifier_service_factory_owner;
}
// Passing nullptr will reset the current remote.
......@@ -578,10 +565,10 @@ class CertVerifierServiceFactoryOwner {
// See for example InitializeNSSForChromeOSUser().
GetIOThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(&RunInProcessCertVerifierServiceFactory,
base::BindOnce(&CreateInProcessCertVerifierServiceOnThread,
service_factory_remote_.BindNewPipeAndPassReceiver()));
#else
RunInProcessCertVerifierServiceFactory(
CreateInProcessCertVerifierServiceOnThread(
service_factory_remote_.BindNewPipeAndPassReceiver());
#endif
service_factory_ = service_factory_remote_.get();
......
......@@ -56,6 +56,21 @@ class ResetNetworkServiceBetweenTests : public testing::EmptyTestEventListener {
DISALLOW_COPY_AND_ASSIGN(ResetNetworkServiceBetweenTests);
};
// Similarly to the above, the global CertVerifierServiceFactory object needs
// to be destructed in between tests.
class ResetCertVerifierServiceFactoryBetweenTests
: public testing::EmptyTestEventListener {
public:
ResetCertVerifierServiceFactoryBetweenTests() = default;
void OnTestEnd(const testing::TestInfo& test_info) override {
SetCertVerifierServiceFactoryForTesting(nullptr);
}
private:
DISALLOW_COPY_AND_ASSIGN(ResetCertVerifierServiceFactoryBetweenTests);
};
} // namespace
UnitTestTestSuite::UnitTestTestSuite(base::TestSuite* test_suite)
......@@ -72,6 +87,7 @@ UnitTestTestSuite::UnitTestTestSuite(base::TestSuite* test_suite)
testing::TestEventListeners& listeners =
testing::UnitTest::GetInstance()->listeners();
listeners.Append(new ResetNetworkServiceBetweenTests);
listeners.Append(new ResetCertVerifierServiceFactoryBetweenTests);
// The ThreadPool created by the test launcher is never destroyed.
// Similarly, the FeatureList created here is never destroyed so it
......
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