Commit 4c6feff5 authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Commit Bot

Hide MediaSession controls when flinging

When flinging, we currently have 3 notifications show up. The
notification surfaced from the Chrome MediaRouter offers a better path
to control the media route directly, and the media session controls are
redundant.

This CL adds |is_flinging_| into UpdatePlayState's computations, in
order to hide the MediaSession controller when using RemotePlayack.

Bug: 790766
Change-Id: If1acd5eb845875cbc19f08b71b74e27aba048c50
Reviewed-on: https://chromium-review.googlesource.com/c/1327537Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606943}
parent 66791658
...@@ -2670,7 +2670,7 @@ void WebMediaPlayerImpl::UpdatePlayState() { ...@@ -2670,7 +2670,7 @@ void WebMediaPlayerImpl::UpdatePlayState() {
bool is_suspended = pipeline_controller_.IsSuspended(); bool is_suspended = pipeline_controller_.IsSuspended();
bool is_backgrounded = IsBackgroundSuspendEnabled(delegate_) && IsHidden(); bool is_backgrounded = IsBackgroundSuspendEnabled(delegate_) && IsHidden();
PlayState state = UpdatePlayState_ComputePlayState( PlayState state = UpdatePlayState_ComputePlayState(
is_remote, can_auto_suspend, is_suspended, is_backgrounded); is_remote, is_flinging_, can_auto_suspend, is_suspended, is_backgrounded);
SetDelegateState(state.delegate_state, state.is_idle); SetDelegateState(state.delegate_state, state.is_idle);
SetMemoryReportingState(state.is_memory_reporting_enabled); SetMemoryReportingState(state.is_memory_reporting_enabled);
SetSuspendState(state.is_suspended || pending_suspend_resume_cycle_); SetSuspendState(state.is_suspended || pending_suspend_resume_cycle_);
...@@ -2765,8 +2765,18 @@ void WebMediaPlayerImpl::SetSuspendState(bool is_suspended) { ...@@ -2765,8 +2765,18 @@ void WebMediaPlayerImpl::SetSuspendState(bool is_suspended) {
} }
} }
// NOTE: |is_remote| and |is_flinging| both indicate that we are in a remote
// playback session, with the following differences:
// - |is_remote| : we are using |cast_impl_|, and most of WMPI's functions
// are forwarded to it. This method of remote playback is scheduled
// for deprecation soon, in favor of the |is_flinging| path.
// - |is_flinging| : we are using the FlingingRenderer, and WMPI should
// behave exactly if we are using the DefaultRenderer, except for the
// disabling of certain optimizations.
// See https://crbug.com/790766.
WebMediaPlayerImpl::PlayState WebMediaPlayerImpl::PlayState
WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote, WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
bool is_flinging,
bool can_auto_suspend, bool can_auto_suspend,
bool is_suspended, bool is_suspended,
bool is_backgrounded) { bool is_backgrounded) {
...@@ -2845,7 +2855,8 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote, ...@@ -2845,7 +2855,8 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
// - |have_future_data|, since we need to know whether we are paused to // - |have_future_data|, since we need to know whether we are paused to
// correctly configure the session and also because the tracks and // correctly configure the session and also because the tracks and
// duration are passed to DidPlay(), // duration are passed to DidPlay(),
// - |is_remote| is false as remote playback is not handled by the delegate, // - |is_remote| and |is_flinging| are false as remote playback is not
// handled by the delegate,
// - |has_error| is false as player should have no errors, // - |has_error| is false as player should have no errors,
// - |background_suspended| is false, otherwise |has_remote_controls| must // - |background_suspended| is false, otherwise |has_remote_controls| must
// be true. // be true.
...@@ -2859,14 +2870,16 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote, ...@@ -2859,14 +2870,16 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
bool backgrounded_video_has_no_remote_controls = bool backgrounded_video_has_no_remote_controls =
IsBackgroundSuspendEnabled(delegate_) && IsBackgroundSuspendEnabled(delegate_) &&
!IsResumeBackgroundVideosEnabled() && is_backgrounded && HasVideo(); !IsResumeBackgroundVideosEnabled() && is_backgrounded && HasVideo();
bool can_play = !has_error && !is_remote && have_future_data; bool can_play = !has_error && have_future_data;
bool has_remote_controls = bool has_remote_controls =
HasAudio() && !backgrounded_video_has_no_remote_controls; HasAudio() && !backgrounded_video_has_no_remote_controls;
bool alive = can_play && !must_suspend && bool in_remote_playback = is_remote || is_flinging;
bool alive = can_play && !in_remote_playback && !must_suspend &&
(!background_suspended || has_remote_controls); (!background_suspended || has_remote_controls);
if (!alive) { if (!alive) {
// Do not mark players as idle when flinging.
result.delegate_state = DelegateState::GONE; result.delegate_state = DelegateState::GONE;
result.is_idle = delegate_->IsIdle(delegate_id_); result.is_idle = delegate_->IsIdle(delegate_id_) && !is_flinging;
} else if (paused_) { } else if (paused_) {
// TODO(sandersd): Is it possible to have a suspended session, be ended, // TODO(sandersd): Is it possible to have a suspended session, be ended,
// and not be paused? If so we should be in a PLAYING state. // and not be paused? If so we should be in a PLAYING state.
...@@ -2881,7 +2894,8 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote, ...@@ -2881,7 +2894,8 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
// It's not critical if some cases where memory usage can change are missed, // It's not critical if some cases where memory usage can change are missed,
// since media memory changes are usually gradual. // since media memory changes are usually gradual.
result.is_memory_reporting_enabled = result.is_memory_reporting_enabled =
!has_error && can_play && !result.is_suspended && (!paused_ || seeking_); !has_error && can_play && !in_remote_playback && !result.is_suspended &&
(!paused_ || seeking_);
return result; return result;
} }
......
...@@ -432,7 +432,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl ...@@ -432,7 +432,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
// //
// This method should be called any time its dependent values change. These // This method should be called any time its dependent values change. These
// are: // are:
// - isRemote(), // - isRemote(), is_flinging_,
// - hasVideo(), // - hasVideo(),
// - delegate_->IsHidden(), // - delegate_->IsHidden(),
// - network_state_, ready_state_, // - network_state_, ready_state_,
...@@ -444,6 +444,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl ...@@ -444,6 +444,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
// Methods internal to UpdatePlayState(). // Methods internal to UpdatePlayState().
PlayState UpdatePlayState_ComputePlayState(bool is_remote, PlayState UpdatePlayState_ComputePlayState(bool is_remote,
bool is_flinging,
bool can_auto_suspend, bool can_auto_suspend,
bool is_suspended, bool is_suspended,
bool is_backgrounded); bool is_backgrounded);
......
...@@ -502,31 +502,43 @@ class WebMediaPlayerImplTest : public testing::Test { ...@@ -502,31 +502,43 @@ class WebMediaPlayerImplTest : public testing::Test {
WebMediaPlayerImpl::PlayState ComputePlayState() { WebMediaPlayerImpl::PlayState ComputePlayState() {
EXPECT_CALL(client_, WasAlwaysMuted()) EXPECT_CALL(client_, WasAlwaysMuted())
.WillRepeatedly(Return(client_.was_always_muted_)); .WillRepeatedly(Return(client_.was_always_muted_));
return wmpi_->UpdatePlayState_ComputePlayState(false, true, false, false); return wmpi_->UpdatePlayState_ComputePlayState(false, false, true, false,
false);
} }
WebMediaPlayerImpl::PlayState ComputePlayState_FrameHidden() { WebMediaPlayerImpl::PlayState ComputePlayState_FrameHidden() {
EXPECT_CALL(client_, WasAlwaysMuted()) EXPECT_CALL(client_, WasAlwaysMuted())
.WillRepeatedly(Return(client_.was_always_muted_)); .WillRepeatedly(Return(client_.was_always_muted_));
return wmpi_->UpdatePlayState_ComputePlayState(false, true, false, true); return wmpi_->UpdatePlayState_ComputePlayState(false, false, true, false,
true);
} }
WebMediaPlayerImpl::PlayState ComputePlayState_Suspended() { WebMediaPlayerImpl::PlayState ComputePlayState_Suspended() {
EXPECT_CALL(client_, WasAlwaysMuted()) EXPECT_CALL(client_, WasAlwaysMuted())
.WillRepeatedly(Return(client_.was_always_muted_)); .WillRepeatedly(Return(client_.was_always_muted_));
return wmpi_->UpdatePlayState_ComputePlayState(false, true, true, false); return wmpi_->UpdatePlayState_ComputePlayState(false, false, true, true,
false);
} }
WebMediaPlayerImpl::PlayState ComputePlayState_Remote() { WebMediaPlayerImpl::PlayState ComputePlayState_Remote() {
EXPECT_CALL(client_, WasAlwaysMuted()) EXPECT_CALL(client_, WasAlwaysMuted())
.WillRepeatedly(Return(client_.was_always_muted_)); .WillRepeatedly(Return(client_.was_always_muted_));
return wmpi_->UpdatePlayState_ComputePlayState(true, true, false, false); return wmpi_->UpdatePlayState_ComputePlayState(true, false, true, false,
false);
}
WebMediaPlayerImpl::PlayState ComputePlayState_Flinging() {
EXPECT_CALL(client_, WasAlwaysMuted())
.WillRepeatedly(Return(client_.was_always_muted_));
return wmpi_->UpdatePlayState_ComputePlayState(false, true, true, false,
false);
} }
WebMediaPlayerImpl::PlayState ComputePlayState_BackgroundedStreaming() { WebMediaPlayerImpl::PlayState ComputePlayState_BackgroundedStreaming() {
EXPECT_CALL(client_, WasAlwaysMuted()) EXPECT_CALL(client_, WasAlwaysMuted())
.WillRepeatedly(Return(client_.was_always_muted_)); .WillRepeatedly(Return(client_.was_always_muted_));
return wmpi_->UpdatePlayState_ComputePlayState(false, false, false, true); return wmpi_->UpdatePlayState_ComputePlayState(false, false, false, false,
true);
} }
bool IsSuspended() { return wmpi_->pipeline_controller_.IsSuspended(); } bool IsSuspended() { return wmpi_->pipeline_controller_.IsSuspended(); }
...@@ -1132,7 +1144,7 @@ TEST_F(WebMediaPlayerImplTest, ComputePlayState_Remote) { ...@@ -1132,7 +1144,7 @@ TEST_F(WebMediaPlayerImplTest, ComputePlayState_Remote) {
SetMetadata(true, true); SetMetadata(true, true);
SetReadyState(blink::WebMediaPlayer::kReadyStateHaveFutureData); SetReadyState(blink::WebMediaPlayer::kReadyStateHaveFutureData);
// Remote media is always suspended. // Remote media via wmpi_cast is always suspended.
// TODO(sandersd): Decide whether this should count as idle or not. // TODO(sandersd): Decide whether this should count as idle or not.
WebMediaPlayerImpl::PlayState state = ComputePlayState_Remote(); WebMediaPlayerImpl::PlayState state = ComputePlayState_Remote();
EXPECT_EQ(WebMediaPlayerImpl::DelegateState::GONE, state.delegate_state); EXPECT_EQ(WebMediaPlayerImpl::DelegateState::GONE, state.delegate_state);
...@@ -1140,6 +1152,19 @@ TEST_F(WebMediaPlayerImplTest, ComputePlayState_Remote) { ...@@ -1140,6 +1152,19 @@ TEST_F(WebMediaPlayerImplTest, ComputePlayState_Remote) {
EXPECT_FALSE(state.is_memory_reporting_enabled); EXPECT_FALSE(state.is_memory_reporting_enabled);
} }
TEST_F(WebMediaPlayerImplTest, ComputePlayState_Flinging) {
InitializeWebMediaPlayerImpl();
SetMetadata(true, true);
SetReadyState(blink::WebMediaPlayer::kReadyStateHaveFutureData);
// Remote media via the FlingingRenderer should not be idle.
WebMediaPlayerImpl::PlayState state = ComputePlayState_Flinging();
EXPECT_EQ(WebMediaPlayerImpl::DelegateState::GONE, state.delegate_state);
EXPECT_FALSE(state.is_idle);
EXPECT_FALSE(state.is_suspended);
EXPECT_FALSE(state.is_memory_reporting_enabled);
}
TEST_F(WebMediaPlayerImplTest, ComputePlayState_Fullscreen) { TEST_F(WebMediaPlayerImplTest, ComputePlayState_Fullscreen) {
InitializeWebMediaPlayerImpl(); InitializeWebMediaPlayerImpl();
SetMetadata(true, true); SetMetadata(true, true);
......
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