Skip to content

Commit

Permalink
cros: Replace some use of mojo::BindingSet in ash with Binding
Browse files Browse the repository at this point in the history
Several mojo interface implementation in //ash only support a single
connection. For example, many interfaces exist primarily to provide
a single client/delegate interface back to the browser. Replace
their usage of BindingSet with Binding.

Also clean up some interface overrides where a FooImpl in ash
overrides both mojom::Foo and mojom::FooClient. These impl objects
are not "is-a" client interface impls, so don't derive from that
interface.

Bug: none
Test: ash_unittests, browser_tests
Change-Id: Id65aadda0ad9dd74702fbf21fbb8aa307b39d9fc
Reviewed-on: https://chromium-review.googlesource.com/766649
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516100}
  • Loading branch information
James Cook authored and Commit Bot committed Nov 13, 2017
1 parent fb9b79b commit 8f1e606
Show file tree
Hide file tree
Showing 17 changed files with 54 additions and 52 deletions.
4 changes: 2 additions & 2 deletions ash/cast_config_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace ash {

CastConfigController::CastConfigController() {}
CastConfigController::CastConfigController() : binding_(this) {}

CastConfigController::~CastConfigController() {}

Expand All @@ -27,7 +27,7 @@ void CastConfigController::RemoveObserver(
}

void CastConfigController::BindRequest(mojom::CastConfigRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

void CastConfigController::SetClient(
Expand Down
17 changes: 8 additions & 9 deletions ash/cast_config_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include "base/macros.h"
#include "base/observer_list.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"

namespace ash {

Expand All @@ -29,8 +29,7 @@ class CastConfigControllerObserver {

// We want to establish our connection lazily and preferably only once, as
// TrayCast instances will come and go.
class CastConfigController : public ash::mojom::CastConfig,
public ash::mojom::CastConfigClient {
class CastConfigController : public ash::mojom::CastConfig {
public:
CastConfigController();
~CastConfigController() override;
Expand All @@ -48,14 +47,14 @@ class CastConfigController : public ash::mojom::CastConfig,
void SetClient(mojom::CastConfigClientAssociatedPtrInfo client) override;
void OnDevicesUpdated(std::vector<mojom::SinkAndRoutePtr> devices) override;

// ash::mojom::CastConfigClient:
void RequestDeviceRefresh() override;
void CastToSink(mojom::CastSinkPtr sink) override;
void StopCasting(mojom::CastRoutePtr route) override;
// Methods to forward to |client_|.
void RequestDeviceRefresh();
void CastToSink(mojom::CastSinkPtr sink);
void StopCasting(mojom::CastRoutePtr route);

private:
// Bindings for the CastConfig interface.
mojo::BindingSet<mojom::CastConfig> bindings_;
// Binding for the CastConfig interface.
mojo::Binding<mojom::CastConfig> binding_;

mojom::CastConfigClientAssociatedPtr client_;

Expand Down
4 changes: 2 additions & 2 deletions ash/ime/ime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

namespace ash {

ImeController::ImeController() = default;
ImeController::ImeController() : binding_(this) {}

ImeController::~ImeController() = default;

Expand All @@ -24,7 +24,7 @@ void ImeController::RemoveObserver(Observer* observer) {
}

void ImeController::BindRequest(mojom::ImeControllerRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

void ImeController::SetClient(mojom::ImeControllerClientPtr client) {
Expand Down
6 changes: 3 additions & 3 deletions ash/ime/ime_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include "ash/public/interfaces/ime_info.mojom.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"

namespace ui {
class Accelerator;
Expand Down Expand Up @@ -100,8 +100,8 @@ class ASH_EXPORT ImeController : public mojom::ImeController {
std::vector<std::string> GetCandidateImesForAccelerator(
const ui::Accelerator& accelerator) const;

// Bindings for users of the mojo interface.
mojo::BindingSet<mojom::ImeController> bindings_;
// Binding for the mojo interface.
mojo::Binding<mojom::ImeController> binding_;

// Client interface back to IME code in chrome.
mojom::ImeControllerClientPtr client_;
Expand Down
5 changes: 3 additions & 2 deletions ash/login/lock_screen_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ std::string CalculateHash(const std::string& password,

} // namespace

LockScreenController::LockScreenController() = default;
LockScreenController::LockScreenController() : binding_(this) {}

LockScreenController::~LockScreenController() = default;

// static
Expand All @@ -48,7 +49,7 @@ void LockScreenController::RegisterProfilePrefs(PrefRegistrySimple* registry,
}

void LockScreenController::BindRequest(mojom::LockScreenRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

void LockScreenController::SetClient(mojom::LockScreenClientPtr client) {
Expand Down
6 changes: 3 additions & 3 deletions ash/login/lock_screen_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "ash/public/interfaces/lock_screen.mojom.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"

class PrefRegistrySimple;

Expand Down Expand Up @@ -109,8 +109,8 @@ class ASH_EXPORT LockScreenController : public mojom::LockScreen {
// Client interface in chrome browser. May be null in tests.
mojom::LockScreenClientPtr lock_screen_client_;

// Bindings for the LockScreen interface.
mojo::BindingSet<mojom::LockScreen> bindings_;
// Binding for the LockScreen interface.
mojo::Binding<mojom::LockScreen> binding_;

// User authentication call that will run when we have system salt.
PendingAuthenticateUserCall pending_user_auth_;
Expand Down
4 changes: 2 additions & 2 deletions ash/media_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@

namespace ash {

MediaController::MediaController() {}
MediaController::MediaController() : binding_(this) {}

MediaController::~MediaController() {}

void MediaController::BindRequest(mojom::MediaControllerRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

void MediaController::AddObserver(MediaCaptureObserver* observer) {
Expand Down
19 changes: 9 additions & 10 deletions ash/media_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "base/macros.h"
#include "base/observer_list.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"

namespace ash {

Expand All @@ -27,8 +27,7 @@ class MediaCaptureObserver {
// Provides the MediaController interface to the outside world. This lets a
// consumer of ash provide a MediaClient, which we will dispatch to if one has
// been provided to us.
class MediaController : public mojom::MediaController,
public mojom::MediaClient {
class MediaController : public mojom::MediaController {
public:
MediaController();
~MediaController() override;
Expand All @@ -43,17 +42,17 @@ class MediaController : public mojom::MediaController,
void NotifyCaptureState(
const std::vector<mojom::MediaCaptureState>& capture_states) override;

// mojom::MediaClient:
void HandleMediaNextTrack() override;
void HandleMediaPlayPause() override;
void HandleMediaPrevTrack() override;
void RequestCaptureState() override;
void SuspendMediaSessions() override;
// Methods that forward to |client_|.
void HandleMediaNextTrack();
void HandleMediaPlayPause();
void HandleMediaPrevTrack();
void RequestCaptureState();
void SuspendMediaSessions();

private:
friend class MultiProfileMediaTrayItemTest;

mojo::BindingSet<mojom::MediaController> bindings_;
mojo::Binding<mojom::MediaController> binding_;

mojom::MediaClientAssociatedPtr client_;

Expand Down
4 changes: 2 additions & 2 deletions ash/new_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

namespace ash {

NewWindowController::NewWindowController() {}
NewWindowController::NewWindowController() : binding_(this) {}

NewWindowController::~NewWindowController() {}

void NewWindowController::BindRequest(
mojom::NewWindowControllerRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

void NewWindowController::SetClient(
Expand Down
4 changes: 2 additions & 2 deletions ash/new_window_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "ash/public/interfaces/new_window.mojom.h"
#include "base/macros.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"

namespace ash {

Expand Down Expand Up @@ -38,7 +38,7 @@ class ASH_EXPORT NewWindowController : public mojom::NewWindowController {
void OpenFeedbackPage();

private:
mojo::BindingSet<mojom::NewWindowController> bindings_;
mojo::Binding<mojom::NewWindowController> binding_;

mojom::NewWindowClientAssociatedPtr client_;

Expand Down
2 changes: 2 additions & 0 deletions ash/session/session_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ class ASH_EXPORT SessionController : public mojom::SessionController {
std::unique_ptr<PrefService> pref_service);

// Bindings for mojom::SessionController interface.
// TODO(jamescook): This should be mojo::Binding<> but that causes crashes in
// browser test UserAddingScreenTest.AddingSeveralUsers.
mojo::BindingSet<mojom::SessionController> bindings_;

// Client interface to session manager code (chrome).
Expand Down
4 changes: 2 additions & 2 deletions ash/system/tray/system_tray_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace ash {

SystemTrayController::SystemTrayController()
: hour_clock_type_(base::GetHourClockType()) {}
: binding_(this), hour_clock_type_(base::GetHourClockType()) {}

SystemTrayController::~SystemTrayController() {}

Expand Down Expand Up @@ -140,7 +140,7 @@ void SystemTrayController::RequestRestartForUpdate() {
}

void SystemTrayController::BindRequest(mojom::SystemTrayRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

void SystemTrayController::SetClient(mojom::SystemTrayClientPtr client) {
Expand Down
6 changes: 3 additions & 3 deletions ash/system/tray/system_tray_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "base/compiler_specific.h"
#include "base/i18n/time_formatting.h"
#include "base/macros.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"

namespace ash {

Expand Down Expand Up @@ -80,8 +80,8 @@ class ASH_EXPORT SystemTrayController : public mojom::SystemTray {
// Client interface in chrome browser. May be null in tests.
mojom::SystemTrayClientPtr system_tray_client_;

// Bindings for the SystemTray interface.
mojo::BindingSet<mojom::SystemTray> bindings_;
// Binding for the SystemTray interface.
mojo::Binding<mojom::SystemTray> binding_;

// The type of clock hour display: 12 or 24 hour.
base::HourClockType hour_clock_type_;
Expand Down
4 changes: 2 additions & 2 deletions ash/tray_action/tray_action.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace ash {

TrayAction::TrayAction() = default;
TrayAction::TrayAction() : binding_(this) {}

TrayAction::~TrayAction() = default;

Expand All @@ -26,7 +26,7 @@ void TrayAction::RemoveObserver(TrayActionObserver* observer) {
}

void TrayAction::BindRequest(mojom::TrayActionRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

mojom::TrayActionState TrayAction::GetLockScreenNoteState() const {
Expand Down
4 changes: 2 additions & 2 deletions ash/tray_action/tray_action.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "ash/public/interfaces/tray_action.mojom.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"

namespace ash {

Expand Down Expand Up @@ -67,7 +67,7 @@ class ASH_EXPORT TrayAction : public mojom::TrayAction {

base::ObserverList<TrayActionObserver> observers_;

mojo::BindingSet<mojom::TrayAction> bindings_;
mojo::Binding<mojom::TrayAction> binding_;

mojom::TrayActionClientPtr tray_action_client_;

Expand Down
5 changes: 3 additions & 2 deletions ash/wm/tablet_mode/tablet_mode_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ TabletModeController::TabletModeController()
lid_is_closed_(false),
auto_hide_title_bars_(!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kAshDisableTabletAutohideTitlebars)),
binding_(this),
scoped_session_observer_(this),
weak_factory_(this) {
Shell::Get()->AddShellObserver(this);
Expand Down Expand Up @@ -201,7 +202,7 @@ void TabletModeController::AddWindow(aura::Window* window) {

void TabletModeController::BindRequest(
mojom::TabletModeControllerRequest request) {
bindings_.AddBinding(this, std::move(request));
binding_.Bind(std::move(request));
}

void TabletModeController::AddObserver(TabletModeObserver* observer) {
Expand Down Expand Up @@ -389,7 +390,7 @@ void TabletModeController::LeaveTabletMode() {
}

void TabletModeController::FlushForTesting() {
bindings_.FlushForTesting();
binding_.FlushForTesting();
}

void TabletModeController::OnShellInitialized() {
Expand Down
8 changes: 4 additions & 4 deletions ash/wm/tablet_mode/tablet_mode_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#include "chromeos/accelerometer/accelerometer_reader.h"
#include "chromeos/accelerometer/accelerometer_types.h"
#include "chromeos/dbus/power_manager_client.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "ui/gfx/geometry/vector3d_f.h"

Expand Down Expand Up @@ -213,16 +213,16 @@ class ASH_EXPORT TabletModeController
bool lid_is_closed_;

// Whether title bars should be shown be auto hidden in tablet mode.
const bool auto_hide_title_bars_ = false;
const bool auto_hide_title_bars_;

// Tracks smoothed accelerometer data over time. This is done when the hinge
// is approaching vertical to remove abrupt acceleration that can lead to
// incorrect calculations of hinge angles.
gfx::Vector3dF base_smoothed_;
gfx::Vector3dF lid_smoothed_;

// Bindings for the TabletModeController interface.
mojo::BindingSet<mojom::TabletModeController> bindings_;
// Binding for the TabletModeController interface.
mojo::Binding<mojom::TabletModeController> binding_;

// Client interface (e.g. in chrome).
mojom::TabletModeClientPtr client_;
Expand Down

0 comments on commit 8f1e606

Please sign in to comment.