Commit 1e7c1af4 authored by Becca Hughes's avatar Becca Hughes Committed by Commit Bot

[Media Session] Remove MediaSessionObserver

Remove content::MediaSessionObserver in favour of the
mojo observer. Also simplifies some of the state logic
in MediaSessionImpl now that the legacy observers have
been removed.

BUG=925406

Change-Id: Ia275dd3d1ebe3fb3e5acbbf610258b710f12f3ff
Reviewed-on: https://chromium-review.googlesource.com/c/1444013Reviewed-by: default avatarSean Topping <seantopping@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627699}
parent 874554ad
......@@ -37,10 +37,8 @@ class MockMediaSession : public content::MediaSession {
MOCK_METHOD1(SetDuckingVolumeMultiplier, void(double));
MOCK_METHOD1(DidReceiveAction,
void(media_session::mojom::MediaSessionAction));
MOCK_METHOD1(AddObserver, void(content::MediaSessionObserver*));
MOCK_METHOD1(AddObserver,
void(media_session::mojom::MediaSessionObserverPtr));
MOCK_METHOD1(RemoveObserver, void(content::MediaSessionObserver*));
MOCK_METHOD1(GetMediaSessionInfo, void(GetMediaSessionInfoCallback));
MOCK_METHOD1(GetDebugInfo, void(GetDebugInfoCallback));
MOCK_METHOD0(PreviousTrack, void());
......
......@@ -12,17 +12,14 @@
#include <unordered_map>
#include <unordered_set>
#include "base/callback_list.h"
#include "base/containers/id_map.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "base/optional.h"
#include "base/timer/timer.h"
#include "content/browser/media/session/audio_focus_delegate.h"
#include "content/browser/media/session/media_session_uma_helper.h"
#include "content/common/content_export.h"
#include "content/public/browser/media_session.h"
#include "content/public/browser/media_session_observer.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"
#include "mojo/public/cpp/bindings/binding.h"
......@@ -50,7 +47,6 @@ class AudioFocusManagerTest;
class MediaSessionImplServiceRoutingTest;
class MediaSessionImplStateObserver;
class MediaSessionImplVisibilityBrowserTest;
class MediaSessionObserver;
class MediaSessionPlayerObserver;
class MediaSessionServiceImpl;
class MediaSessionServiceImplBrowserTest;
......@@ -121,9 +117,6 @@ class MediaSessionImpl : public MediaSession,
CONTENT_EXPORT void OnPlayerPaused(MediaSessionPlayerObserver* observer,
int player_id);
void AddObserver(MediaSessionObserver* observer) override;
void RemoveObserver(MediaSessionObserver* observer) override;
// Returns if the session is currently active.
CONTENT_EXPORT bool IsActive() const;
......@@ -168,15 +161,12 @@ class MediaSessionImpl : public MediaSession,
mojo::InterfaceRequest<media_session::mojom::MediaSession> request);
// Returns information about the MediaSession.
media_session::mojom::MediaSessionInfoPtr GetMediaSessionInfoSync();
CONTENT_EXPORT media_session::mojom::MediaSessionInfoPtr
GetMediaSessionInfoSync();
// Returns if the session can be controlled by the user.
CONTENT_EXPORT bool IsControllable() const;
// Compute if the actual playback state is paused by combining the
// MediaSessionService declared state and guessed state (audio_focus_state_).
CONTENT_EXPORT bool IsActuallyPaused() const;
// MediaSession overrides ---------------------------------------------------
// Resume the media session.
......@@ -299,13 +289,6 @@ class MediaSessionImpl : public MediaSession,
// delegate to abandon the audio focus.
CONTENT_EXPORT void AbandonSystemAudioFocusIfNeeded();
// Notify all information that an observer needs to know when it's added.
void NotifyAddedObserver(MediaSessionObserver* observer);
// Notifies legacy (non-mojo) observers about the state change of the media
// session.
void NotifyLegacyObserversStateChange();
// Internal method that should be used instead of setting audio_focus_state_.
// It sets audio_focus_state_ and notifies observers about the state change.
void SetAudioFocusState(State audio_focus_state);
......@@ -313,9 +296,8 @@ class MediaSessionImpl : public MediaSession,
// Flushes any mojo bindings for testing.
CONTENT_EXPORT void FlushForTesting();
// Notifies mojo observers that the MediaSessionInfo has changed.
void OnMediaSessionInfoChanged();
void NotifyMojoObserversMediaSessionInfoChanged();
// Notifies |observers_| and |delegate_| that |MediaSessionInfo| has changed.
void RebuildAndNotifyMediaSessionInfoChanged();
// Update the volume multiplier when ducking state changes.
void UpdateVolumeMultiplier();
......@@ -324,11 +306,6 @@ class MediaSessionImpl : public MediaSession,
// ducking.
double GetVolumeMultiplier() const;
// Registers a MediaSessionImpl state change callback.
CONTENT_EXPORT std::unique_ptr<base::CallbackList<void(State)>::Subscription>
RegisterMediaSessionStateChangedCallbackForTest(
const StateChangedCallback& cb);
CONTENT_EXPORT bool AddPepperPlayer(MediaSessionPlayerObserver* observer,
int player_id);
......@@ -376,6 +353,9 @@ class MediaSessionImpl : public MediaSession,
// we request system audio focus.
media_session::mojom::AudioFocusType desired_audio_focus_type_;
// The last updated |MediaSessionInfo| that was sent to |observers_|.
media_session::mojom::MediaSessionInfoPtr session_info_;
MediaSessionUmaHelper uma_helper_;
// The ducking state of this media session. The initial value is |false|, and
......@@ -387,10 +367,6 @@ class MediaSessionImpl : public MediaSession,
double ducking_volume_multiplier_;
base::CallbackList<void(State)> media_session_state_listeners_;
base::ObserverList<MediaSessionObserver>::Unchecked observers_;
#if defined(OS_ANDROID)
std::unique_ptr<MediaSessionAndroid> session_android_;
#endif // defined(OS_ANDROID)
......@@ -408,11 +384,7 @@ class MediaSessionImpl : public MediaSession,
// Bindings for Mojo pointers to |this| held by media route providers.
mojo::BindingSet<media_session::mojom::MediaSession> bindings_;
mojo::InterfacePtrSet<media_session::mojom::MediaSessionObserver>
mojo_observers_;
// Timer used for debouncing MediaSessionInfoChanged events.
std::unique_ptr<base::OneShotTimer> info_changed_timer_;
mojo::InterfacePtrSet<media_session::mojom::MediaSessionObserver> observers_;
WEB_CONTENTS_USER_DATA_KEY_DECL();
......
......@@ -33,7 +33,6 @@
using content::WebContents;
using content::MediaSession;
using content::MediaSessionImpl;
using content::MediaSessionObserver;
using content::AudioFocusDelegate;
using content::MediaSessionPlayerObserver;
using content::MediaSessionUmaHelper;
......
......@@ -126,12 +126,12 @@ class MediaSessionImplTest : public RenderViewHostTestHarness {
return media_session::test::GetMediaSessionInfoSync(session)->state;
}
void ClearMojoObservers(MediaSessionImpl* session) {
session->mojo_observers_.CloseAll();
void ClearObservers(MediaSessionImpl* session) {
session->observers_.CloseAll();
}
bool HasMojoObservers(MediaSessionImpl* session) {
return !session->mojo_observers_.empty();
bool HasObservers(MediaSessionImpl* session) {
return !session->observers_.empty();
}
void FlushForTesting(MediaSessionImpl* session) {
......@@ -305,17 +305,17 @@ TEST_F(MediaSessionImplTest, PepperForcesDuckAndRequestsFocus) {
EXPECT_FALSE(GetForceDuck(GetMediaSession()));
}
TEST_F(MediaSessionImplTest, RegisterMojoObserver) {
TEST_F(MediaSessionImplTest, RegisterObserver) {
// There is no way to get the number of mojo observers so we should just
// remove them all and check if the mojo observers interface ptr set is
// empty or not.
ClearMojoObservers(GetMediaSession());
EXPECT_FALSE(HasMojoObservers(GetMediaSession()));
ClearObservers(GetMediaSession());
EXPECT_FALSE(HasObservers(GetMediaSession()));
MockMediaSessionMojoObserver observer(*GetMediaSession());
FlushForTesting(GetMediaSession());
EXPECT_TRUE(HasMojoObservers(GetMediaSession()));
EXPECT_TRUE(HasObservers(GetMediaSession()));
}
TEST_F(MediaSessionImplTest, SessionInfo_PlaybackState) {
......
......@@ -22,9 +22,13 @@
#include "content/shell/browser/shell.h"
#include "media/base/media_switches.h"
#include "services/media_session/public/cpp/features.h"
#include "services/media_session/public/cpp/test/mock_media_session.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace content {
using media_session::mojom::MediaSessionInfo;
namespace {
static const char kStartPlayerScript[] =
"document.getElementById('long-video').play()";
......@@ -41,17 +45,11 @@ enum class BackgroundResuming {
DISABLED,
};
enum class SessionState {
ACTIVE,
SUSPENDED,
INACTIVE,
};
struct VisibilityTestData {
MediaSuspend media_suspend;
BackgroundResuming background_resuming;
SessionState session_state_before_hide;
SessionState session_state_after_hide;
MediaSessionInfo::SessionState session_state_before_hide;
MediaSessionInfo::SessionState session_state_after_hide;
};
}
......@@ -80,24 +78,6 @@ class MediaSessionImplVisibilityBrowserTest
ContentBrowserTest::SetUpOnMainThread();
web_contents_ = shell()->web_contents();
media_session_ = MediaSessionImpl::Get(web_contents_);
media_session_state_loop_runners_[MediaSessionImpl::State::ACTIVE] =
new MessageLoopRunner();
media_session_state_loop_runners_[MediaSessionImpl::State::SUSPENDED] =
new MessageLoopRunner();
media_session_state_loop_runners_[MediaSessionImpl::State::INACTIVE] =
new MessageLoopRunner();
media_session_state_callback_subscription_ =
media_session_->RegisterMediaSessionStateChangedCallbackForTest(
base::Bind(&MediaSessionImplVisibilityBrowserTest::
OnMediaSessionStateChanged,
base::Unretained(this)));
}
void TearDownOnMainThread() override {
// Unsubscribe the callback subscription before tearing down, so that the
// CallbackList in MediaSession will be empty when it is destroyed.
media_session_state_callback_subscription_.reset();
}
void EnableDisableResumingBackgroundVideos(bool enable) {
......@@ -126,52 +106,40 @@ class MediaSessionImplVisibilityBrowserTest
void StartPlayer() {
LoadTestPage();
LOG(INFO) << "Starting player";
ClearMediaSessionStateLoopRunners();
RunScript(kStartPlayerScript);
LOG(INFO) << "Waiting for session to be active";
WaitForMediaSessionState(MediaSessionImpl::State::ACTIVE);
WaitForMediaSessionState(MediaSessionInfo::SessionState::kActive);
}
// Maybe pause the player depending on whether the session state before hide
// is SUSPENDED.
void MaybePausePlayer() {
ASSERT_TRUE(GetVisibilityTestData().session_state_before_hide !=
SessionState::INACTIVE);
MediaSessionInfo::SessionState::kInactive);
if (GetVisibilityTestData().session_state_before_hide ==
SessionState::ACTIVE)
MediaSessionInfo::SessionState::kActive)
return;
LOG(INFO) << "Pausing player";
ClearMediaSessionStateLoopRunners();
RunScript(kPausePlayerScript);
LOG(INFO) << "Waiting for session to be suspended";
WaitForMediaSessionState(MediaSessionImpl::State::SUSPENDED);
WaitForMediaSessionState(MediaSessionInfo::SessionState::kSuspended);
}
void HideTab() {
LOG(INFO) << "Hiding the tab";
ClearMediaSessionStateLoopRunners();
web_contents_->WasHidden();
}
void CheckSessionStateAfterHide() {
MediaSessionImpl::State state_before_hide =
ToMediaSessionState(GetVisibilityTestData().session_state_before_hide);
MediaSessionImpl::State state_after_hide =
ToMediaSessionState(GetVisibilityTestData().session_state_after_hide);
MediaSessionInfo::SessionState state_before_hide =
GetVisibilityTestData().session_state_before_hide;
MediaSessionInfo::SessionState state_after_hide =
GetVisibilityTestData().session_state_after_hide;
if (state_before_hide == state_after_hide) {
LOG(INFO) << "Waiting for 1 second and check session state is unchanged";
Wait(base::TimeDelta::FromSeconds(1));
ASSERT_EQ(state_after_hide, media_session_->audio_focus_state_);
ASSERT_EQ(state_after_hide,
media_session_->GetMediaSessionInfoSync()->state);
} else {
LOG(INFO) << "Waiting for Session to change";
WaitForMediaSessionState(state_after_hide);
}
LOG(INFO) << "Test succeeded";
}
private:
......@@ -185,16 +153,6 @@ class MediaSessionImplVisibilityBrowserTest
ASSERT_TRUE(ExecuteScript(web_contents_->GetMainFrame(), script));
}
void ClearMediaSessionStateLoopRunners() {
for (auto& state_loop_runner : media_session_state_loop_runners_)
state_loop_runner.second = new MessageLoopRunner();
}
void OnMediaSessionStateChanged(MediaSessionImpl::State state) {
ASSERT_TRUE(media_session_state_loop_runners_.count(state));
media_session_state_loop_runners_[state]->Quit();
}
// TODO(zqzhang): This method is shared with
// MediaRouterIntegrationTests. Move it into a general place.
void Wait(base::TimeDelta timeout) {
......@@ -204,26 +162,9 @@ class MediaSessionImplVisibilityBrowserTest
run_loop.Run();
}
void WaitForMediaSessionState(MediaSessionImpl::State state) {
ASSERT_TRUE(media_session_state_loop_runners_.count(state));
media_session_state_loop_runners_[state]->Run();
}
MediaSessionImpl::State ToMediaSessionState(SessionState state) {
switch (state) {
case SessionState::ACTIVE:
return MediaSessionImpl::State::ACTIVE;
break;
case SessionState::SUSPENDED:
return MediaSessionImpl::State::SUSPENDED;
break;
case SessionState::INACTIVE:
return MediaSessionImpl::State::INACTIVE;
break;
default:
ADD_FAILURE() << "invalid SessionState to convert";
return MediaSessionImpl::State::INACTIVE;
}
void WaitForMediaSessionState(MediaSessionInfo::SessionState state) {
media_session::test::MockMediaSessionMojoObserver observer(*media_session_);
observer.WaitForState(state);
}
base::test::ScopedFeatureList ms_feature_list_;
......@@ -231,17 +172,6 @@ class MediaSessionImplVisibilityBrowserTest
WebContents* web_contents_;
MediaSessionImpl* media_session_;
// MessageLoopRunners for waiting MediaSession state to change. Note that the
// MessageLoopRunners can accept Quit() before calling Run(), thus the state
// change can still be captured before waiting. For example, the MediaSession
// might go active immediately after calling HTMLMediaElement.play(). A test
// can listen to the state change before calling play(), and then wait for the
// state change after play().
std::map<MediaSessionImpl::State, scoped_refptr<MessageLoopRunner>>
media_session_state_loop_runners_;
std::unique_ptr<
base::CallbackList<void(MediaSessionImpl::State)>::Subscription>
media_session_state_callback_subscription_;
DISALLOW_COPY_AND_ASSIGN(MediaSessionImplVisibilityBrowserTest);
};
......@@ -250,21 +180,29 @@ namespace {
VisibilityTestData kTestParams[] = {
{MediaSuspend::ENABLED, BackgroundResuming::DISABLED,
SessionState::SUSPENDED, SessionState::INACTIVE},
{MediaSuspend::ENABLED, BackgroundResuming::DISABLED, SessionState::ACTIVE,
SessionState::INACTIVE},
{MediaSuspend::ENABLED, BackgroundResuming::ENABLED, SessionState::ACTIVE,
SessionState::SUSPENDED},
MediaSessionInfo::SessionState::kSuspended,
MediaSessionInfo::SessionState::kInactive},
{MediaSuspend::ENABLED, BackgroundResuming::DISABLED,
MediaSessionInfo::SessionState::kActive,
MediaSessionInfo::SessionState::kInactive},
{MediaSuspend::ENABLED, BackgroundResuming::ENABLED,
MediaSessionInfo::SessionState::kActive,
MediaSessionInfo::SessionState::kSuspended},
{MediaSuspend::ENABLED, BackgroundResuming::ENABLED,
SessionState::SUSPENDED, SessionState::SUSPENDED},
MediaSessionInfo::SessionState::kSuspended,
MediaSessionInfo::SessionState::kSuspended},
{MediaSuspend::DISABLED, BackgroundResuming::DISABLED,
SessionState::SUSPENDED, SessionState::SUSPENDED},
{MediaSuspend::DISABLED, BackgroundResuming::DISABLED, SessionState::ACTIVE,
SessionState::ACTIVE},
{MediaSuspend::DISABLED, BackgroundResuming::ENABLED, SessionState::ACTIVE,
SessionState::ACTIVE},
MediaSessionInfo::SessionState::kSuspended,
MediaSessionInfo::SessionState::kSuspended},
{MediaSuspend::DISABLED, BackgroundResuming::DISABLED,
MediaSessionInfo::SessionState::kActive,
MediaSessionInfo::SessionState::kActive},
{MediaSuspend::DISABLED, BackgroundResuming::ENABLED,
MediaSessionInfo::SessionState::kActive,
MediaSessionInfo::SessionState::kActive},
{MediaSuspend::DISABLED, BackgroundResuming::ENABLED,
SessionState::SUSPENDED, SessionState::SUSPENDED},
MediaSessionInfo::SessionState::kSuspended,
MediaSessionInfo::SessionState::kSuspended},
};
} // anonymous namespace
......
......@@ -5,7 +5,6 @@
#include "content/browser/media/session/media_session_service_impl.h"
#include "base/command_line.h"
#include "base/run_loop.h"
#include "content/browser/media/session/media_session_impl.h"
#include "content/browser/media/session/media_session_player_observer.h"
#include "content/public/browser/web_contents_observer.h"
......@@ -14,34 +13,13 @@
#include "content/public/test/content_browser_test_utils.h"
#include "content/shell/browser/shell.h"
#include "media/base/media_content_type.h"
#include "services/media_session/public/cpp/test/mock_media_session.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace content {
namespace {
class MockMediaSessionObserver : public MediaSessionObserver {
public:
explicit MockMediaSessionObserver(
MediaSession* session,
const base::Closure& closure_on_actions_change)
: MediaSessionObserver(session),
closure_on_actions_change_(closure_on_actions_change) {}
void MediaSessionActionsChanged(
const std::set<media_session::mojom::MediaSessionAction>& actions)
override {
// Wait for the page action to be present.
if (base::ContainsKey(
actions, media_session::mojom::MediaSessionAction::kSeekForward)) {
closure_on_actions_change_.Run();
}
}
private:
base::Closure closure_on_actions_change_;
};
class MockWebContentsObserver : public WebContentsObserver {
public:
explicit MockWebContentsObserver(WebContents* contents,
......@@ -129,11 +107,12 @@ class MediaSessionServiceImplBrowserTest : public ContentBrowserTest {
}
bool ExecuteScriptToSetUpMediaSessionSync() {
// Using the actions change as the signal of completion.
base::RunLoop run_loop;
MockMediaSessionObserver observer(GetSession(), run_loop.QuitClosure());
bool result = ExecuteScript(shell(), kSetUpMediaSessionScript);
run_loop.Run();
media_session::test::MockMediaSessionMojoObserver observer(*GetSession());
observer.WaitForActions();
EXPECT_TRUE(base::ContainsKey(
observer.actions_set(),
media_session::mojom::MediaSessionAction::kSeekForward));
return result;
}
......
......@@ -173,8 +173,6 @@ jumbo_source_set("browser_sources") {
"media_keys_listener_manager.h",
"media_request_state.h",
"media_session.h",
"media_session_observer.cc",
"media_session_observer.h",
"media_stream_request.cc",
"media_stream_request.h",
"message_port_provider.h",
......
......@@ -13,7 +13,6 @@
namespace content {
class MediaSessionObserver;
class WebContents;
// MediaSession manages the media session and audio focus for a given
......@@ -92,12 +91,6 @@ class MediaSession : public media_session::mojom::MediaSession {
protected:
MediaSession() = default;
private:
friend class MediaSessionObserver;
virtual void AddObserver(MediaSessionObserver* observer) = 0;
virtual void RemoveObserver(MediaSessionObserver* observer) = 0;
};
} // namespace content
......
// 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 "content/public/browser/media_session_observer.h"
#include "content/browser/media/session/media_session_impl.h"
namespace content {
MediaSessionObserver::MediaSessionObserver(MediaSession* media_session)
: media_session_(media_session) {
if (media_session_)
media_session_->AddObserver(this);
}
MediaSessionObserver::~MediaSessionObserver() {
StopObserving();
}
MediaSession* MediaSessionObserver::media_session() const {
return media_session_;
}
void MediaSessionObserver::StopObserving() {
if (media_session_)
media_session_->RemoveObserver(this);
media_session_ = nullptr;
}
} // namespace content
// 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 CONTENT_PUBLIC_BROWSER_MEDIA_SESSION_OBSERVER_H_
#define CONTENT_PUBLIC_BROWSER_MEDIA_SESSION_OBSERVER_H_
#include <set>
#include "base/macros.h"
#include "base/optional.h"
#include "content/common/content_export.h"
namespace media_session {
namespace mojom {
enum class MediaSessionAction;
} // namespace mojom
} // namespace media_session
namespace media_session {
struct MediaMetadata;
} // namespace media_session
namespace content {
class MediaSession;
// The observer for observing MediaSession events.
class CONTENT_EXPORT MediaSessionObserver {
public:
// Gets the observed MediaSession. Will return null when the session is
// destroyed.
MediaSession* media_session() const;
// Called when the observed MediaSession is being destroyed. Give subclass a
// chance to clean up. media_session() will return nullptr after this method
// is called.
virtual void MediaSessionDestroyed() {}
// Called when the observed MediaSession has changed its state.
virtual void MediaSessionStateChanged(bool is_controllable,
bool is_suspended) {}
// Called when the observed MediaSession has changed metadata.
virtual void MediaSessionMetadataChanged(
const base::Optional<media_session::MediaMetadata>& metadata) {}
// Called when the media session action list has changed.
virtual void MediaSessionActionsChanged(
const std::set<media_session::mojom::MediaSessionAction>& action) {}
protected:
// Create a MediaSessionObserver and start observing a session.
explicit MediaSessionObserver(MediaSession* media_session);
// Destruct a MediaSessionObserver and remove it from the session if it's
// still observing.
virtual ~MediaSessionObserver();
private:
friend class MediaSessionImpl;
void StopObserving();
// Weak pointer to MediaSession
MediaSession* media_session_;
DISALLOW_COPY_AND_ASSIGN(MediaSessionObserver);
};
} // namespace content
#endif // CONTENT_PUBLIC_BROWSER_MEDIA_SESSION_OBSERVER_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment