Skip to content

Commit

Permalink
TestCertVerifierServiceFactoryImpl: allow usage in ChromeOS tests
Browse files Browse the repository at this point in the history
On ChromeOS the real CertVerifierServiceFactoryImpl must be called on
the IO thread. Move TestCertVerifierServiceFactoryImpl into
content/public/test so that it can know about browser threads and change
the test helper to run the delegate on the IO thread for Chrome OS.

Bug: 1340420
Change-Id: I72eca31e4a232bfa3be10876e14a5405e65b0863
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3817202
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034101}
  • Loading branch information
matt-mueller authored and Chromium LUCI CQ committed Aug 11, 2022
1 parent 821ba91 commit b816ce5
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 25 deletions.
Expand Up @@ -57,6 +57,7 @@
#include "content/public/common/content_features.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/simple_url_loader_test_helper.h"
#include "content/public/test/test_cert_verifier_service_factory.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/system/data_pipe_utils.h"
#include "net/base/features.h"
Expand All @@ -70,7 +71,6 @@
#include "net/test/embedded_test_server/request_handler_util.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "services/cert_verifier/public/mojom/cert_verifier_service_factory.mojom.h"
#include "services/cert_verifier/test_cert_verifier_service_factory.h"
#include "services/network/public/cpp/cors/cors.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/mojom/network_context.mojom.h"
Expand Down
Expand Up @@ -35,11 +35,11 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/frame_test_utils.h"
#include "content/public/test/test_cert_verifier_service_factory.h"
#include "content/public/test/test_utils.h"
#include "net/cookies/canonical_cookie_test_helpers.h"
#include "net/dns/mock_host_resolver.h"
#include "net/net_buildflags.h"
#include "services/cert_verifier/test_cert_verifier_service_factory.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/network_service_buildflags.h"
#include "services/network/public/mojom/network_context.mojom.h"
Expand Down
1 change: 0 additions & 1 deletion chrome/test/BUILD.gn
Expand Up @@ -299,7 +299,6 @@ static_library("test_support") {
"//net:test_support",
"//ppapi/buildflags",
"//printing/buildflags",
"//services/cert_verifier:test_support",
"//services/data_decoder/public/cpp:test_support",
"//services/device/public/cpp:test_support",
"//services/device/public/cpp/geolocation",
Expand Down
1 change: 1 addition & 0 deletions content/public/test/DEPS
Expand Up @@ -31,6 +31,7 @@ include_rules = [
"+components/viz/test",
"+device/vr/public/mojom",
"+services/audio",
"+services/cert_verifier",
"+services/metrics/public/cpp",
"+services/network",
"+services/service_manager",
Expand Down
3 changes: 3 additions & 0 deletions content/public/test/OWNERS
Expand Up @@ -22,6 +22,9 @@ per-file test_aggregation_service*=file://content/browser/aggregation_service/OW
# Network Service
per-file network_service_test_helper.*=file://services/network/OWNERS

# Cert Verifier Service
per-file test_cert_verifier_service_factory.*=file://services/cert_verifier/OWNERS

# DevTools-specific changes
per-file *devtools*=file://content/browser/devtools/OWNERS

Expand Down
Expand Up @@ -2,12 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "services/cert_verifier/test_cert_verifier_service_factory.h"
#include "content/public/test/test_cert_verifier_service_factory.h"

#include <memory>
#include <type_traits>

#include "base/memory/scoped_refptr.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "net/net_buildflags.h"
#include "services/cert_verifier/cert_verifier_service.h"
#include "services/cert_verifier/cert_verifier_service_factory.h"
Expand Down Expand Up @@ -72,8 +74,37 @@ void TestCertVerifierServiceFactoryImpl::ReleaseNextCertVerifierParams() {
}

void TestCertVerifierServiceFactoryImpl::InitDelegate() {
delegate_ = std::make_unique<CertVerifierServiceFactoryImpl>(
delegate_remote_.BindNewPipeAndPassReceiver());
delegate_ = base::MakeRefCounted<DelegateOwner>(
#if BUILDFLAG(IS_CHROMEOS)
// In-process CertVerifierService in Ash and Lacros should run on the IO
// thread because it interacts with IO-bound NSS and ChromeOS user slots.
// See for example InitializeNSSForChromeOSUser() or
// CertDbInitializerIOImpl.
content::GetIOThreadTaskRunner({})
#else
base::SequencedTaskRunnerHandle::Get()
#endif
);
delegate_->Init(delegate_remote_.BindNewPipeAndPassReceiver());
}

TestCertVerifierServiceFactoryImpl::DelegateOwner::DelegateOwner(
scoped_refptr<base::SequencedTaskRunner> task_runner)
: base::RefCountedDeleteOnSequence<DelegateOwner>(std::move(task_runner)) {}

TestCertVerifierServiceFactoryImpl::DelegateOwner::~DelegateOwner() = default;

void TestCertVerifierServiceFactoryImpl::DelegateOwner::Init(
mojo::PendingReceiver<cert_verifier::mojom::CertVerifierServiceFactory>
receiver) {
if (!owning_task_runner()->RunsTasksInCurrentSequence()) {
owning_task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&DelegateOwner::Init, this, std::move(receiver)));
return;
}
delegate_ =
std::make_unique<CertVerifierServiceFactoryImpl>(std::move(receiver));
}

} // namespace cert_verifier
Expand Up @@ -2,12 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef SERVICES_CERT_VERIFIER_TEST_CERT_VERIFIER_SERVICE_FACTORY_H_
#define SERVICES_CERT_VERIFIER_TEST_CERT_VERIFIER_SERVICE_FACTORY_H_
#ifndef CONTENT_PUBLIC_TEST_TEST_CERT_VERIFIER_SERVICE_FACTORY_H_
#define CONTENT_PUBLIC_TEST_TEST_CERT_VERIFIER_SERVICE_FACTORY_H_

#include <memory>

#include "base/containers/circular_deque.h"
#include "base/memory/ref_counted.h"
#include "base/memory/ref_counted_delete_on_sequence.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
Expand Down Expand Up @@ -63,14 +65,32 @@ class TestCertVerifierServiceFactoryImpl
}

private:
class DelegateOwner : public base::RefCountedDeleteOnSequence<DelegateOwner> {
public:
explicit DelegateOwner(
scoped_refptr<base::SequencedTaskRunner> owning_task_runner);

void Init(
mojo::PendingReceiver<cert_verifier::mojom::CertVerifierServiceFactory>
receiver);

private:
friend class base::RefCountedDeleteOnSequence<DelegateOwner>;
friend class base::DeleteHelper<DelegateOwner>;

~DelegateOwner();

std::unique_ptr<CertVerifierServiceFactoryImpl> delegate_;
};

void InitDelegate();

mojo::Remote<mojom::CertVerifierServiceFactory> delegate_remote_;
std::unique_ptr<CertVerifierServiceFactoryImpl> delegate_;
scoped_refptr<DelegateOwner> delegate_;

base::circular_deque<GetNewCertVerifierParams> captured_params_;
};

} // namespace cert_verifier

#endif // SERVICES_CERT_VERIFIER_TEST_CERT_VERIFIER_SERVICE_FACTORY_H_
#endif // CONTENT_PUBLIC_TEST_TEST_CERT_VERIFIER_SERVICE_FACTORY_H_
3 changes: 3 additions & 0 deletions content/test/BUILD.gn
Expand Up @@ -290,6 +290,8 @@ static_library("test_support") {
"../public/test/test_aggregation_service.h",
"../public/test/test_browser_context.cc",
"../public/test/test_browser_context.h",
"../public/test/test_cert_verifier_service_factory.cc",
"../public/test/test_cert_verifier_service_factory.h",
"../public/test/test_content_client_initializer.cc",
"../public/test/test_content_client_initializer.h",
"../public/test/test_devtools_protocol_client.cc",
Expand Down Expand Up @@ -539,6 +541,7 @@ static_library("test_support") {
"//net:test_support",
"//services/audio",
"//services/audio/public/mojom",
"//services/cert_verifier:lib",
"//services/data_decoder/public/cpp:test_support",
"//services/data_decoder/public/mojom",
"//services/device/public/mojom",
Expand Down
15 changes: 0 additions & 15 deletions services/cert_verifier/BUILD.gn
Expand Up @@ -70,18 +70,3 @@ source_set("tests") {
"//testing/gtest",
]
}

source_set("test_support") {
testonly = true

sources = [
"test_cert_verifier_service_factory.cc",
"test_cert_verifier_service_factory.h",
]

deps = [
":lib",
"//base",
"//net",
]
}

0 comments on commit b816ce5

Please sign in to comment.