Skip to content

Commit

Permalink
Revert "Move Ash off Service Manager"
Browse files Browse the repository at this point in the history
This reverts commit 008b726.

Reason for revert: causing ARC-related tast tests to fail
eg https://ci.chromium.org/p/chrome/builders/ci/chromeos-kevin-chrome/2

Original change's description:
> Move Ash off Service Manager
> 
> This removes all dependences on the Service Manager from within Ash and
> clients of Ash. The "Ash Service" is deleted, and the few interfaces
> which the browser bound through it are now bound through a handful of
> public helper functions that route through Shell.
> 
> The only service interfaces used from within Ash are from the
> Device Service, and those are now acquired through ShellDelegate.
> 
> Bug: 977637
> Change-Id: Ia117241f4adddd14303176af78075d0df6c5b563
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956160
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Ken Rockot <rockot@google.com>
> Cr-Commit-Position: refs/heads/master@{#723475}

TBR=sky@chromium.org,rockot@google.com,tsepez@chromium.org

Change-Id: I60bd3b9803f7017e44f2adc41b3304beee5fddd2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 977637, 1032816
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1961271
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723623}
  • Loading branch information
bpastene authored and Commit Bot committed Dec 11, 2019
1 parent 983f38e commit 2060897
Show file tree
Hide file tree
Showing 86 changed files with 704 additions and 172 deletions.
13 changes: 11 additions & 2 deletions ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import("//build/config/features.gni")
import("//build/config/ui.gni")
import("//mojo/public/tools/bindings/mojom.gni")
import("//services/service_manager/public/cpp/service_executable.gni")
import("//testing/test.gni")
import("//tools/grit/repack.gni")
import("//ui/base/ui_features.gni")
Expand All @@ -23,7 +24,8 @@ component("ash") {
friend = [ ":*" ]

public = [
"public/ash_interfaces.h",
# This is the only header that should be used externally.
"ash_service.h",

# TODO: move the following to source. Do NOT add new files here.
"accessibility/accessibility_controller_impl.h",
Expand Down Expand Up @@ -180,8 +182,8 @@ component("ash") {
"app_list/app_list_presenter_delegate_impl.cc",
"app_list/app_list_presenter_delegate_impl.h",
"ash_export.h",
"ash_interfaces.cc",
"ash_prefs.cc",
"ash_service.cc",
"assistant/assistant_alarm_timer_controller.cc",
"assistant/assistant_alarm_timer_controller.h",
"assistant/assistant_controller.cc",
Expand Down Expand Up @@ -516,6 +518,8 @@ component("ash") {
"metrics/user_metrics_action.h",
"metrics/user_metrics_recorder.cc",
"metrics/user_metrics_recorder.h",
"mojo_interface_factory.cc",
"mojo_interface_factory.h",
"multi_device_setup/multi_device_notification_presenter.cc",
"multi_device_setup/multi_device_notification_presenter.h",
"multi_profile_uma.cc",
Expand Down Expand Up @@ -1372,6 +1376,7 @@ component("ash") {
"//services/device/public/mojom",
"//services/media_session/public/cpp",
"//services/media_session/public/mojom",
"//services/service_manager/public/cpp",
"//skia",
"//ui/aura",
"//ui/events",
Expand Down Expand Up @@ -1463,6 +1468,7 @@ component("ash") {
"//services/content/public/cpp",
"//services/data_decoder/public/cpp",
"//services/preferences/public/cpp",
"//services/service_manager/public/cpp",

# TODO(msw): Remove this; ash should not depend on blink/webkit.
"//third_party/blink/public:blink_headers",
Expand Down Expand Up @@ -1602,6 +1608,7 @@ static_library("ash_shell_lib_with_content") {
":test_support",
"//ash/components/shortcut_viewer",
"//ash/public/cpp",
"//ash/public/cpp:manifest",
"//base:i18n",
"//chrome:packed_resources",

Expand Down Expand Up @@ -2029,6 +2036,7 @@ test("ash_unittests") {
"//ash/keyboard/ui",
"//ash/keyboard/ui:test_support",
"//ash/public/cpp",
"//ash/public/cpp:manifest",
"//ash/public/cpp:test_support",
"//ash/public/cpp:unit_tests",
"//ash/public/cpp/vector_icons",
Expand Down Expand Up @@ -2359,6 +2367,7 @@ static_library("test_support") {
"//chromeos/services/assistant/public/mojom",
"//chromeos/services/network_config/public/cpp:test_support",
"//dbus",
"//services/service_manager/public/cpp/test:test_support",
"//testing/gtest",
"//third_party/blink/public:blink_headers",
"//ui/display:display_manager_test_api",
Expand Down
2 changes: 1 addition & 1 deletion ash/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ include_rules = [
"+media",
"+mojo/public",
"+services/content/public",
"+services/device/public",
"+services/data_decoder/public",
"+services/media_session/public",
"+services/preferences/public",
"+services/service_manager/public",
"+services/viz/public",
"+skia/ext",
"+third_party/cros_system_api",
Expand Down
1 change: 1 addition & 0 deletions ash/accelerators/debug_commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#include "base/strings/utf_string_conversions.h"
#include "services/service_manager/public/cpp/connector.h"
#include "ui/accessibility/ax_tree_id.h"
#include "ui/accessibility/platform/aura_window_properties.h"
#include "ui/aura/client/aura_constants.h"
Expand Down
26 changes: 0 additions & 26 deletions ash/ash_interfaces.cc

This file was deleted.

28 changes: 28 additions & 0 deletions ash/ash_service.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/ash_service.h"

#include "ash/mojo_interface_factory.h"

namespace ash {

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

AshService::~AshService() = default;

void AshService::OnStart() {
mojo_interface_factory::RegisterInterfaces(
&registry_, base::ThreadTaskRunnerHandle::Get());
}

void AshService::OnBindInterface(
const service_manager::BindSourceInfo& remote_info,
const std::string& interface_name,
mojo::ScopedMessagePipeHandle handle) {
registry_.BindInterface(interface_name, std::move(handle));
}

} // namespace ash
39 changes: 39 additions & 0 deletions ash/ash_service.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef ASH_ASH_SERVICE_H_
#define ASH_ASH_SERVICE_H_

#include "ash/ash_export.h"
#include "base/macros.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"

namespace ash {

// Used to export Ash's mojo services, specifically the interfaces defined in
// Ash's manifest.
class ASH_EXPORT AshService : public service_manager::Service {
public:
explicit AshService(service_manager::mojom::ServiceRequest request);
~AshService() override;

// service_manager::Service:
void OnStart() override;
void OnBindInterface(const service_manager::BindSourceInfo& remote_info,
const std::string& interface_name,
mojo::ScopedMessagePipeHandle handle) override;

private:
service_manager::ServiceBinding service_binding_;
service_manager::BinderRegistry registry_;

DISALLOW_COPY_AND_ASSIGN(AshService);
};

} // namespace ash

#endif // ASH_ASH_SERVICE_H_
1 change: 1 addition & 0 deletions ash/components/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ include_rules = [

# Components can be mojo services.
"+mojo/public",
"+services/service_manager/public",

# Components support views UI with aura.
"+cc/paint",
Expand Down
1 change: 1 addition & 0 deletions ash/dbus/display_service_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/bind.h"
#include "base/callback.h"
#include "dbus/message.h"
#include "services/service_manager/public/cpp/connector.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
#include "ui/base/user_activity/user_activity_detector.h"
#include "ui/display/manager/display_configurator.h"
Expand Down
2 changes: 2 additions & 0 deletions ash/dbus/gesture_properties_service_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "dbus/message.h"
#include "dbus/object_path.h"
#include "gmock/gmock.h"
#include "services/service_manager/public/cpp/test/test_connector_factory.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
#include "ui/ozone/testhelpers/mock_gesture_properties_service.h"

Expand Down Expand Up @@ -62,6 +63,7 @@ double expect_double(dbus::MessageReader* reader) {
class GesturePropertiesServiceProviderTest : public testing::Test {
public:
GesturePropertiesServiceProviderTest() {
service_manager::TestConnectorFactory test_connector_factory;
mock_service_ = std::make_unique<MockGesturePropertiesService>();
ON_CALL(*mock_service_, ListDevices(_))
.WillByDefault(Invoke(
Expand Down
1 change: 1 addition & 0 deletions ash/keyboard/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ test("keyboard_unittests") {
"//base/test:test_support",
"//components/ukm:test_support",
"//mojo/core/embedder",
"//services/service_manager/public/cpp",
"//testing/gmock",
"//testing/gtest",
"//ui/aura:test_support",
Expand Down
1 change: 1 addition & 0 deletions ash/keyboard/ui/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ include_rules = [
"+components/ukm",
"+mojo/public",
"+services/metrics/public/cpp",
"+services/service_manager/public",
"+ui/aura",
"+ui/base",
"+ui/compositor",
Expand Down
69 changes: 69 additions & 0 deletions ash/mojo_interface_factory.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/mojo_interface_factory.h"

#include <utility>

#include "ash/app_list/app_list_controller_impl.h"
#include "ash/display/cros_display_config.h"
#include "ash/ime/ime_controller_impl.h"
#include "ash/login/login_screen_controller.h"
#include "ash/media/media_controller_impl.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/mojom/cros_display_config.mojom.h"
#include "ash/shell.h"
#include "ash/shell_delegate.h"
#include "ash/tray_action/tray_action.h"
#include "base/bind.h"
#include "base/lazy_instance.h"
#include "base/single_thread_task_runner.h"
#include "chromeos/constants/chromeos_features.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"

namespace ash {
namespace mojo_interface_factory {
namespace {

base::LazyInstance<RegisterInterfacesCallback>::Leaky
g_register_interfaces_callback = LAZY_INSTANCE_INITIALIZER;

void BindCrosDisplayConfigControllerReceiverOnMainThread(
mojo::PendingReceiver<mojom::CrosDisplayConfigController> receiver) {
if (Shell::HasInstance())
Shell::Get()->cros_display_config()->BindReceiver(std::move(receiver));
}

void BindTrayActionReceiverOnMainThread(
mojo::PendingReceiver<mojom::TrayAction> receiver) {
if (Shell::HasInstance())
Shell::Get()->tray_action()->BindReceiver(std::move(receiver));
}

} // namespace

void RegisterInterfaces(
service_manager::BinderRegistry* registry,
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner) {
registry->AddInterface(
base::BindRepeating(&BindCrosDisplayConfigControllerReceiverOnMainThread),
main_thread_task_runner);
registry->AddInterface(
base::BindRepeating(&BindTrayActionReceiverOnMainThread),
main_thread_task_runner);

// Inject additional optional interfaces.
if (g_register_interfaces_callback.Get()) {
std::move(g_register_interfaces_callback.Get())
.Run(registry, main_thread_task_runner);
}
}

void SetRegisterInterfacesCallback(RegisterInterfacesCallback callback) {
g_register_interfaces_callback.Get() = std::move(callback);
}

} // namespace mojo_interface_factory

} // namespace ash
40 changes: 40 additions & 0 deletions ash/mojo_interface_factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef ASH_MOJO_INTERFACE_FACTORY_H_
#define ASH_MOJO_INTERFACE_FACTORY_H_

#include "ash/ash_export.h"
#include "base/callback_forward.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "services/service_manager/public/cpp/binder_registry.h"

namespace base {
class SingleThreadTaskRunner;
}

namespace ash {

namespace mojo_interface_factory {

// Registers all mojo services provided by ash. May be called on IO thread
// (when running ash in-process in chrome) or on the main thread (when running
// in mash).
ASH_EXPORT void RegisterInterfaces(
service_manager::BinderRegistry* registry,
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner);

// Registers a callback that runs during |RegisterInterfaces|. Can be used to
// inject interfaces from test-only build targets.
using RegisterInterfacesCallback =
base::OnceCallback<void(service_manager::BinderRegistry*,
scoped_refptr<base::SingleThreadTaskRunner>)>;
ASH_EXPORT void SetRegisterInterfacesCallback(RegisterInterfacesCallback cb);

} // namespace mojo_interface_factory

} // namespace ash

#endif // ASH_MOJO_INTERFACE_FACTORY_H_
24 changes: 0 additions & 24 deletions ash/public/ash_interfaces.h

This file was deleted.

Loading

0 comments on commit 2060897

Please sign in to comment.