Commit 7f5596e8 authored by zqzhang's avatar zqzhang Committed by Commit bot

[MediaSession] A fix for MediaSessionService routing

Previously, MediaSessionImpl uses the metadata and controls to determine
whether a page uses the MediaSession API. However there comes an issues
that we need to update the routed MediaSession every time when a service
is created/destroyed or its metadata/controls changed, which is a lot of
maintainance work.

This CL simplifies the MediaSessionService routing strategy. It only
checks whether a service is created for a given frame. The rationale is
that a MediaSessionService is created when and only when anything passes
through mojo, which is enough for indicating whether a frame uses
MediaSession API.

BUG=670319

Review-Url: https://codereview.chromium.org/2585243002
Cr-Commit-Position: refs/heads/master@{#439488}
parent fbf954d4
...@@ -575,6 +575,7 @@ bool MediaSessionImpl::AddOneShotPlayer(MediaSessionPlayerObserver* observer, ...@@ -575,6 +575,7 @@ bool MediaSessionImpl::AddOneShotPlayer(MediaSessionPlayerObserver* observer,
void MediaSessionImpl::OnServiceCreated(MediaSessionServiceImpl* service) { void MediaSessionImpl::OnServiceCreated(MediaSessionServiceImpl* service) {
services_[service->GetRenderFrameHost()] = service; services_[service->GetRenderFrameHost()] = service;
UpdateRoutedService();
} }
void MediaSessionImpl::OnServiceDestroyed(MediaSessionServiceImpl* service) { void MediaSessionImpl::OnServiceDestroyed(MediaSessionServiceImpl* service) {
...@@ -617,11 +618,7 @@ void MediaSessionImpl::DidReceiveAction( ...@@ -617,11 +618,7 @@ void MediaSessionImpl::DidReceiveAction(
} }
bool MediaSessionImpl::IsServiceActiveForRenderFrameHost(RenderFrameHost* rfh) { bool MediaSessionImpl::IsServiceActiveForRenderFrameHost(RenderFrameHost* rfh) {
if (!services_.count(rfh)) return services_.find(rfh) != services_.end();
return false;
return services_[rfh]->metadata().has_value() ||
!services_[rfh]->actions().empty();
} }
void MediaSessionImpl::UpdateRoutedService() { void MediaSessionImpl::UpdateRoutedService() {
......
...@@ -157,7 +157,6 @@ class MediaSessionImplBrowserTest : public content::ContentBrowserTest { ...@@ -157,7 +157,6 @@ class MediaSessionImplBrowserTest : public content::ContentBrowserTest {
void EnsureMediaSessionService() { void EnsureMediaSessionService() {
mock_media_session_service_.reset(new MockMediaSessionServiceImpl( mock_media_session_service_.reset(new MockMediaSessionServiceImpl(
shell()->web_contents()->GetMainFrame())); shell()->web_contents()->GetMainFrame()));
mock_media_session_service_->SetMetadata(content::MediaMetadata());
} }
void SetPlaybackState(blink::mojom::MediaSessionPlaybackState state) { void SetPlaybackState(blink::mojom::MediaSessionPlaybackState state) {
......
...@@ -74,7 +74,10 @@ class MediaSessionImplServiceRoutingTest ...@@ -74,7 +74,10 @@ class MediaSessionImplServiceRoutingTest
protected: protected:
void CreateServiceForFrame(TestRenderFrameHost* frame) { void CreateServiceForFrame(TestRenderFrameHost* frame) {
services_[frame] = base::MakeUnique<MockMediaSessionServiceImpl>(frame); services_[frame] = base::MakeUnique<MockMediaSessionServiceImpl>(frame);
services_[frame]->SetMetadata(MediaMetadata()); }
void DestroyServiceForFrame(TestRenderFrameHost* frame) {
services_.erase(frame);
} }
void StartPlayerForFrame(TestRenderFrameHost* frame) { void StartPlayerForFrame(TestRenderFrameHost* frame) {
...@@ -133,23 +136,37 @@ TEST_F(MediaSessionImplServiceRoutingTest, ...@@ -133,23 +136,37 @@ TEST_F(MediaSessionImplServiceRoutingTest,
} }
TEST_F(MediaSessionImplServiceRoutingTest, TEST_F(MediaSessionImplServiceRoutingTest,
OnlyMainFrameProducesAudioButHasInactiveService) { OnlyMainFrameProducesAudioButHasDestroyedService) {
StartPlayerForFrame(main_frame_);
CreateServiceForFrame(main_frame_); CreateServiceForFrame(main_frame_);
services_[main_frame_]->SetMetadata(base::nullopt); StartPlayerForFrame(main_frame_);
DestroyServiceForFrame(main_frame_);
ASSERT_EQ(nullptr, ComputeServiceForRouting()); ASSERT_EQ(nullptr, ComputeServiceForRouting());
} }
TEST_F(MediaSessionImplServiceRoutingTest, TEST_F(MediaSessionImplServiceRoutingTest,
OnlySubFrameProducesAudioButHasInactiveService) { OnlySubFrameProducesAudioButHasDestroyedService) {
CreateServiceForFrame(sub_frame_);
StartPlayerForFrame(sub_frame_); StartPlayerForFrame(sub_frame_);
DestroyServiceForFrame(sub_frame_);
ASSERT_EQ(nullptr, ComputeServiceForRouting());
}
TEST_F(MediaSessionImplServiceRoutingTest,
OnlyMainFrameProducesAudioAndServiceIsCreatedAfterwards) {
StartPlayerForFrame(main_frame_);
CreateServiceForFrame(main_frame_);
ASSERT_EQ(services_[main_frame_].get(), ComputeServiceForRouting());
}
TEST_F(MediaSessionImplServiceRoutingTest,
OnlySubFrameProducesAudioAndServiceIsCreatedAfterwards) {
StartPlayerForFrame(sub_frame_);
CreateServiceForFrame(sub_frame_); CreateServiceForFrame(sub_frame_);
services_[sub_frame_]->SetMetadata(base::nullopt);
ASSERT_EQ(nullptr, ComputeServiceForRouting()); ASSERT_EQ(services_[sub_frame_].get(), ComputeServiceForRouting());
} }
TEST_F(MediaSessionImplServiceRoutingTest, TEST_F(MediaSessionImplServiceRoutingTest,
......
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