Skip to content

Commit

Permalink
Revert "[Media Session] Convert interface from mojo to CPP"
Browse files Browse the repository at this point in the history
This reverts commit 530c8fd.

Reason for revert: caused tree closer due to missing mojom file: https://ci.chromium.org/p/chromium/builders/ci/chromeos-arm-generic-rel/39820?

Original change's description:
> [Media Session] Convert interface from mojo to CPP
>
> Context: https://crrev.com/c/2485850
>
> This converts the MSS interface to CPP since it does
> not need to be mojo.
>
> BUG=1140215
>
> Change-Id: I595bef21cddf755bc0fb423f87db9e620227d3e6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503393
> Commit-Queue: Becca Hughes <beccahughes@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#823358}

TBR=avi@chromium.org,xiyuan@chromium.org,dcheng@chromium.org,beccahughes@chromium.org

Change-Id: I729b898ef30c220c779be615b0b6eb6abf8cfed5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1140215
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515502
Reviewed-by: Meredith Lane <meredithl@chromium.org>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823379}
  • Loading branch information
Meredith Lane authored and Commit Bot committed Nov 3, 2020
1 parent 208c819 commit fc7b628
Show file tree
Hide file tree
Showing 22 changed files with 119 additions and 97 deletions.
4 changes: 2 additions & 2 deletions ash/ambient/ui/media_string_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
#include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "components/prefs/pref_service.h"
#include "services/media_session/public/cpp/media_session_service.h"
#include "services/media_session/public/mojom/media_session.mojom.h"
#include "services/media_session/public/mojom/media_session_service.mojom.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/compositor/layer.h"
#include "ui/compositor/paint_recorder.h"
Expand Down Expand Up @@ -242,7 +242,7 @@ void MediaStringView::InitLayout() {
}

void MediaStringView::BindMediaControllerObserver() {
media_session::MediaSessionService* service =
media_session::mojom::MediaSessionService* service =
Shell::Get()->shell_delegate()->GetMediaSessionService();
// Service might be unavailable under some test environments.
if (!service)
Expand Down
2 changes: 1 addition & 1 deletion ash/login/ui/lock_screen_media_controls_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ LockScreenMediaControlsView::LockScreenMediaControlsView(
SetArtwork(base::nullopt);

// |service| can be null in tests.
media_session::MediaSessionService* service =
media_session::mojom::MediaSessionService* service =
Shell::Get()->shell_delegate()->GetMediaSessionService();
if (!service)
return;
Expand Down
2 changes: 1 addition & 1 deletion ash/media/media_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ MediaControllerImpl::GetMediaSessionController() {
if (!Shell::HasInstance())
return nullptr;

media_session::MediaSessionService* service =
media_session::mojom::MediaSessionService* service =
Shell::Get()->shell_delegate()->GetMediaSessionService();
if (!service)
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion ash/media/media_notification_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ MediaNotificationControllerImpl::MediaNotificationControllerImpl()
}

// May be null in tests.
media_session::MediaSessionService* service =
media_session::mojom::MediaSessionService* service =
Shell::Get()->shell_delegate()->GetMediaSessionService();
if (!service)
return;
Expand Down
3 changes: 2 additions & 1 deletion ash/shell_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ aura::Window* ShellDelegate::CreateBrowserForTabDrop(
return nullptr;
}

media_session::MediaSessionService* ShellDelegate::GetMediaSessionService() {
media_session::mojom::MediaSessionService*
ShellDelegate::GetMediaSessionService() {
return nullptr;
}

Expand Down
4 changes: 2 additions & 2 deletions ash/shell_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "services/device/public/mojom/bluetooth_system.mojom-forward.h"
#include "services/device/public/mojom/fingerprint.mojom-forward.h"
#include "services/media_session/public/cpp/media_session_service.h"
#include "services/media_session/public/mojom/media_session_service.mojom-forward.h"
#include "ui/gfx/native_widget_types.h"

namespace aura {
Expand Down Expand Up @@ -104,7 +104,7 @@ class ASH_EXPORT ShellDelegate {

// Returns an interface to the Media Session service, or null if not
// available.
virtual media_session::MediaSessionService* GetMediaSessionService();
virtual media_session::mojom::MediaSessionService* GetMediaSessionService();

virtual void OpenKeyboardShortcutHelpPage() const {}
};
Expand Down
2 changes: 1 addition & 1 deletion ash/system/media/unified_media_controls_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ constexpr base::TimeDelta kHideArtworkDelay =
UnifiedMediaControlsController::UnifiedMediaControlsController(
Delegate* delegate)
: delegate_(delegate) {
media_session::MediaSessionService* service =
media_session::mojom::MediaSessionService* service =
Shell::Get()->shell_delegate()->GetMediaSessionService();
// Happens in test.
if (!service)
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/ash/chrome_shell_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ void ChromeShellDelegate::BindMultiDeviceSetup(
service->BindMultiDeviceSetup(std::move(receiver));
}

media_session::MediaSessionService*
media_session::mojom::MediaSessionService*
ChromeShellDelegate::GetMediaSessionService() {
return &content::GetMediaSessionService();
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/ash/chrome_shell_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class ChromeShellDelegate : public ash::ShellDelegate {
mojo::PendingReceiver<
chromeos::multidevice_setup::mojom::MultiDeviceSetup> receiver)
override;
media_session::MediaSessionService* GetMediaSessionService() override;
media_session::mojom::MediaSessionService* GetMediaSessionService() override;
std::unique_ptr<ash::NearbyShareDelegate> CreateNearbyShareDelegate(
ash::NearbyShareController* controller) const override;

Expand Down
3 changes: 3 additions & 0 deletions content/browser/media/session/media_session_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ TEST_F(MediaSessionImplTest, SessionInfoState) {
MockMediaSessionMojoObserver observer(*GetMediaSession());
GetMediaSession()->StartDucking();
observer.WaitForState(MediaSessionInfo::SessionState::kDucking);

EXPECT_TRUE(observer.session_info().Equals(
media_session::test::GetMediaSessionInfoSync(GetMediaSession())));
}

{
Expand Down
28 changes: 24 additions & 4 deletions content/browser/media_session/media_session_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,36 @@
#include "base/threading/sequence_local_storage_slot.h"
#include "content/public/browser/browser_thread.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/media_session/media_session_service_impl.h"
#include "services/media_session/media_session_service.h"
#include "services/media_session/public/cpp/features.h"

namespace content {

media_session::MediaSessionService& GetMediaSessionService() {
media_session::mojom::MediaSessionService& GetMediaSessionService() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

static base::NoDestructor<media_session::MediaSessionServiceImpl> service;
return *service;
// NOTE: We use sequence-local storage here strictly to limit the lifetime of
// this Remote to that of the UI-thread sequence. This ensures that it doesn't
// persist when the task environment is torn down and reinitialized e.g.
// between unit tests.
static base::NoDestructor<base::SequenceLocalStorageSlot<
mojo::Remote<media_session::mojom::MediaSessionService>>>
remote_slot;
auto& remote = remote_slot->GetOrCreateValue();
if (!remote) {
if (base::FeatureList::IsEnabled(
media_session::features::kMediaSessionService)) {
static base::NoDestructor<
std::unique_ptr<media_session::MediaSessionService>>
service;
*service = std::make_unique<media_session::MediaSessionService>(
remote.BindNewPipeAndPassReceiver());
} else {
// If the service is not enabled, bind to a disconnected pipe.
ignore_result(remote.BindNewPipeAndPassReceiver());
}
}
return *remote.get();
}

} // namespace content
5 changes: 3 additions & 2 deletions content/public/browser/media_session_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
#define CONTENT_PUBLIC_BROWSER_MEDIA_SESSION_SERVICE_H_

#include "content/common/content_export.h"
#include "services/media_session/public/cpp/media_session_service.h"
#include "services/media_session/public/mojom/media_session_service.mojom.h"

namespace content {

// Returns the main control interface into the Media Session Service which runs
// in the browser process.
CONTENT_EXPORT media_session::MediaSessionService& GetMediaSessionService();
CONTENT_EXPORT media_session::mojom::MediaSessionService&
GetMediaSessionService();

} // namespace content

Expand Down
6 changes: 3 additions & 3 deletions services/media_session/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ source_set("lib") {
"audio_focus_request.h",
"media_controller.cc",
"media_controller.h",
"media_session_service_impl.cc",
"media_session_service_impl.h",
"media_session_service.cc",
"media_session_service.h",
]

configs += [ "//build/config/compiler:wexit_time_destructors" ]
Expand All @@ -36,7 +36,7 @@ source_set("tests") {
sources = [
"audio_focus_manager_unittest.cc",
"media_controller_unittest.cc",
"media_session_service_impl_unittest.cc",
"media_session_service_unittest.cc",
]

deps = [
Expand Down
18 changes: 11 additions & 7 deletions services/media_session/audio_focus_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
#include "build/chromeos_buildflags.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/media_session/audio_focus_request.h"
#include "services/media_session/media_session_service_impl.h"
#include "services/media_session/media_session_service.h"
#include "services/media_session/public/cpp/test/audio_focus_test_util.h"
#include "services/media_session/public/cpp/test/mock_media_session.h"
#include "services/media_session/public/mojom/audio_focus.mojom.h"
#include "services/media_session/public/mojom/media_session.mojom.h"
#include "services/media_session/public/mojom/media_session_service.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace media_session {
Expand All @@ -47,12 +48,13 @@ class AudioFocusManagerTest
base::PowerMonitor::Initialize(std::move(power_source));

// Create an instance of the MediaSessionService.
service_ = std::make_unique<MediaSessionServiceImpl>();
service_->BindAudioFocusManager(
service_ = std::make_unique<MediaSessionService>(
service_remote_.BindNewPipeAndPassReceiver());
service_remote_->BindAudioFocusManager(
audio_focus_remote_.BindNewPipeAndPassReceiver());
service_->BindAudioFocusManagerDebug(
service_remote_->BindAudioFocusManagerDebug(
audio_focus_debug_remote_.BindNewPipeAndPassReceiver());
service_->BindMediaControllerManager(
service_remote_->BindMediaControllerManager(
controller_manager_remote_.BindNewPipeAndPassReceiver());

audio_focus_remote_->SetEnforcementMode(GetParam());
Expand All @@ -64,6 +66,7 @@ class AudioFocusManagerTest
base::RunLoop().RunUntilIdle();

service_.reset();
service_remote_.reset();
base::PowerMonitor::ShutdownForTesting();
}

Expand Down Expand Up @@ -181,7 +184,7 @@ class AudioFocusManagerTest

mojo::Remote<mojom::AudioFocusManager> CreateAudioFocusManagerRemote() {
mojo::Remote<mojom::AudioFocusManager> remote;
service_->BindAudioFocusManager(remote.BindNewPipeAndPassReceiver());
service_remote_->BindAudioFocusManager(remote.BindNewPipeAndPassReceiver());
return remote;
}

Expand Down Expand Up @@ -279,7 +282,8 @@ class AudioFocusManagerTest

base::test::TaskEnvironment task_environment_;

std::unique_ptr<MediaSessionServiceImpl> service_;
std::unique_ptr<MediaSessionService> service_;
mojo::Remote<mojom::MediaSessionService> service_remote_;

mojo::Remote<mojom::AudioFocusManager> audio_focus_remote_;
mojo::Remote<mojom::AudioFocusManagerDebug> audio_focus_debug_remote_;
Expand Down
11 changes: 7 additions & 4 deletions services/media_session/media_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@
#include "base/time/time.h"
#include "base/unguessable_token.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/media_session/media_session_service_impl.h"
#include "services/media_session/media_session_service.h"
#include "services/media_session/public/cpp/media_metadata.h"
#include "services/media_session/public/cpp/test/mock_media_session.h"
#include "services/media_session/public/cpp/test/test_media_controller.h"
#include "services/media_session/public/mojom/constants.mojom.h"
#include "services/media_session/public/mojom/media_session.mojom.h"
#include "services/media_session/public/mojom/media_session_service.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace media_session {
Expand All @@ -30,10 +31,11 @@ class MediaControllerTest : public testing::Test {

void SetUp() override {
// Create an instance of the MediaSessionService and bind some interfaces.
service_ = std::make_unique<MediaSessionServiceImpl>();
service_->BindAudioFocusManager(
service_ = std::make_unique<MediaSessionService>(
service_remote_.BindNewPipeAndPassReceiver());
service_remote_->BindAudioFocusManager(
audio_focus_remote_.BindNewPipeAndPassReceiver());
service_->BindMediaControllerManager(
service_remote_->BindMediaControllerManager(
controller_manager_remote_.BindNewPipeAndPassReceiver());

controller_manager_remote_->CreateActiveMediaController(
Expand Down Expand Up @@ -70,6 +72,7 @@ class MediaControllerTest : public testing::Test {
private:
base::test::TaskEnvironment task_environment_;
std::unique_ptr<MediaSessionService> service_;
mojo::Remote<mojom::MediaSessionService> service_remote_;
mojo::Remote<mojom::AudioFocusManager> audio_focus_remote_;
mojo::Remote<mojom::MediaController> media_controller_remote_;
mojo::Remote<mojom::MediaControllerManager> controller_manager_remote_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,31 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "services/media_session/media_session_service_impl.h"
#include "services/media_session/media_session_service.h"

#include "base/bind.h"
#include "services/media_session/audio_focus_manager.h"

namespace media_session {

MediaSessionServiceImpl::MediaSessionServiceImpl()
: audio_focus_manager_(std::make_unique<AudioFocusManager>()) {}
MediaSessionService::MediaSessionService(
mojo::PendingReceiver<mojom::MediaSessionService> receiver)
: receiver_(this, std::move(receiver)),
audio_focus_manager_(std::make_unique<AudioFocusManager>()) {}

MediaSessionServiceImpl::~MediaSessionServiceImpl() = default;
MediaSessionService::~MediaSessionService() = default;

void MediaSessionServiceImpl::BindAudioFocusManager(
void MediaSessionService::BindAudioFocusManager(
mojo::PendingReceiver<mojom::AudioFocusManager> receiver) {
audio_focus_manager_->BindToInterface(std::move(receiver));
}

void MediaSessionServiceImpl::BindAudioFocusManagerDebug(
void MediaSessionService::BindAudioFocusManagerDebug(
mojo::PendingReceiver<mojom::AudioFocusManagerDebug> receiver) {
audio_focus_manager_->BindToDebugInterface(std::move(receiver));
}

void MediaSessionServiceImpl::BindMediaControllerManager(
void MediaSessionService::BindMediaControllerManager(
mojo::PendingReceiver<mojom::MediaControllerManager> receiver) {
audio_focus_manager_->BindToControllerManagerInterface(std::move(receiver));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,46 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef SERVICES_MEDIA_SESSION_MEDIA_SESSION_SERVICE_IMPL_H_
#define SERVICES_MEDIA_SESSION_MEDIA_SESSION_SERVICE_IMPL_H_
#ifndef SERVICES_MEDIA_SESSION_MEDIA_SESSION_SERVICE_H_
#define SERVICES_MEDIA_SESSION_MEDIA_SESSION_SERVICE_H_

#include <memory>
#include <string>

#include "base/macros.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "services/media_session/public/cpp/media_session_service.h"
#include "services/media_session/public/mojom/media_session_service.mojom.h"

namespace media_session {

class AudioFocusManager;

class MediaSessionServiceImpl : public MediaSessionService {
class MediaSessionService : public mojom::MediaSessionService {
public:
MediaSessionServiceImpl();
~MediaSessionServiceImpl() override;
MediaSessionServiceImpl(const MediaSessionServiceImpl&) = delete;
MediaSessionServiceImpl& operator=(const MediaSessionServiceImpl&) = delete;
explicit MediaSessionService(
mojo::PendingReceiver<mojom::MediaSessionService> receiver);
~MediaSessionService() override;

// MediaSessionService implementation:
const AudioFocusManager& audio_focus_manager_for_testing() const {
return *audio_focus_manager_.get();
}

private:
// mojom::MediaSessionService implementation:
void BindAudioFocusManager(
mojo::PendingReceiver<mojom::AudioFocusManager> receiver) override;
void BindAudioFocusManagerDebug(
mojo::PendingReceiver<mojom::AudioFocusManagerDebug> receiver) override;
void BindMediaControllerManager(
mojo::PendingReceiver<mojom::MediaControllerManager> receiver) override;

const AudioFocusManager& audio_focus_manager_for_testing() const {
return *audio_focus_manager_.get();
}

private:
mojo::Receiver<mojom::MediaSessionService> receiver_;
std::unique_ptr<AudioFocusManager> audio_focus_manager_;

DISALLOW_COPY_AND_ASSIGN(MediaSessionService);
};

} // namespace media_session

#endif // SERVICES_MEDIA_SESSION_MEDIA_SESSION_SERVICE_IMPL_H_
#endif // SERVICES_MEDIA_SESSION_MEDIA_SESSION_SERVICE_H_
Loading

0 comments on commit fc7b628

Please sign in to comment.