Skip to content

Commit

Permalink
Migrate ash to ServiceBinding
Browse files Browse the repository at this point in the history
Replaces usage of the deprecated ServiceContext with ServiceBinding.

Also adds support for a C++ ServiceMain which is passed a proper
ServiceRequest instead of a raw Mojo handle. This makes writing
ServiceMain slightly less awkward.

Also cleans up service.gni using forward_variables_from, which was
unsupported when it was originally written.

Finally, removes the unused RegisterInProcessServices method from
BrowserProcessPlatformPart/Base.

Bug: 891780
Change-Id: I99734414586b6b3a355fa22afa85930249f8e62b
Reviewed-on: https://chromium-review.googlesource.com/c/1355336
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612503}
  • Loading branch information
krockot authored and Commit Bot committed Nov 30, 2018
1 parent bc585ae commit e116c20
Show file tree
Hide file tree
Showing 23 changed files with 184 additions and 165 deletions.
1 change: 1 addition & 0 deletions ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2307,6 +2307,7 @@ static_library("interactive_ui_test_support") {

service("ash_service") {
output_name = "ash"
use_cpp_main = true

sources = [
"main.cc",
Expand Down
26 changes: 8 additions & 18 deletions ash/ash_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@
namespace ash {
namespace {

std::unique_ptr<service_manager::Service> CreateAshService() {
return std::make_unique<AshService>();
}

class AshViewsDelegate : public views::ViewsDelegate {
public:
AshViewsDelegate() = default;
Expand All @@ -71,7 +67,8 @@ class AshViewsDelegate : public views::ViewsDelegate {

} // namespace

AshService::AshService() = default;
AshService::AshService(service_manager::mojom::ServiceRequest request)
: service_binding_(this, std::move(request)) {}

AshService::~AshService() {
if (!base::FeatureList::IsEnabled(features::kMash))
Expand Down Expand Up @@ -104,36 +101,29 @@ AshService::~AshService() {
gpu_host_.reset();
}

// static
service_manager::EmbeddedServiceInfo AshService::CreateEmbeddedServiceInfo() {
service_manager::EmbeddedServiceInfo info;
info.factory = base::BindRepeating(&CreateAshService);
info.task_runner = base::ThreadTaskRunnerHandle::Get();
return info;
}

void AshService::InitForMash() {
wm_state_ = std::make_unique<::wm::WMState>();

discardable_shared_memory_manager_ =
std::make_unique<discardable_memory::DiscardableSharedMemoryManager>();

gpu_host_ = std::make_unique<ws::gpu_host::GpuHost>(
this, context()->connector(), discardable_shared_memory_manager_.get());
this, service_binding_.GetConnector(),
discardable_shared_memory_manager_.get());

host_frame_sink_manager_ = std::make_unique<viz::HostFrameSinkManager>();
CreateFrameSinkManager();
io_thread_ = std::make_unique<base::Thread>("IOThread");
base::Thread::Options thread_options(base::MessageLoop::TYPE_IO, 0);
thread_options.priority = base::ThreadPriority::NORMAL;
CHECK(io_thread_->StartWithOptions(thread_options));
gpu_ = ws::Gpu::Create(context()->connector(), ws::mojom::kServiceName,
io_thread_->task_runner());
gpu_ = ws::Gpu::Create(service_binding_.GetConnector(),
ws::mojom::kServiceName, io_thread_->task_runner());

context_factory_ = std::make_unique<ws::HostContextFactory>(
gpu_.get(), host_frame_sink_manager_.get());

env_ = aura::Env::CreateInstanceToHostViz(context()->connector());
env_ = aura::Env::CreateInstanceToHostViz(service_binding_.GetConnector());

views_delegate_ = std::make_unique<AshViewsDelegate>();

Expand Down Expand Up @@ -173,7 +163,7 @@ void AshService::InitForMash() {
shell_init_params.context_factory = context_factory_.get();
shell_init_params.context_factory_private =
context_factory_->GetContextFactoryPrivate();
shell_init_params.connector = context()->connector();
shell_init_params.connector = service_binding_.GetConnector();
shell_init_params.gpu_interface_provider =
std::make_unique<AshGpuInterfaceProvider>(
gpu_host_.get(), discardable_shared_memory_manager_.get());
Expand Down
12 changes: 4 additions & 8 deletions ash/ash_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "mojo/public/cpp/bindings/binding_set.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/cpp/service_binding.h"
#include "services/service_manager/public/mojom/service.mojom.h"
#include "services/service_manager/public/mojom/service_factory.mojom.h"
#include "services/ws/gpu_host/gpu_host_delegate.h"
#include "services/ws/public/mojom/gpu.mojom.h"
Expand All @@ -32,10 +34,6 @@ namespace discardable_memory {
class DiscardableSharedMemoryManager;
}

namespace service_manager {
struct EmbeddedServiceInfo;
}

namespace views {
class ViewsDelegate;
}
Expand Down Expand Up @@ -68,12 +66,9 @@ class ASH_EXPORT AshService : public service_manager::Service,
public service_manager::mojom::ServiceFactory,
public ws::gpu_host::GpuHostDelegate {
public:
AshService();
explicit AshService(service_manager::mojom::ServiceRequest request);
~AshService() override;

// Returns an appropriate EmbeddedServiceInfo that creates AshService.
static service_manager::EmbeddedServiceInfo CreateEmbeddedServiceInfo();

// service_manager::Service:
void OnStart() override;
void OnBindInterface(const service_manager::BindSourceInfo& remote_info,
Expand All @@ -99,6 +94,7 @@ class ASH_EXPORT AshService : public service_manager::Service,
// ui::ws::GpuHostDelegate:
void OnGpuServiceInitialized() override;

service_manager::ServiceBinding service_binding_;
service_manager::BinderRegistry registry_;
mojo::BindingSet<service_manager::mojom::ServiceFactory>
service_factory_bindings_;
Expand Down
11 changes: 5 additions & 6 deletions ash/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "services/service_manager/public/c/main.h"
#include "ash/ash_service.h"
#include "base/feature_list.h"
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/message_loop/message_loop.h"
#include "base/path_service.h"
#include "services/service_manager/public/cpp/service_runner.h"
#include "base/run_loop.h"
#include "ui/base/material_design/material_design_controller.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/base/ui_base_features.h"

// This path is only hit in testing, not production. Production launches ash by
// way of the utility process, which does not use this.
MojoResult ServiceMain(MojoHandle service_request_handle) {
void ServiceMain(service_manager::mojom::ServiceRequest request) {
logging::SetLogPrefix("ash");
// Load ash resources and strings.
// TODO: investigate nuking ash_service_resources and use the same resources
Expand Down Expand Up @@ -54,7 +54,6 @@ MojoResult ServiceMain(MojoHandle service_request_handle) {

ui::MaterialDesignController::Initialize();

service_manager::ServiceRunner runner(new ash::AshService);
runner.set_message_loop_type(base::MessageLoop::TYPE_UI);
return runner.Run(service_request_handle);
base::MessageLoop message_loop(base::MessageLoop::TYPE_UI);
ash::AshService(std::move(request)).RunUntilTermination();
}
10 changes: 5 additions & 5 deletions ash/shell/content/client/shell_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ void ShellContentBrowserClient::RegisterOutOfProcessServices(
&base::ASCIIToUTF16, test_ime_driver::mojom::kServiceName);
}

void ShellContentBrowserClient::RegisterInProcessServices(
StaticServiceMap* services,
content::ServiceManagerConnection* connection) {
services->insert(std::make_pair(mojom::kServiceName,
AshService::CreateEmbeddedServiceInfo()));
void ShellContentBrowserClient::HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request) {
service_manager::Service::RunAsyncUntilTermination(
std::make_unique<AshService>(std::move(request)));
}

} // namespace shell
Expand Down
6 changes: 3 additions & 3 deletions ash/shell/content/client/shell_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ class ShellContentBrowserClient : public content::ContentBrowserClient {
base::StringPiece name) override;
std::vector<ServiceManifestInfo> GetExtraServiceManifests() override;
void RegisterOutOfProcessServices(OutOfProcessServiceMap* services) override;
void RegisterInProcessServices(
StaticServiceMap* services,
content::ServiceManagerConnection* connection) override;
void HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request) override;

private:
ShellBrowserMainParts* shell_browser_main_parts_;
Expand Down
13 changes: 8 additions & 5 deletions chrome/browser/ash_service_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,15 @@ void RegisterInProcessServices(
});
(*services)[ax::mojom::kAXHostServiceName] = info;
}
}

if (features::IsMultiProcessMash())
return;

(*services)[ash::mojom::kServiceName] =
ash::AshService::CreateEmbeddedServiceInfo();
void HandleServiceRequest(const std::string& service_name,
service_manager::mojom::ServiceRequest request) {
if (!features::IsMultiProcessMash() &&
service_name == ash::mojom::kServiceName) {
service_manager::Service::RunAsyncUntilTermination(
std::make_unique<ash::AshService>(std::move(request)));
}
}

bool IsAshRelatedServiceName(const std::string& name) {
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ash_service_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ void RegisterInProcessServices(
content::ContentBrowserClient::StaticServiceMap* services,
content::ServiceManagerConnection* connection);

// Handles an incoming ServiceRequest. Newly added in-process services should be
// handled here instead of being registered by RegisterInProcessServices.
void HandleServiceRequest(const std::string& service_name,
service_manager::mojom::ServiceRequest request);

// Returns true if |name| identifies an Ash related service.
bool IsAshRelatedServiceName(const std::string& name);

Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/browser_process_platform_part_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,3 @@ std::unique_ptr<policy::ChromeBrowserPolicyConnector>
BrowserProcessPlatformPartBase::CreateBrowserPolicyConnector() {
return std::make_unique<policy::ChromeBrowserPolicyConnector>();
}

void BrowserProcessPlatformPartBase::RegisterInProcessServices(
content::ContentBrowserClient::StaticServiceMap* services,
content::ServiceManagerConnection* connection) {}
5 changes: 0 additions & 5 deletions chrome/browser/browser_process_platform_part_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ class BrowserProcessPlatformPartBase {
virtual std::unique_ptr<policy::ChromeBrowserPolicyConnector>
CreateBrowserPolicyConnector();

// Called from ChromeContentBrowserClient::RegisterInProcessServices().
virtual void RegisterInProcessServices(
content::ContentBrowserClient::StaticServiceMap* services,
content::ServiceManagerConnection* connection);

private:
DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPartBase);
};
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/browser_process_platform_part_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,6 @@ BrowserProcessPlatformPart::CreateBrowserPolicyConnector() {
new policy::BrowserPolicyConnectorChromeOS());
}

void BrowserProcessPlatformPart::RegisterInProcessServices(
content::ContentBrowserClient::StaticServiceMap* services,
content::ServiceManagerConnection* connection) {
ash_service_registry::RegisterInProcessServices(services, connection);
}

chromeos::system::SystemClock* BrowserProcessPlatformPart::GetSystemClock() {
if (!system_clock_.get())
system_clock_.reset(new chromeos::system::SystemClock());
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/browser_process_platform_part_chromeos.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
void StartTearDown() override;
std::unique_ptr<policy::ChromeBrowserPolicyConnector>
CreateBrowserPolicyConnector() override;
void RegisterInProcessServices(
content::ContentBrowserClient::StaticServiceMap* services,
content::ServiceManagerConnection* connection) override;

chromeos::system::SystemClock* GetSystemClock();
void DestroySystemClock();
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3898,6 +3898,10 @@ void ChromeContentBrowserClient::HandleServiceRequest(
media::CreateMediaService(std::move(request)));
}
#endif

#if defined(OS_CHROMEOS)
ash_service_registry::HandleServiceRequest(service_name, std::move(request));
#endif
}

bool ChromeContentBrowserClient::ShouldTerminateOnServiceQuit(
Expand Down
8 changes: 5 additions & 3 deletions chrome/utility/mash_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ void RecordMashServiceLaunch(MashService service) {
UMA_HISTOGRAM_ENUMERATION("Launch.MashService", service);
}

std::unique_ptr<service_manager::Service> CreateAshService() {
std::unique_ptr<service_manager::Service> CreateAshService(
service_manager::mojom::ServiceRequest request) {
RecordMashServiceLaunch(MashService::kAsh);
logging::SetLogPrefix("ash");
return std::make_unique<ash::AshService>();
return std::make_unique<ash::AshService>(std::move(request));
}

std::unique_ptr<service_manager::Service> CreateQuickLaunchService(
Expand Down Expand Up @@ -86,7 +87,6 @@ MashServiceFactory::~MashServiceFactory() = default;

void MashServiceFactory::RegisterOutOfProcessServices(
content::ContentUtilityClient::StaticServiceMap* services) {
RegisterMashService(services, ash::mojom::kServiceName, &CreateAshService);
RegisterMashService(services, shortcut_viewer::mojom::kServiceName,
&CreateShortcutViewerApp);
RegisterMashService(services, tap_visualizer::mojom::kServiceName,
Expand All @@ -99,6 +99,8 @@ std::unique_ptr<service_manager::Service>
MashServiceFactory::HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request) {
if (service_name == ash::mojom::kServiceName)
return CreateAshService(std::move(request));
if (service_name == quick_launch::mojom::kServiceName)
return CreateQuickLaunchService(std::move(request));

Expand Down
7 changes: 7 additions & 0 deletions services/service_manager/public/cpp/service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "services/service_manager/public/cpp/service.h"

#include "base/logging.h"
#include "base/run_loop.h"
#include "services/service_manager/public/cpp/service_context.h"

namespace service_manager {
Expand Down Expand Up @@ -34,6 +35,12 @@ bool Service::OnServiceManagerConnectionLost() {
return true;
}

void Service::RunUntilTermination() {
base::RunLoop loop;
set_termination_closure(loop.QuitClosure());
loop.Run();
}

void Service::Terminate() {
if (termination_closure_)
std::move(termination_closure_).Run();
Expand Down
Loading

0 comments on commit e116c20

Please sign in to comment.