Skip to content

Commit

Permalink
Revert "[Media Session] Fix for Lacros"
Browse files Browse the repository at this point in the history
This reverts commit ddb531f.

Reason for revert: speculative fix of crbug.com/1146633

Original change's description:
> [Media Session] Fix for Lacros
>
> At the moment in Lacros there are two instances of the
> Media Session Service (MSS) one hosted in ash-chrome and
> the other in lacros-chrome. This means that sessions in
> either browser process cannot see sessions from the other.
>
> This changes it so when a client asks for lacros-chrome
> MSS instead of providing a locally hosted MSS it binds
> the MSS in ash-chrome.
>
> BUG=1140215
>
> Change-Id: Ic155a6cd928f0735095f849f309f9075dbb1d36f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485850
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Tommy Steimel <steimel@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Commit-Queue: Becca Hughes <beccahughes@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825016}

TBR=avi@chromium.org,dcheng@chromium.org,erikchen@chromium.org,beccahughes@chromium.org,steimel@chromium.org

Change-Id: Iff68cde46d811d466f142738b694c11f1ae332e3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1140215, 1146633
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2522562
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825139}
  • Loading branch information
xiaochengh authored and Commit Bot committed Nov 7, 2020
1 parent a4995f4 commit 0482e51
Show file tree
Hide file tree
Showing 18 changed files with 42 additions and 357 deletions.
20 changes: 0 additions & 20 deletions chrome/browser/chromeos/crosapi/ash_chrome_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "chromeos/crosapi/mojom/select_file.mojom.h"
#include "components/user_manager/user_manager.h"
#include "content/public/browser/device_service.h"
#include "content/public/browser/media_session_service.h"

namespace crosapi {

Expand Down Expand Up @@ -115,25 +114,6 @@ void AshChromeServiceImpl::BindFeedback(
feedback_ash_ = std::make_unique<FeedbackAsh>(std::move(receiver));
}

void AshChromeServiceImpl::BindMediaSessionController(
mojo::PendingReceiver<media_session::mojom::MediaControllerManager>
receiver) {
content::GetMediaSessionService().BindMediaControllerManager(
std::move(receiver));
}

void AshChromeServiceImpl::BindMediaSessionAudioFocus(
mojo::PendingReceiver<media_session::mojom::AudioFocusManager> receiver) {
content::GetMediaSessionService().BindAudioFocusManager(std::move(receiver));
}

void AshChromeServiceImpl::BindMediaSessionAudioFocusDebug(
mojo::PendingReceiver<media_session::mojom::AudioFocusManagerDebug>
receiver) {
content::GetMediaSessionService().BindAudioFocusManagerDebug(
std::move(receiver));
}

void AshChromeServiceImpl::OnLacrosStartup(mojom::LacrosInfoPtr lacros_info) {
BrowserManager::Get()->set_lacros_version(lacros_info->lacros_version);
}
Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/chromeos/crosapi/ash_chrome_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,6 @@ class AshChromeServiceImpl : public mojom::AshChromeService {
mojo::PendingReceiver<device::mojom::HidManager> receiver) override;
void BindFeedback(mojo::PendingReceiver<mojom::Feedback> receiver) override;
void OnLacrosStartup(mojom::LacrosInfoPtr lacros_info) override;
void BindMediaSessionController(
mojo::PendingReceiver<media_session::mojom::MediaControllerManager>
receiver) override;
void BindMediaSessionAudioFocus(
mojo::PendingReceiver<media_session::mojom::AudioFocusManager> receiver)
override;
void BindMediaSessionAudioFocusDebug(
mojo::PendingReceiver<media_session::mojom::AudioFocusManagerDebug>
receiver) override;

private:
mojo::Receiver<mojom::AshChromeService> receiver_;
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/chromeos/crosapi/browser_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ bool IsLacrosWindow(const aura::Window* window) {

base::flat_map<base::Token, uint32_t> GetInterfaceVersions() {
static_assert(
crosapi::mojom::AshChromeService::Version_ == 6,
crosapi::mojom::AshChromeService::Version_ == 5,
"if you add a new crosapi, please add it to the version map here");
InterfaceVersions versions;
AddVersion<crosapi::mojom::AccountManager>(&versions);
Expand All @@ -162,9 +162,6 @@ base::flat_map<base::Token, uint32_t> GetInterfaceVersions() {
AddVersion<crosapi::mojom::SnapshotCapturer>(&versions);
AddVersion<device::mojom::HidConnection>(&versions);
AddVersion<device::mojom::HidManager>(&versions);
AddVersion<media_session::mojom::MediaControllerManager>(&versions);
AddVersion<media_session::mojom::AudioFocusManager>(&versions);
AddVersion<media_session::mojom::AudioFocusManagerDebug>(&versions);
return versions;
}

Expand Down
45 changes: 0 additions & 45 deletions chrome/browser/lacros/media_session_lacros_browsertest.cc

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/media/router/chrome_media_router_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/global_media_controls/media_toolbar_button_observer.h"
Expand Down Expand Up @@ -36,9 +35,6 @@

using media_session::mojom::MediaSessionAction;

// Global Media Controls are not supported on Chrome OS.
#if !BUILDFLAG(IS_LACROS)

namespace {

class MediaToolbarButtonWatcher : public MediaToolbarButtonObserver,
Expand Down Expand Up @@ -898,5 +894,3 @@ IN_PROC_BROWSER_TEST_F(MediaDialogViewBrowserTest, LiveCaption) {
EXPECT_FALSE(
browser()->profile()->GetPrefs()->GetBoolean(prefs::kLiveCaptionEnabled));
}

#endif // !BUILDFLAG(IS_LACROS)
3 changes: 0 additions & 3 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3179,7 +3179,6 @@ if (chromeos_is_browser_only) {
sources = [
"../browser/lacros/file_manager_lacros_browsertest.cc",
"../browser/lacros/keystore_service_lacros_browsertest.cc",
"../browser/lacros/media_session_lacros_browsertest.cc",
"../browser/lacros/message_center_lacros_browsertest.cc",
"../browser/lacros/screen_manager_lacros_browsertest.cc",
]
Expand All @@ -3189,8 +3188,6 @@ if (chromeos_is_browser_only) {
":test_support",
]

data = [ "data/media/" ]

data_deps = [ "//chrome:packed_resources" ]
}
}
Expand Down
1 change: 0 additions & 1 deletion chromeos/crosapi/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ mojom("mojom") {
public_deps = [
"//mojo/public/mojom/base",
"//services/device/public/mojom:mojom",
"//services/media_session/public/mojom:mojom",
"//ui/gfx/image/mojom",
"//url/mojom:url_mojom_gurl",
]
Expand Down
24 changes: 2 additions & 22 deletions chromeos/crosapi/mojom/crosapi.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import "mojo/public/mojom/base/big_string.mojom";
import "mojo/public/mojom/base/token.mojom";
import "mojo/public/mojom/base/values.mojom";
import "services/device/public/mojom/hid.mojom";
import "services/media_session/public/mojom/audio_focus.mojom";
import "services/media_session/public/mojom/media_controller.mojom";
import "url/mojom/url.mojom";

// LacrosInfo is a set of parameters passed to ash from lacros-chrome
Expand All @@ -38,8 +36,8 @@ struct LacrosInfo {
// milestone when you added it, to help us reason about compatibility between
// lacros-chrome and older ash-chrome binaries.
//
// Next version: 7
// Next method id: 12
// Next version: 6
// Next method id: 9
[Stable, Uuid="8b79c34f-2bf8-4499-979a-b17cac522c1e"]
interface AshChromeService {
// Binds Chrome OS Account Manager for Identity management.
Expand Down Expand Up @@ -77,24 +75,6 @@ interface AshChromeService {
// Added in M87.
[MinVersion=3] BindFeedback@5(pending_receiver<Feedback> receiver);

// Binds the Media Session service (controller) for enabling media playback
// control.
// Added in M88.
[MinVersion=6] BindMediaSessionController@9(
pending_receiver<media_session.mojom.MediaControllerManager> receiver);

// Binds the Media Session service (audio focus) for enabling media sessions
// to register with the service so they can be controlled.
// Added in M88.
[MinVersion=6] BindMediaSessionAudioFocus@10(
pending_receiver<media_session.mojom.AudioFocusManager> receiver);

// Binds the Media Session service (audio focus debug) for enabling debugging
// of media playback sessions.
// Added in M88.
[MinVersion=6] BindMediaSessionAudioFocusDebug@11(
pending_receiver<media_session.mojom.AudioFocusManagerDebug> receiver);

// Passes generic lacros information such as lacros version, etc into ash
// in |lacros_info| during startup.
// Added in M87.
Expand Down
71 changes: 0 additions & 71 deletions chromeos/lacros/lacros_chrome_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,30 +180,6 @@ class LacrosChromeServiceNeverBlockingState
ash_chrome_service_->BindFileManager(std::move(pending_receiver));
}

void BindMediaSessionAudioFocusReceiver(
mojo::PendingReceiver<media_session::mojom::AudioFocusManager>
pending_receiver) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
ash_chrome_service_->BindMediaSessionAudioFocus(
std::move(pending_receiver));
}

void BindMediaSessionAudioFocusDebugReceiver(
mojo::PendingReceiver<media_session::mojom::AudioFocusManagerDebug>
pending_receiver) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
ash_chrome_service_->BindMediaSessionAudioFocusDebug(
std::move(pending_receiver));
}

void BindMediaSessionControllerReceiver(
mojo::PendingReceiver<media_session::mojom::MediaControllerManager>
pending_receiver) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
ash_chrome_service_->BindMediaSessionController(
std::move(pending_receiver));
}

base::WeakPtr<LacrosChromeServiceNeverBlockingState> GetWeakPtr() {
return weak_factory_.GetWeakPtr();
}
Expand Down Expand Up @@ -421,53 +397,6 @@ bool LacrosChromeServiceImpl::IsScreenManagerAvailable() {
return AshChromeServiceVersion() >= 0;
}

bool LacrosChromeServiceImpl::IsMediaSessionAudioFocusAvailable() {
return AshChromeServiceVersion() >= 6;
}

void LacrosChromeServiceImpl::BindAudioFocusManager(
mojo::PendingReceiver<media_session::mojom::AudioFocusManager> remote) {
DCHECK_CALLED_ON_VALID_SEQUENCE(affine_sequence_checker_);
DCHECK(IsMediaSessionAudioFocusAvailable());

never_blocking_sequence_->PostTask(
FROM_HERE, base::BindOnce(&LacrosChromeServiceNeverBlockingState::
BindMediaSessionAudioFocusReceiver,
weak_sequenced_state_, std::move(remote)));
}

bool LacrosChromeServiceImpl::IsMediaSessionAudioFocusDebugAvailable() {
return AshChromeServiceVersion() >= 6;
}

void LacrosChromeServiceImpl::BindAudioFocusManagerDebug(
mojo::PendingReceiver<media_session::mojom::AudioFocusManagerDebug>
remote) {
DCHECK_CALLED_ON_VALID_SEQUENCE(affine_sequence_checker_);
DCHECK(IsMediaSessionAudioFocusAvailable());

never_blocking_sequence_->PostTask(
FROM_HERE, base::BindOnce(&LacrosChromeServiceNeverBlockingState::
BindMediaSessionAudioFocusDebugReceiver,
weak_sequenced_state_, std::move(remote)));
}

bool LacrosChromeServiceImpl::IsMediaSessionControllerAvailable() {
return AshChromeServiceVersion() >= 6;
}

void LacrosChromeServiceImpl::BindMediaControllerManager(
mojo::PendingReceiver<media_session::mojom::MediaControllerManager>
remote) {
DCHECK_CALLED_ON_VALID_SEQUENCE(affine_sequence_checker_);
DCHECK(IsMediaSessionAudioFocusAvailable());

never_blocking_sequence_->PostTask(
FROM_HERE, base::BindOnce(&LacrosChromeServiceNeverBlockingState::
BindMediaSessionControllerReceiver,
weak_sequenced_state_, std::move(remote)));
}

bool LacrosChromeServiceImpl::IsOnLacrosStartupAvailable() {
return AshChromeServiceVersion() >= 3;
}
Expand Down
26 changes: 0 additions & 26 deletions chromeos/lacros/lacros_chrome_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,32 +135,6 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
return feedback_remote_;
}

// media_session_audio_focus_remote() can only be used when this method
// returns true;
bool IsMediaSessionAudioFocusAvailable();

// This must be called on the affine sequence.
void BindAudioFocusManager(
mojo::PendingReceiver<media_session::mojom::AudioFocusManager> remote);

// media_session_audio_focus_debug_remote() can only be used when this method
// returns true;
bool IsMediaSessionAudioFocusDebugAvailable();

// This must be called on the affine sequence.
void BindAudioFocusManagerDebug(
mojo::PendingReceiver<media_session::mojom::AudioFocusManagerDebug>
remote);

// media_session_controller_remote() can only be used when this method returns
// true;
bool IsMediaSessionControllerAvailable();

// This must be called on the affine sequence.
void BindMediaControllerManager(
mojo::PendingReceiver<media_session::mojom::MediaControllerManager>
remote);

// account_manager_remote() can only be used if this method returns true.
bool IsAccountManagerAvailable();

Expand Down
Loading

0 comments on commit 0482e51

Please sign in to comment.