Skip to content

Commit

Permalink
[dPWA][SystemMediaControls] Update SystemMediaControls to be instance…
Browse files Browse the repository at this point in the history 
…able

Support for system media controls per-dPWA.

Related changes:
- Remove SMCWin's singleton instance and update references accordingly
- Update SMC::Create API to take an optional window parameter to identify which dPWA the controls are for
- Update SMCObserver interface to pass a sender arg so observers know which controls are sending a message
- update Linux/Mac files as needed to account for these changes

Bug: 1463747
Change-Id: I659f70e925a6c8138820f6a9a2d704c088a4d372
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4943395
Commit-Queue: Lia Hiscock <liahiscock@microsoft.com>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212307}
  • Loading branch information
Lia Hiscock authored and Chromium LUCI CQ committed Oct 19, 2023
1 parent e85d247 commit 8a9b8fd
Show file tree
Hide file tree
Showing 12 changed files with 155 additions and 114 deletions.
19 changes: 10 additions & 9 deletions components/system_media_controls/linux/system_media_controls_linux.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ namespace system_media_controls {

// static
std::unique_ptr<SystemMediaControls> SystemMediaControls::Create(
const std::string& product_name) {
const std::string& product_name,
int window) {
auto service =
std::make_unique<internal::SystemMediaControlsLinux>(product_name);
service->StartService();
Expand Down Expand Up @@ -390,7 +391,7 @@ void SystemMediaControlsLinux::Next(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
for (SystemMediaControlsObserver& obs : observers_) {
obs.OnNext();
obs.OnNext(this);
}
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
Expand All @@ -399,7 +400,7 @@ void SystemMediaControlsLinux::Previous(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
for (SystemMediaControlsObserver& obs : observers_) {
obs.OnPrevious();
obs.OnPrevious(this);
}
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
Expand All @@ -408,7 +409,7 @@ void SystemMediaControlsLinux::Pause(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
for (SystemMediaControlsObserver& obs : observers_) {
obs.OnPause();
obs.OnPause(this);
}
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
Expand All @@ -417,7 +418,7 @@ void SystemMediaControlsLinux::PlayPause(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
for (SystemMediaControlsObserver& obs : observers_) {
obs.OnPlayPause();
obs.OnPlayPause(this);
}
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
Expand All @@ -426,7 +427,7 @@ void SystemMediaControlsLinux::Stop(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
for (SystemMediaControlsObserver& obs : observers_) {
obs.OnStop();
obs.OnStop(this);
}
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
Expand All @@ -435,7 +436,7 @@ void SystemMediaControlsLinux::Play(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
for (SystemMediaControlsObserver& obs : observers_) {
obs.OnPlay();
obs.OnPlay(this);
}
std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
}
Expand All @@ -451,7 +452,7 @@ void SystemMediaControlsLinux::Seek(
}

for (SystemMediaControlsObserver& obs : observers_) {
obs.OnSeek(base::Microseconds(offset));
obs.OnSeek(this, base::Microseconds(offset));
}

std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
Expand All @@ -475,7 +476,7 @@ void SystemMediaControlsLinux::SetPositionMpris(
}

for (SystemMediaControlsObserver& obs : observers_) {
obs.OnSeekTo(base::Microseconds(position));
obs.OnSeekTo(this, base::Microseconds(position));
}

std::move(response_sender).Run(dbus::Response::FromMethodCall(method_call));
Expand Down
43 changes: 20 additions & 23 deletions components/system_media_controls/linux/system_media_controls_linux_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,18 @@ class MockSystemMediaControlsObserver : public SystemMediaControlsObserver {

// SystemMediaControlsObserver implementation.
MOCK_METHOD0(OnServiceReady, void());
MOCK_METHOD0(OnNext, void());
MOCK_METHOD0(OnPrevious, void());
MOCK_METHOD0(OnPause, void());
MOCK_METHOD0(OnPlayPause, void());
MOCK_METHOD0(OnStop, void());
MOCK_METHOD0(OnPlay, void());
MOCK_METHOD1(OnSeek, void(const base::TimeDelta&));
MOCK_METHOD1(OnSeekTo, void(const base::TimeDelta&));
MOCK_METHOD1(OnNext, void(system_media_controls::SystemMediaControls*));
MOCK_METHOD1(OnPrevious, void(system_media_controls::SystemMediaControls*));
MOCK_METHOD1(OnPause, void(system_media_controls::SystemMediaControls*));
MOCK_METHOD1(OnPlayPause, void(system_media_controls::SystemMediaControls*));
MOCK_METHOD1(OnStop, void(system_media_controls::SystemMediaControls*));
MOCK_METHOD1(OnPlay, void(system_media_controls::SystemMediaControls*));
MOCK_METHOD2(OnSeek,
void(system_media_controls::SystemMediaControls*,
const base::TimeDelta&));
MOCK_METHOD2(OnSeekTo,
void(system_media_controls::SystemMediaControls*,
const base::TimeDelta&));
};

class SystemMediaControlsLinuxTest : public testing::Test,
Expand Down Expand Up @@ -227,13 +231,6 @@ class SystemMediaControlsLinuxTest : public testing::Test,
if (service_wait_loop_)
service_wait_loop_->Quit();
}
void OnNext() override {}
void OnPrevious() override {}
void OnPlay() override {}
void OnPause() override {}
void OnPlayPause() override {}
void OnStop() override {}
void OnSeekTo(const base::TimeDelta& time) override {}

base::test::TaskEnvironment task_environment_;
std::unique_ptr<base::RunLoop> service_wait_loop_;
Expand All @@ -256,56 +253,56 @@ TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfServiceReadyWhenAdded) {

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfNextCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnNext());
EXPECT_CALL(observer, OnNext(GetService()));
AddObserver(&observer);
CallMediaPlayer2PlayerMethodAndBlock("Next");
}

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfPreviousCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnPrevious());
EXPECT_CALL(observer, OnPrevious(GetService()));
AddObserver(&observer);
CallMediaPlayer2PlayerMethodAndBlock("Previous");
}

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfPauseCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnPause());
EXPECT_CALL(observer, OnPause(GetService()));
AddObserver(&observer);
CallMediaPlayer2PlayerMethodAndBlock("Pause");
}

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfPlayPauseCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnPlayPause());
EXPECT_CALL(observer, OnPlayPause(GetService()));
AddObserver(&observer);
CallMediaPlayer2PlayerMethodAndBlock("PlayPause");
}

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfStopCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnStop());
EXPECT_CALL(observer, OnStop(GetService()));
AddObserver(&observer);
CallMediaPlayer2PlayerMethodAndBlock("Stop");
}

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfPlayCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnPlay());
EXPECT_CALL(observer, OnPlay(GetService()));
AddObserver(&observer);
CallMediaPlayer2PlayerMethodAndBlock("Play");
}

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfSeekCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnSeek(base::Seconds(3)));
EXPECT_CALL(observer, OnSeek(GetService(), base::Seconds(3)));
AddObserver(&observer);
CallSeekAndBlock(/*is_seek_to=*/false, base::Seconds(3).InMicroseconds());
}

TEST_F(SystemMediaControlsLinuxTest, ObserverNotifiedOfSetPositionCalls) {
MockSystemMediaControlsObserver observer;
EXPECT_CALL(observer, OnSeekTo(base::Seconds(7)));
EXPECT_CALL(observer, OnSeekTo(GetService(), base::Seconds(7)));
AddObserver(&observer);
CallSeekAndBlock(/*is_seek_to=*/true, base::Seconds(7).InMicroseconds());
}
Expand Down
5 changes: 4 additions & 1 deletion components/system_media_controls/mac/remote_command_center_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,16 @@ class TimeDelta;

namespace system_media_controls {

class SystemMediaControls;
class SystemMediaControlsObserver;

namespace internal {

// Wraps an NSObject which interfaces with the MPRemoteCommandCenter.
class RemoteCommandCenterDelegate {
public:
RemoteCommandCenterDelegate();
explicit RemoteCommandCenterDelegate(
SystemMediaControls* system_media_controls);

RemoteCommandCenterDelegate(const RemoteCommandCenterDelegate&) = delete;
RemoteCommandCenterDelegate& operator=(const RemoteCommandCenterDelegate&) =
Expand Down Expand Up @@ -65,6 +67,7 @@ class RemoteCommandCenterDelegate {
remote_command_center_delegate_cocoa_;
base::ObserverList<SystemMediaControlsObserver> observers_;
base::flat_set<Command> enabled_commands_;
const raw_ptr<SystemMediaControls> system_media_controls_;
};

} // namespace internal
Expand Down
18 changes: 10 additions & 8 deletions components/system_media_controls/mac/remote_command_center_delegate.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@

namespace system_media_controls::internal {

RemoteCommandCenterDelegate::RemoteCommandCenterDelegate() {
RemoteCommandCenterDelegate::RemoteCommandCenterDelegate(
SystemMediaControls* system_media_controls)
: system_media_controls_(system_media_controls) {
remote_command_center_delegate_cocoa_ =
[[RemoteCommandCenterDelegateCocoa alloc] initWithDelegate:this];
}
Expand Down Expand Up @@ -74,43 +76,43 @@
void RemoteCommandCenterDelegate::OnNext() {
DCHECK(enabled_commands_.contains(Command::kNextTrack));
for (auto& observer : observers_)
observer.OnNext();
observer.OnNext(system_media_controls_);
}

void RemoteCommandCenterDelegate::OnPrevious() {
DCHECK(enabled_commands_.contains(Command::kPreviousTrack));
for (auto& observer : observers_)
observer.OnPrevious();
observer.OnPrevious(system_media_controls_);
}

void RemoteCommandCenterDelegate::OnPlay() {
DCHECK(enabled_commands_.contains(Command::kPlayPause));
for (auto& observer : observers_)
observer.OnPlay();
observer.OnPlay(system_media_controls_);
}

void RemoteCommandCenterDelegate::OnPause() {
DCHECK(enabled_commands_.contains(Command::kPlayPause));
for (auto& observer : observers_)
observer.OnPause();
observer.OnPause(system_media_controls_);
}

void RemoteCommandCenterDelegate::OnPlayPause() {
DCHECK(enabled_commands_.contains(Command::kPlayPause));
for (auto& observer : observers_)
observer.OnPlayPause();
observer.OnPlayPause(system_media_controls_);
}

void RemoteCommandCenterDelegate::OnStop() {
DCHECK(enabled_commands_.contains(Command::kStop));
for (auto& observer : observers_)
observer.OnStop();
observer.OnStop(system_media_controls_);
}

void RemoteCommandCenterDelegate::OnSeekTo(const base::TimeDelta& time) {
DCHECK(enabled_commands_.contains(Command::kSeekTo));
for (auto& observer : observers_)
observer.OnSeekTo(time);
observer.OnSeekTo(system_media_controls_, time);
}

bool RemoteCommandCenterDelegate::ShouldSetCommandEnabled(Command command,
Expand Down
6 changes: 4 additions & 2 deletions components/system_media_controls/mac/system_media_controls_mac.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@

// static
std::unique_ptr<SystemMediaControls> SystemMediaControls::Create(
const std::string& product_name) {
const std::string& product_name,
int window) {
return std::make_unique<internal::SystemMediaControlsMac>();
}

namespace internal {

SystemMediaControlsMac::SystemMediaControlsMac() = default;
SystemMediaControlsMac::SystemMediaControlsMac()
: remote_command_center_delegate_(this) {}

SystemMediaControlsMac::~SystemMediaControlsMac() = default;

Expand Down
4 changes: 3 additions & 1 deletion components/system_media_controls/system_media_controls.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ class COMPONENT_EXPORT(SYSTEM_MEDIA_CONTROLS) SystemMediaControls {
kStopped,
};

// |window| used by Windows OS for web app (dPWA) connections.
static std::unique_ptr<SystemMediaControls> Create(
const std::string& product_name);
const std::string& product_name,
int window = -1);

virtual ~SystemMediaControls() = default;

Expand Down
20 changes: 12 additions & 8 deletions components/system_media_controls/system_media_controls_observer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ class TimeDelta;

namespace system_media_controls {

class SystemMediaControls;

// Interface to observe events on the SystemMediaControls.
class COMPONENT_EXPORT(SYSTEM_MEDIA_CONTROLS) SystemMediaControlsObserver
: public base::CheckedObserver {
Expand All @@ -23,14 +25,16 @@ class COMPONENT_EXPORT(SYSTEM_MEDIA_CONTROLS) SystemMediaControlsObserver
virtual void OnServiceReady() {}

// Called when the observer should handle the given control.
virtual void OnNext() {}
virtual void OnPrevious() {}
virtual void OnPlay() {}
virtual void OnPause() {}
virtual void OnPlayPause() {}
virtual void OnStop() {}
virtual void OnSeek(const base::TimeDelta& time) {}
virtual void OnSeekTo(const base::TimeDelta& time) {}
virtual void OnNext(SystemMediaControls* sender) {}
virtual void OnPrevious(SystemMediaControls* sender) {}
virtual void OnPlay(SystemMediaControls* sender) {}
virtual void OnPause(SystemMediaControls* sender) {}
virtual void OnPlayPause(SystemMediaControls* sender) {}
virtual void OnStop(SystemMediaControls* sender) {}
virtual void OnSeek(SystemMediaControls* sender,
const base::TimeDelta& time) {}
virtual void OnSeekTo(SystemMediaControls* sender,
const base::TimeDelta& time) {}

protected:
~SystemMediaControlsObserver() override = default;
Expand Down
3 changes: 2 additions & 1 deletion components/system_media_controls/system_media_controls_stub.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ namespace system_media_controls {

// static
std::unique_ptr<SystemMediaControls> SystemMediaControls::Create(
const std::string& product_name) {
const std::string& product_name,
int window) {
return nullptr;
}

Expand Down

0 comments on commit 8a9b8fd

Please sign in to comment.