diff options
Diffstat (limited to 'chromium/content/browser/picture_in_picture')
8 files changed, 542 insertions, 220 deletions
diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_content_browsertest.cc b/chromium/content/browser/picture_in_picture/picture_in_picture_content_browsertest.cc new file mode 100644 index 00000000000..81898cffe91 --- /dev/null +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_content_browsertest.cc @@ -0,0 +1,229 @@ +// Copyright 2020 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/browser/picture_in_picture/picture_in_picture_service_impl.h" +#include "content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h" +#include "content/public/browser/content_browser_client.h" +#include "content/public/browser/overlay_window.h" +#include "content/public/common/content_client.h" +#include "content/public/common/content_switches.h" +#include "content/public/test/browser_test.h" +#include "content/public/test/content_browser_test.h" +#include "content/public/test/content_browser_test_utils.h" +#include "content/shell/browser/shell.h" +#include "net/dns/mock_host_resolver.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "third_party/blink/public/mojom/picture_in_picture/picture_in_picture.mojom.h" + +using testing::Mock; +using testing::NiceMock; + +namespace content { + +namespace { + +class TestOverlayWindow : public OverlayWindow { + public: + TestOverlayWindow() = default; + ~TestOverlayWindow() override = default; + + static std::unique_ptr<OverlayWindow> Create( + PictureInPictureWindowController* controller) { + return std::unique_ptr<OverlayWindow>(new TestOverlayWindow()); + } + + bool IsActive() override { return false; } + void Close() override {} + void ShowInactive() override {} + void Hide() override {} + bool IsVisible() override { return false; } + bool IsAlwaysOnTop() override { return false; } + gfx::Rect GetBounds() override { return gfx::Rect(size_); } + void UpdateVideoSize(const gfx::Size& natural_size) override { + size_ = natural_size; + } + void SetPlaybackState(PlaybackState playback_state) override {} + void SetPlayPauseButtonVisibility(bool is_visible) override {} + void SetSkipAdButtonVisibility(bool is_visible) override {} + void SetNextTrackButtonVisibility(bool is_visible) override {} + void SetPreviousTrackButtonVisibility(bool is_visible) override {} + void SetSurfaceId(const viz::SurfaceId& surface_id) override {} + cc::Layer* GetLayerForTesting() override { return nullptr; } + + private: + gfx::Size size_; + + DISALLOW_COPY_AND_ASSIGN(TestOverlayWindow); +}; + +class TestContentBrowserClient : public ContentBrowserClient { + public: + TestContentBrowserClient() = default; + ~TestContentBrowserClient() override = default; + + std::unique_ptr<OverlayWindow> CreateWindowForPictureInPicture( + PictureInPictureWindowController* controller) override { + return TestOverlayWindow::Create(controller); + } +}; + +class TestWebContentsDelegate : public WebContentsDelegate { + public: + TestWebContentsDelegate() = default; + ~TestWebContentsDelegate() override = default; + + PictureInPictureResult EnterPictureInPicture( + WebContents* web_contents, + const viz::SurfaceId&, + const gfx::Size& natural_size) override { + return PictureInPictureResult::kSuccess; + } + + MOCK_METHOD0(ExitPictureInPicture, void()); +}; + +class PictureInPictureContentBrowserTest : public ContentBrowserTest { + public: + void SetUpCommandLine(base::CommandLine* command_line) override { + ContentBrowserTest::SetUpCommandLine(command_line); + + base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kEnableBlinkFeatures, "PictureInPictureAPI"); + } + + void SetUpOnMainThread() override { + ContentBrowserTest::SetUpOnMainThread(); + + host_resolver()->AddRule("*", "127.0.0.1"); + + old_browser_client_ = SetBrowserClientForTesting(&content_browser_client_); + shell()->web_contents()->SetDelegate(&web_contents_delegate_); + } + + void TearDownOnMainThread() override { + SetBrowserClientForTesting(old_browser_client_); + + ContentBrowserTest::TearDownOnMainThread(); + } + + protected: + NiceMock<TestWebContentsDelegate> web_contents_delegate_; + + private: + ContentBrowserClient* old_browser_client_ = nullptr; + TestContentBrowserClient content_browser_client_; +}; + +} // namespace + +IN_PROC_BROWSER_TEST_F(PictureInPictureContentBrowserTest, + RequestSecondVideoInSameRFHDoesNotCloseWindow) { + EXPECT_CALL(web_contents_delegate_, ExitPictureInPicture()).Times(0); + + EXPECT_TRUE(NavigateToURL( + shell(), GetTestUrl("media/picture_in_picture", "two-videos.html"))); + + // Play first video. + ASSERT_TRUE(ExecuteScript(shell()->web_contents(), "videos[0].play();")); + + base::string16 expected_title = base::ASCIIToUTF16("videos[0] playing"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // Play second video. + ASSERT_TRUE(ExecuteScript(shell()->web_contents(), "videos[1].play();")); + + expected_title = base::ASCIIToUTF16("videos[1] playing"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // Send first video in Picture-in-Picture. + ASSERT_TRUE(ExecuteScript(shell()->web_contents(), + "videos[0].requestPictureInPicture();")); + + expected_title = base::ASCIIToUTF16("videos[0] entered picture-in-picture"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // Send second video in Picture-in-Picture. + ASSERT_TRUE(ExecuteScript(shell()->web_contents(), + "videos[1].requestPictureInPicture();")); + + expected_title = base::ASCIIToUTF16("videos[1] entered picture-in-picture"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // The session should still be active and ExitPictureInPicture() never called. + EXPECT_NE(nullptr, PictureInPictureWindowControllerImpl::FromWebContents( + shell()->web_contents()) + ->active_session_for_testing()); + + Mock::VerifyAndClearExpectations(&web_contents_delegate_); +} + +IN_PROC_BROWSER_TEST_F(PictureInPictureContentBrowserTest, + RequestSecondVideoInDifferentRFHDoesNotCloseWindow) { + EXPECT_CALL(web_contents_delegate_, ExitPictureInPicture()).Times(0); + + ASSERT_TRUE(embedded_test_server()->Start()); + + EXPECT_TRUE(NavigateToURL( + shell(), + embedded_test_server()->GetURL( + "example.com", "/media/picture_in_picture/two-videos.html"))); + + base::string16 expected_title = base::ASCIIToUTF16("iframe loaded"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // Play first video. + ASSERT_TRUE(ExecuteScript(shell()->web_contents(), "videos[0].play();")); + + expected_title = base::ASCIIToUTF16("videos[0] playing"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // Play second video (in iframe). + ASSERT_TRUE( + ExecuteScript(shell()->web_contents(), "iframeVideos[0].play();")); + + expected_title = base::ASCIIToUTF16("iframeVideos[0] playing"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // Send first video in Picture-in-Picture. + ASSERT_TRUE(ExecuteScript(shell()->web_contents(), + "videos[0].requestPictureInPicture();")); + + expected_title = base::ASCIIToUTF16("videos[0] entered picture-in-picture"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // Send second video in Picture-in-Picture. + ASSERT_TRUE(ExecuteScript(shell()->web_contents(), + "iframeVideos[0].requestPictureInPicture();")); + + expected_title = + base::ASCIIToUTF16("iframeVideos[0] entered picture-in-picture"); + EXPECT_EQ( + expected_title, + TitleWatcher(shell()->web_contents(), expected_title).WaitAndGetTitle()); + + // The session should still be active and ExitPictureInPicture() never called. + EXPECT_NE(nullptr, PictureInPictureWindowControllerImpl::FromWebContents( + shell()->web_contents()) + ->active_session_for_testing()); + + Mock::VerifyAndClearExpectations(&web_contents_delegate_); +} + +} // namespace content diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.cc b/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.cc index 0911fbe1fc1..1112a9cad60 100644 --- a/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.cc +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.cc @@ -6,8 +6,9 @@ #include <utility> +#include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/picture_in_picture/picture_in_picture_session.h" -#include "content/browser/web_contents/web_contents_impl.h" +#include "content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h" #include "content/public/browser/web_contents_delegate.h" namespace content { @@ -36,30 +37,22 @@ void PictureInPictureServiceImpl::StartSession( mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> observer, StartSessionCallback callback) { gfx::Size window_size; - - WebContentsImpl* web_contents_impl = static_cast<WebContentsImpl*>( - WebContents::FromRenderFrameHost(render_frame_host())); - - auto result = web_contents_impl->EnterPictureInPicture(surface_id.value(), - natural_size); - mojo::PendingRemote<blink::mojom::PictureInPictureSession> session_remote; - // Picture-in-Picture may not be supported by all embedders, so we should only - // create the session if the EnterPictureInPicture request was successful. - if (result == PictureInPictureResult::kSuccess) { - active_session_ = std::make_unique<PictureInPictureSession>( - this, MediaPlayerId(render_frame_host_, player_id), surface_id, - natural_size, show_play_pause_button, - session_remote.InitWithNewPipeAndPassReceiver(), std::move(observer), - &window_size); - - // Frames are to be blocklisted from the back-forward cache because the - // picture-in-picture continues to be displayed while the page is in the - // cache instead of closing. - static_cast<RenderFrameHostImpl*>(render_frame_host_) - ->OnSchedulerTrackedFeatureUsed( - blink::scheduler::WebSchedulerTrackedFeature::kPictureInPicture); + if (surface_id.has_value()) { + auto result = GetController().StartSession( + this, MediaPlayerId(render_frame_host(), player_id), surface_id.value(), + natural_size, show_play_pause_button, std::move(observer), + &session_remote, &window_size); + + if (result == PictureInPictureResult::kSuccess) { + // Frames are to be blocklisted from the back-forward cache because the + // picture-in-picture continues to be displayed while the page is in the + // cache instead of closing. + static_cast<RenderFrameHostImpl*>(render_frame_host()) + ->OnSchedulerTrackedFeatureUsed( + blink::scheduler::WebSchedulerTrackedFeature::kPictureInPicture); + } } std::move(callback).Run(std::move(session_remote), window_size); @@ -68,14 +61,18 @@ void PictureInPictureServiceImpl::StartSession( PictureInPictureServiceImpl::PictureInPictureServiceImpl( RenderFrameHost* render_frame_host, mojo::PendingReceiver<blink::mojom::PictureInPictureService> receiver) - : FrameServiceBase(render_frame_host, std::move(receiver)), - render_frame_host_(render_frame_host) {} + : FrameServiceBase(render_frame_host, std::move(receiver)) {} PictureInPictureServiceImpl::~PictureInPictureServiceImpl() { // If the service is destroyed because the frame was destroyed, the session // may still be active and it has to be shutdown before its dtor runs. - if (active_session_) - active_session_->Shutdown(); + GetController().OnServiceDeleted(this); +} + +PictureInPictureWindowControllerImpl& +PictureInPictureServiceImpl::GetController() { + return *PictureInPictureWindowControllerImpl::GetOrCreateForWebContents( + WebContents::FromRenderFrameHost(render_frame_host())); } } // namespace content diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.h b/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.h index c06407a97e0..4ff5afe0ae3 100644 --- a/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.h +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl.h @@ -7,7 +7,6 @@ #include <memory> -#include "base/containers/unique_ptr_adapters.h" #include "content/common/content_export.h" #include "content/public/browser/frame_service_base.h" #include "mojo/public/cpp/bindings/pending_receiver.h" @@ -16,13 +15,16 @@ namespace content { -class PictureInPictureSession; +class PictureInPictureWindowControllerImpl; // Receives Picture-in-Picture messages from a given RenderFrame. There is one -// PictureInPictureServiceImpl per RenderFrameHost. The service gets a hold of -// a PictureInPictureSession to which it delegates most of the interactions with -// the rest of the Picture-in-Picture classes such as -// PictureInPictureWindowController. +// PictureInPictureServiceImpl per RenderFrameHost. The service pipes the +// `StartSession()` call to the PictureInPictureWindowControllerImpl which owns +// the created session. The same object will get notified when the service is +// killed given that the PictureInPictureWindowControllerImpl is +// WebContents-bound instead of RenderFrameHost. +// PictureInPictureServiceImpl owns itself. It self-destruct as needed, see the +// FrameServiceBase's documentation for more information. class CONTENT_EXPORT PictureInPictureServiceImpl final : public content::FrameServiceBase<blink::mojom::PictureInPictureService> { public: @@ -43,10 +45,6 @@ class CONTENT_EXPORT PictureInPictureServiceImpl final mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>, StartSessionCallback) final; - PictureInPictureSession* active_session_for_testing() const { - return active_session_.get(); - } - private: friend class PictureInPictureSession; @@ -55,9 +53,7 @@ class CONTENT_EXPORT PictureInPictureServiceImpl final mojo::PendingReceiver<blink::mojom::PictureInPictureService>); ~PictureInPictureServiceImpl() override; - RenderFrameHost* render_frame_host_ = nullptr; - - std::unique_ptr<PictureInPictureSession> active_session_; + PictureInPictureWindowControllerImpl& GetController(); DISALLOW_COPY_AND_ASSIGN(PictureInPictureServiceImpl); }; diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc b/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc index 093a1e9efde..925d178a3e4 100644 --- a/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc @@ -9,6 +9,7 @@ #include "base/test/bind_test_util.h" #include "build/build_config.h" +#include "content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h" #include "content/common/media/media_player_delegate_messages.h" #include "content/public/browser/overlay_window.h" #include "content/public/browser/web_contents_delegate.h" @@ -22,6 +23,8 @@ #include "mojo/public/cpp/bindings/remote.h" #include "testing/gmock/include/gmock/gmock.h" +using testing::_; + namespace content { class DummyPictureInPictureSessionObserver @@ -72,7 +75,7 @@ class TestOverlayWindow : public OverlayWindow { size_ = natural_size; } void SetPlaybackState(PlaybackState playback_state) override {} - void SetAlwaysHidePlayPauseButton(bool is_visible) override {} + void SetPlayPauseButtonVisibility(bool is_visible) override {} void SetSkipAdButtonVisibility(bool is_visible) override {} void SetNextTrackButtonVisibility(bool is_visible) override {} void SetPreviousTrackButtonVisibility(bool is_visible) override {} @@ -137,6 +140,11 @@ class PictureInPictureServiceImplTest : public RenderViewHostImplTestHarness { TEST_F(PictureInPictureServiceImplTest, MAYBE_EnterPictureInPicture) { const int kPlayerVideoOnlyId = 30; + const PictureInPictureWindowControllerImpl* controller = + PictureInPictureWindowControllerImpl::GetOrCreateForWebContents( + contents()); + + ASSERT_TRUE(controller); DummyPictureInPictureSessionObserver observer; mojo::Receiver<blink::mojom::PictureInPictureSessionObserver> @@ -146,7 +154,7 @@ TEST_F(PictureInPictureServiceImplTest, MAYBE_EnterPictureInPicture) { observer_receiver.Bind(observer_remote.InitWithNewPipeAndPassReceiver()); // If Picture-in-Picture there shouldn't be an active session. - EXPECT_FALSE(service().active_session_for_testing()); + EXPECT_FALSE(controller->active_session_for_testing()); viz::SurfaceId surface_id = viz::SurfaceId(viz::FrameSinkId(1, 1), @@ -171,7 +179,7 @@ TEST_F(PictureInPictureServiceImplTest, MAYBE_EnterPictureInPicture) { window_size = b; })); - EXPECT_TRUE(service().active_session_for_testing()); + EXPECT_TRUE(controller->active_session_for_testing()); EXPECT_TRUE(session_remote); EXPECT_EQ(gfx::Size(42, 42), window_size); @@ -181,15 +189,20 @@ TEST_F(PictureInPictureServiceImplTest, MAYBE_EnterPictureInPicture) { contents()->GetMainFrame()->OnMessageReceived( MediaPlayerDelegateHostMsg_OnMediaDestroyed( contents()->GetMainFrame()->GetRoutingID(), kPlayerVideoOnlyId)); - EXPECT_TRUE(service().active_session_for_testing()); + EXPECT_TRUE(controller->active_session_for_testing()); } TEST_F(PictureInPictureServiceImplTest, EnterPictureInPicture_NotSupported) { const int kPlayerVideoOnlyId = 30; + const PictureInPictureWindowControllerImpl* controller = + PictureInPictureWindowControllerImpl::GetOrCreateForWebContents( + contents()); + + ASSERT_TRUE(controller); + EXPECT_FALSE(controller->active_session_for_testing()); + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> observer_remote; - EXPECT_FALSE(service().active_session_for_testing()); - viz::SurfaceId surface_id = viz::SurfaceId(viz::FrameSinkId(1, 1), viz::LocalSurfaceId( @@ -213,10 +226,52 @@ TEST_F(PictureInPictureServiceImplTest, EnterPictureInPicture_NotSupported) { window_size = b; })); - EXPECT_FALSE(service().active_session_for_testing()); - // The |session_remote| won't be bound because the |pending_remote| received - // in the StartSessionCallback will be invalid due to PictureInPictureSession - // not ever being created (meaning the the receiver won't be bound either). + EXPECT_FALSE(controller->active_session_for_testing()); + + // The |session_remote| won't be bound because the |remote| received in the + // StartSessionCallback will be invalid due to PictureInPictureSession not + // ever being created (meaning the the receiver won't be bound either). + EXPECT_FALSE(session_remote); + EXPECT_EQ(gfx::Size(), window_size); +} + +// The |surface_id| is an optional parameter in the StartSession() call but +// needs to be non-null in order to create a session at the moment. The creation +// will early return if that condition isn't satisfied, failing to create the +// session. +TEST_F(PictureInPictureServiceImplTest, EnterPictureInPicture_NoSurfaceId) { + const int kPlayerVideoOnlyId = 30; + const PictureInPictureWindowControllerImpl* controller = + PictureInPictureWindowControllerImpl::GetOrCreateForWebContents( + contents()); + + ASSERT_TRUE(controller); + EXPECT_FALSE(controller->active_session_for_testing()); + + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> + observer_remote; + + EXPECT_CALL(delegate(), EnterPictureInPicture(_, _, _)).Times(0); + + mojo::Remote<blink::mojom::PictureInPictureSession> session_remote; + gfx::Size window_size; + + service().StartSession( + kPlayerVideoOnlyId, base::nullopt, gfx::Size(42, 42), + true /* show_play_pause_button */, std::move(observer_remote), + base::BindLambdaForTesting( + [&](mojo::PendingRemote<blink::mojom::PictureInPictureSession> remote, + const gfx::Size& b) { + if (remote.is_valid()) + session_remote.Bind(std::move(remote)); + window_size = b; + })); + + EXPECT_FALSE(controller->active_session_for_testing()); + + // The |session_remote| won't be bound because the |remote| received in the + // StartSessionCallback will be invalid due to PictureInPictureSession not + // ever being created (meaning the the receiver won't be bound either). EXPECT_FALSE(session_remote); EXPECT_EQ(gfx::Size(), window_size); } diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_session.cc b/chromium/content/browser/picture_in_picture/picture_in_picture_session.cc index 10e9444c688..cb45df54434 100644 --- a/chromium/content/browser/picture_in_picture/picture_in_picture_session.cc +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_session.cc @@ -16,25 +16,14 @@ namespace content { PictureInPictureSession::PictureInPictureSession( PictureInPictureServiceImpl* service, const MediaPlayerId& player_id, - const base::Optional<viz::SurfaceId>& surface_id, - const gfx::Size& natural_size, - bool show_play_pause_button, mojo::PendingReceiver<blink::mojom::PictureInPictureSession> receiver, - mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> observer, - gfx::Size* window_size) + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> observer) : service_(service), receiver_(this, std::move(receiver)), player_id_(player_id), observer_(std::move(observer)) { receiver_.set_disconnect_handler(base::BindOnce( &PictureInPictureSession::OnConnectionError, base::Unretained(this))); - - GetController().SetActiveSession(this); - GetController().EmbedSurface(surface_id.value(), natural_size); - GetController().SetAlwaysHidePlayPauseButton(show_play_pause_button); - GetController().Show(); - - *window_size = GetController().GetSize(); } PictureInPictureSession::~PictureInPictureSession() { @@ -50,17 +39,28 @@ void PictureInPictureSession::Update( const base::Optional<viz::SurfaceId>& surface_id, const gfx::Size& natural_size, bool show_play_pause_button) { - player_id_ = MediaPlayerId(service_->render_frame_host_, player_id); + player_id_ = MediaPlayerId(service_->render_frame_host(), player_id); GetController().EmbedSurface(surface_id.value(), natural_size); - GetController().SetAlwaysHidePlayPauseButton(show_play_pause_button); - GetController().SetActiveSession(this); + GetController().SetShowPlayPauseButton(show_play_pause_button); } void PictureInPictureSession::NotifyWindowResized(const gfx::Size& size) { observer_->OnWindowSizeChanged(size); } +void PictureInPictureSession::Disconnect() { + // |is_stopping_| shouldn't be true for the implementation in //chrome but if + // the WebContentsDelegate's Picture-in-Picture calls are empty, it's possible + // for `Disconnect()` to be called even after `StopInternal()` as the + // expectation of self-destruction will no longer be true. + if (is_stopping_) + return; + + is_stopping_ = true; + observer_->OnStopped(); +} + void PictureInPictureSession::Shutdown() { if (is_stopping_) return; @@ -73,8 +73,6 @@ void PictureInPictureSession::StopInternal(StopCallback callback) { is_stopping_ = true; - GetWebContentsImpl()->ExitPictureInPicture(); - // `OnStopped()` should only be called if there is no callback to run, as a // contract in the API. if (callback) @@ -82,10 +80,8 @@ void PictureInPictureSession::StopInternal(StopCallback callback) { else observer_->OnStopped(); - GetController().SetActiveSession(nullptr); - - // Reset must happen after everything is done as it will destroy |this|. - service_->active_session_.reset(); + // |this| will be deleted after this call. + GetWebContentsImpl()->ExitPictureInPicture(); } void PictureInPictureSession::OnConnectionError() { diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_session.h b/chromium/content/browser/picture_in_picture/picture_in_picture_session.h index d1030e53cd1..520794360a2 100644 --- a/chromium/content/browser/picture_in_picture/picture_in_picture_session.h +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_session.h @@ -20,9 +20,10 @@ class WebContentsImpl; // The PicutreInPictureSession communicates with the // PictureInPictureWindowController and the WebContents. It is created by the -// PictureInPictureService but deletes itself. When created, the session will -// enter Picture-in-Picture and when deleted, it will automatically exit -// Picture-in-Picture unless another session became active. +// PictureInPictureWindowControllerImpl which also deletes it. When created, the +// session will be expected to be active (in Picture-in-Picture) and when +// deleted, it will automatically exit Picture-in-Picture unless another session +// became active. // The session MUST be stopped before its dtor runs to avoid unexpected // deletion. class PictureInPictureSession : public blink::mojom::PictureInPictureSession { @@ -30,13 +31,9 @@ class PictureInPictureSession : public blink::mojom::PictureInPictureSession { PictureInPictureSession( PictureInPictureServiceImpl* service, const MediaPlayerId& player_id, - const base::Optional<viz::SurfaceId>& surface_id, - const gfx::Size& natural_size, - bool show_play_pause_button, mojo::PendingReceiver<blink::mojom::PictureInPictureSession> receiver, mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> - observer, - gfx::Size* window_size); + observer); ~PictureInPictureSession() override; // blink::mojom::PictureInPictureSession interface. @@ -48,14 +45,22 @@ class PictureInPictureSession : public blink::mojom::PictureInPictureSession { void NotifyWindowResized(const gfx::Size& size); - // Returns the player that is currently in Picture-in-Picture. Returns nullopt - // if there are none. - const base::Optional<MediaPlayerId>& player_id() const { return player_id_; } + // Returns the player that is currently in Picture-in-Picture. + MediaPlayerId player_id() const { return player_id_; } + + // Stops the session without closing the window. It will prevent the session + // to later trying to shutdown when the PictureInPictureWindowController is + // notified. + void Disconnect(); // Shuts down the session. Called by the window controller when the window is // closed. void Shutdown(); + // Returns the PictureInPictureServiceImpl instance associated with this + // session. It cannot be null. + PictureInPictureServiceImpl* service() { return service_; } + private: PictureInPictureSession() = delete; @@ -74,12 +79,14 @@ class PictureInPictureSession : public blink::mojom::PictureInPictureSession { // session. PictureInPictureWindowControllerImpl& GetController(); - // Owns |this|. + // Will notified The PictureInPictureWindowControllerImpl who owns |this| when + // it gets destroyed in order for |this| to be destroyed too. Indirectly owns + // |this|. PictureInPictureServiceImpl* service_; mojo::Receiver<blink::mojom::PictureInPictureSession> receiver_; - base::Optional<MediaPlayerId> player_id_; + MediaPlayerId player_id_; // Whether the session is currently stopping. The final stop of stopping is to // be destroyed so once its set to true it will never be set back to false and diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc b/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc index 0bba550f5c2..06674e99421 100644 --- a/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc @@ -5,6 +5,7 @@ #include "content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h" #include <set> +#include <utility> #include "components/viz/common/surfaces/surface_id.h" #include "content/browser/media/media_web_contents_observer.h" @@ -15,6 +16,7 @@ #include "content/public/browser/content_browser_client.h" #include "content/public/browser/overlay_window.h" #include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_delegate.h" // for PictureInPictureResult #include "content/public/browser/web_contents_observer.h" #include "content/public/common/content_client.h" @@ -39,27 +41,13 @@ PictureInPictureWindowControllerImpl::GetOrCreateForWebContents( return FromWebContents(web_contents); } -PictureInPictureWindowControllerImpl::~PictureInPictureWindowControllerImpl() { - if (window_) - window_->Close(); - - // If the initiator WebContents is being destroyed, there is no need to put - // the video's media player in a post-Picture-in-Picture mode. In fact, some - // things, such as the MediaWebContentsObserver, may already been torn down. - if (initiator_->IsBeingDestroyed()) - return; - - initiator_->SetHasPictureInPictureVideo(false); - OnLeavingPictureInPicture(true /* should_pause_video */); -} +PictureInPictureWindowControllerImpl::~PictureInPictureWindowControllerImpl() = + default; PictureInPictureWindowControllerImpl::PictureInPictureWindowControllerImpl( - WebContents* initiator) - : WebContentsObserver(initiator), - initiator_(static_cast<WebContentsImpl* const>(initiator)) { - DCHECK(initiator_); - - media_web_contents_observer_ = initiator_->media_web_contents_observer(); + WebContents* web_contents) + : WebContentsObserver(web_contents) { + DCHECK(web_contents); EnsureWindow(); DCHECK(window_) << "Picture in Picture requires a valid window."; @@ -69,7 +57,7 @@ void PictureInPictureWindowControllerImpl::Show() { DCHECK(window_); DCHECK(surface_id_.is_valid()); - MediaSessionImpl* media_session = MediaSessionImpl::Get(initiator_); + MediaSessionImpl* media_session = MediaSessionImpl::Get(web_contents()); media_session_action_play_handled_ = media_session->ShouldRouteAction( media_session::mojom::MediaSessionAction::kPlay); media_session_action_pause_handled_ = media_session->ShouldRouteAction( @@ -89,7 +77,7 @@ void PictureInPictureWindowControllerImpl::Show() { window_->SetPreviousTrackButtonVisibility( media_session_action_previous_track_handled_); window_->ShowInactive(); - initiator_->SetHasPictureInPictureVideo(true); + GetWebContentsImpl()->SetHasPictureInPictureVideo(true); } void PictureInPictureWindowControllerImpl::Close(bool should_pause_video) { @@ -102,7 +90,7 @@ void PictureInPictureWindowControllerImpl::Close(bool should_pause_video) { void PictureInPictureWindowControllerImpl::CloseAndFocusInitiator() { Close(false /* should_pause_video */); - initiator_->Activate(); + GetWebContentsImpl()->Activate(); } void PictureInPictureWindowControllerImpl::OnWindowDestroyed() { @@ -114,17 +102,18 @@ void PictureInPictureWindowControllerImpl::EmbedSurface( const viz::SurfaceId& surface_id, const gfx::Size& natural_size) { EnsureWindow(); - DCHECK(window_); + DCHECK(window_); + DCHECK(active_session_); DCHECK(surface_id.is_valid()); surface_id_ = surface_id; - // Update the media player id in step with the video surface id. If the - // surface id was updated for the same video, this is a no-op. This could - // be updated for a different video if another media player on the same - // |initiator_| enters Picture-in-Picture mode. - UpdateMediaPlayerId(); + // Update the playback state in step with the video surface id. If the surface + // id was updated for the same video, this is a no-op. This could be updated + // for a different video if another media player on the same WebContents + // enters Picture-in-Picture mode. + UpdatePlaybackState(IsPlayerActive(), false); window_->UpdateVideoSize(natural_size); window_->SetSurfaceId(surface_id_); @@ -135,26 +124,20 @@ OverlayWindow* PictureInPictureWindowControllerImpl::GetWindowForTesting() { } void PictureInPictureWindowControllerImpl::UpdateLayerBounds() { - if (media_player_id_.has_value() && active_session_ && window_ && - window_->IsVisible()) { + if (active_session_ && window_ && window_->IsVisible()) active_session_->NotifyWindowResized(window_->GetBounds().size()); - } } bool PictureInPictureWindowControllerImpl::IsPlayerActive() { - if (!media_player_id_.has_value()) - media_player_id_ = - active_session_ ? active_session_->player_id() : base::nullopt; - - // At creation time, the player id may not be set. - if (!media_player_id_.has_value()) + if (!active_session_) return false; - return media_web_contents_observer_->IsPlayerActive(*media_player_id_); + return GetWebContentsImpl()->media_web_contents_observer()->IsPlayerActive( + active_session_->player_id()); } -WebContents* PictureInPictureWindowControllerImpl::GetInitiatorWebContents() { - return initiator_; +WebContents* PictureInPictureWindowControllerImpl::GetWebContents() { + return web_contents(); } void PictureInPictureWindowControllerImpl::UpdatePlaybackState( @@ -168,7 +151,7 @@ void PictureInPictureWindowControllerImpl::UpdatePlaybackState( return; } - DCHECK(media_player_id_.has_value()); + DCHECK(active_session_); window_->SetPlaybackState(is_playing ? OverlayWindow::PlaybackState::kPlaying : OverlayWindow::PlaybackState::kPaused); @@ -176,67 +159,94 @@ void PictureInPictureWindowControllerImpl::UpdatePlaybackState( bool PictureInPictureWindowControllerImpl::TogglePlayPause() { DCHECK(window_); + DCHECK(active_session_); + + MediaPlayerId player_id = active_session_->player_id(); if (IsPlayerActive()) { if (media_session_action_pause_handled_) { - MediaSessionImpl::Get(initiator_) + MediaSessionImpl::Get(web_contents()) ->Suspend(MediaSession::SuspendType::kUI); return true /* still playing */; } - media_player_id_->render_frame_host->Send(new MediaPlayerDelegateMsg_Pause( - media_player_id_->render_frame_host->GetRoutingID(), - media_player_id_->delegate_id, false /* triggered_by_user */)); + player_id.render_frame_host->Send(new MediaPlayerDelegateMsg_Pause( + player_id.render_frame_host->GetRoutingID(), player_id.delegate_id, + false /* triggered_by_user */)); return false /* paused */; } if (media_session_action_play_handled_) { - MediaSessionImpl::Get(initiator_)->Resume(MediaSession::SuspendType::kUI); + MediaSessionImpl::Get(web_contents()) + ->Resume(MediaSession::SuspendType::kUI); return false /* still paused */; } - media_player_id_->render_frame_host->Send(new MediaPlayerDelegateMsg_Play( - media_player_id_->render_frame_host->GetRoutingID(), - media_player_id_->delegate_id)); + player_id.render_frame_host->Send(new MediaPlayerDelegateMsg_Play( + player_id.render_frame_host->GetRoutingID(), player_id.delegate_id)); return true /* playing */; } -void PictureInPictureWindowControllerImpl::UpdateMediaPlayerId() { - media_player_id_ = - active_session_ ? active_session_->player_id() : base::nullopt; - UpdatePlaybackState(IsPlayerActive(), !media_player_id_.has_value()); +PictureInPictureResult PictureInPictureWindowControllerImpl::StartSession( + PictureInPictureServiceImpl* service, + const MediaPlayerId& player_id, + const viz::SurfaceId& surface_id, + const gfx::Size& natural_size, + bool show_play_pause_button, + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> observer, + mojo::PendingRemote<blink::mojom::PictureInPictureSession>* session_remote, + gfx::Size* window_size) { + auto result = + GetWebContentsImpl()->EnterPictureInPicture(surface_id, natural_size); + + // Picture-in-Picture may not be supported by all embedders, so we should only + // create the session if the EnterPictureInPicture request was successful. + if (result != PictureInPictureResult::kSuccess) + return result; + + if (active_session_) + active_session_->Disconnect(); + + active_session_ = std::make_unique<PictureInPictureSession>( + service, player_id, session_remote->InitWithNewPipeAndPassReceiver(), + std::move(observer)); + + EmbedSurface(surface_id, natural_size); + SetShowPlayPauseButton(show_play_pause_button); + Show(); + + *window_size = GetSize(); + return result; } -void PictureInPictureWindowControllerImpl::SetActiveSession( - PictureInPictureSession* session) { - if (active_session_ == session) +void PictureInPictureWindowControllerImpl::OnServiceDeleted( + PictureInPictureServiceImpl* service) { + if (!active_session_ || active_session_->service() != service) return; - if (active_session_) - active_session_->Shutdown(); - - active_session_ = session; + active_session_->Shutdown(); + active_session_ = nullptr; } -void PictureInPictureWindowControllerImpl::SetAlwaysHidePlayPauseButton( - bool is_visible) { - always_hide_play_pause_button_ = is_visible; +void PictureInPictureWindowControllerImpl::SetShowPlayPauseButton( + bool show_play_pause_button) { + always_show_play_pause_button_ = show_play_pause_button; UpdatePlayPauseButtonVisibility(); } void PictureInPictureWindowControllerImpl::SkipAd() { if (media_session_action_skip_ad_handled_) - MediaSession::Get(initiator_)->SkipAd(); + MediaSession::Get(web_contents())->SkipAd(); } void PictureInPictureWindowControllerImpl::NextTrack() { if (media_session_action_next_track_handled_) - MediaSession::Get(initiator_)->NextTrack(); + MediaSession::Get(web_contents())->NextTrack(); } void PictureInPictureWindowControllerImpl::PreviousTrack() { if (media_session_action_previous_track_handled_) - MediaSession::Get(initiator_)->PreviousTrack(); + MediaSession::Get(web_contents())->PreviousTrack(); } void PictureInPictureWindowControllerImpl::MediaSessionActionsChanged( @@ -280,10 +290,10 @@ gfx::Size PictureInPictureWindowControllerImpl::GetSize() { void PictureInPictureWindowControllerImpl::MediaStartedPlaying( const MediaPlayerInfo&, const MediaPlayerId& media_player_id) { - if (initiator_->IsBeingDestroyed()) + if (web_contents()->IsBeingDestroyed()) return; - if (media_player_id_ != media_player_id) + if (!active_session_ || active_session_->player_id() != media_player_id) return; UpdatePlaybackState(true /* is_playing */, false /* reached_end_of_stream */); @@ -293,10 +303,10 @@ void PictureInPictureWindowControllerImpl::MediaStoppedPlaying( const MediaPlayerInfo&, const MediaPlayerId& media_player_id, WebContentsObserver::MediaStoppedReason reason) { - if (initiator_->IsBeingDestroyed()) + if (web_contents()->IsBeingDestroyed()) return; - if (media_player_id_ != media_player_id) + if (!active_session_ || active_session_->player_id() != media_player_id) return; UpdatePlaybackState( @@ -304,30 +314,39 @@ void PictureInPictureWindowControllerImpl::MediaStoppedPlaying( reason == WebContentsObserver::MediaStoppedReason::kReachedEndOfStream); } +void PictureInPictureWindowControllerImpl::WebContentsDestroyed() { + if (window_) + window_->Close(); +} + void PictureInPictureWindowControllerImpl::OnLeavingPictureInPicture( bool should_pause_video) { + DCHECK(active_session_); + + MediaPlayerId player_id = active_session_->player_id(); + if (IsPlayerActive() && should_pause_video) { // Pause the current video so there is only one video playing at a time. - media_player_id_->render_frame_host->Send(new MediaPlayerDelegateMsg_Pause( - media_player_id_->render_frame_host->GetRoutingID(), - media_player_id_->delegate_id, false /* triggered_by_user */)); + player_id.render_frame_host->Send(new MediaPlayerDelegateMsg_Pause( + player_id.render_frame_host->GetRoutingID(), player_id.delegate_id, + false /* triggered_by_user */)); } - if (media_player_id_.has_value()) { - if (active_session_) - active_session_->Shutdown(); - - active_session_ = nullptr; - media_player_id_.reset(); - } + active_session_->Shutdown(); + active_session_ = nullptr; } void PictureInPictureWindowControllerImpl::CloseInternal( bool should_pause_video) { - if (initiator_->IsBeingDestroyed()) + // We shouldn't have an empty active_session_ in this case but (at least for + // there tests), extensions seem to be closing the window before the + // WebContents is marked as being destroyed. It leads to `CloseInternal()` + // being called twice. This early check avoids the rest of the code having to + // be aware of this oddity. + if (web_contents()->IsBeingDestroyed() || !active_session_) return; - initiator_->SetHasPictureInPictureVideo(false); + GetWebContentsImpl()->SetHasPictureInPictureVideo(false); OnLeavingPictureInPicture(should_pause_video); surface_id_ = viz::SurfaceId(); } @@ -344,9 +363,13 @@ void PictureInPictureWindowControllerImpl::UpdatePlayPauseButtonVisibility() { if (!window_) return; - window_->SetAlwaysHidePlayPauseButton((media_session_action_pause_handled_ && + window_->SetPlayPauseButtonVisibility((media_session_action_pause_handled_ && media_session_action_play_handled_) || - always_hide_play_pause_button_); + always_show_play_pause_button_); +} + +WebContentsImpl* PictureInPictureWindowControllerImpl::GetWebContentsImpl() { + return static_cast<WebContentsImpl*>(web_contents()); } WEB_CONTENTS_USER_DATA_KEY_IMPL(PictureInPictureWindowControllerImpl) diff --git a/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h b/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h index 9e04efb3973..6e3b0204983 100644 --- a/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h +++ b/chromium/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h @@ -10,58 +10,66 @@ #include "base/memory/weak_ptr.h" #include "components/viz/common/surfaces/parent_local_surface_id_allocator.h" +#include "content/common/content_export.h" #include "content/public/browser/media_player_id.h" #include "content/public/browser/picture_in_picture_window_controller.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_user_data.h" #include "services/media_session/public/mojom/media_session.mojom.h" +#include "third_party/blink/public/mojom/picture_in_picture/picture_in_picture.mojom.h" namespace content { -class MediaWebContentsObserver; +class PictureInPictureServiceImpl; class PictureInPictureSession; class WebContents; class WebContentsImpl; - -// TODO(thakis,mlamouri): PictureInPictureWindowControllerImpl isn't -// CONTENT_EXPORT'd because it creates complicated build issues with -// WebContentsUserData being a non-exported template. As a result, the class -// uses CONTENT_EXPORT for methods that are being used from tests. -// CONTENT_EXPORT should be moved back to the class when the Windows build will -// work with it. https://crbug.com/589840. -class PictureInPictureWindowControllerImpl +enum class PictureInPictureResult; + +// PictureInPictureWindowControllerImpl is the corner stone of the +// Picture-in-Picture feature in the //content layer. It handles the session +// creation requests (sent by the PictureInPictureServiceImpl), owns the session +// object and therefore handles its lifetime, and communicate with the rest of +// the browser. Requests to the WebContents are sent by the controller and it +// gets notified when the browser needs it to update the Picture-in-Picture +// session. +// The PictureInPictureWindowControllerImpl is managing Picture-in-Picture at a +// WebContents level. If multiple calls request a Picture-in-Picture session +// either in the same frame or in different frames, the controller will handle +// creating the new session, stopping the current one and making sure the window +// is kept around when possible. +class CONTENT_EXPORT PictureInPictureWindowControllerImpl : public PictureInPictureWindowController, public WebContentsUserData<PictureInPictureWindowControllerImpl>, public WebContentsObserver { public: - // Gets a reference to the controller associated with |initiator| and creates - // one if it does not exist. The returned pointer is guaranteed to be + // Gets a reference to the controller associated with |web_contents| and + // creates one if it does not exist. The returned pointer is guaranteed to be // non-null. - CONTENT_EXPORT static PictureInPictureWindowControllerImpl* - GetOrCreateForWebContents(WebContents* initiator); + static PictureInPictureWindowControllerImpl* GetOrCreateForWebContents( + WebContents* web_contents); ~PictureInPictureWindowControllerImpl() override; using PlayerSet = std::set<int>; // PictureInPictureWindowController: - CONTENT_EXPORT void Show() override; - CONTENT_EXPORT void Close(bool should_pause_video) override; - CONTENT_EXPORT void CloseAndFocusInitiator() override; - CONTENT_EXPORT void OnWindowDestroyed() override; - CONTENT_EXPORT OverlayWindow* GetWindowForTesting() override; - CONTENT_EXPORT void UpdateLayerBounds() override; - CONTENT_EXPORT bool IsPlayerActive() override; - CONTENT_EXPORT WebContents* GetInitiatorWebContents() override; - CONTENT_EXPORT bool TogglePlayPause() override; - CONTENT_EXPORT void UpdatePlaybackState(bool is_playing, - bool reached_end_of_stream) override; - CONTENT_EXPORT void SetAlwaysHidePlayPauseButton(bool is_visible) override; - CONTENT_EXPORT void SkipAd() override; - CONTENT_EXPORT void NextTrack() override; - CONTENT_EXPORT void PreviousTrack() override; - - CONTENT_EXPORT void MediaSessionActionsChanged( + void Show() override; + void Close(bool should_pause_video) override; + void CloseAndFocusInitiator() override; + void OnWindowDestroyed() override; + OverlayWindow* GetWindowForTesting() override; + void UpdateLayerBounds() override; + bool IsPlayerActive() override; + WebContents* GetWebContents() override; + bool TogglePlayPause() override; + void UpdatePlaybackState(bool is_playing, + bool reached_end_of_stream) override; + void SkipAd() override; + void NextTrack() override; + void PreviousTrack() override; + + void MediaSessionActionsChanged( const std::set<media_session::mojom::MediaSessionAction>& actions); gfx::Size GetSize(); @@ -72,29 +80,45 @@ class PictureInPictureWindowControllerImpl void MediaStoppedPlaying(const MediaPlayerInfo&, const MediaPlayerId&, WebContentsObserver::MediaStoppedReason) override; - - // TODO(mlamouri): temporary method used because of the media player id is - // stored in a different location from the one that is used to update the - // state of this object. - void UpdateMediaPlayerId(); + void WebContentsDestroyed() override; // Embeds a surface in the Picture-in-Picture window. void EmbedSurface(const viz::SurfaceId& surface_id, const gfx::Size& natural_size); - // Sets the active Picture-in-Picture session associated with the controller. - // This is different from the service's active session as there is one - // controller per WebContents and one service per RenderFrameHost. - // The current session may be shut down as a side effect of this. - void SetActiveSession(PictureInPictureSession* session); + void SetShowPlayPauseButton(bool show_play_pause_button); + + // Called by PictureInPictureServiceImpl when a session request is received. + // The call should return the |session_remote| and |window_size| as out + // params. A failure to create the session should be expressed with an empty + // |window_size| and uninitialized |session_remote|. + // Returns whether the session creation was successful. + PictureInPictureResult StartSession( + PictureInPictureServiceImpl* service, + const MediaPlayerId&, + const viz::SurfaceId& surface_id, + const gfx::Size& natural_size, + bool show_play_pause_button, + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>, + mojo::PendingRemote<blink::mojom::PictureInPictureSession>* + session_remote, + gfx::Size* window_size); + + // Called by PictureInPictureServiceImpl when the service is about to be + // destroyed. It allows |this| to close the |active_session_| if it is + // associated with the service. + void OnServiceDeleted(PictureInPictureServiceImpl* service); + + PictureInPictureSession* active_session_for_testing() const { + return active_session_.get(); + } private: friend class WebContentsUserData<PictureInPictureWindowControllerImpl>; // Use PictureInPictureWindowControllerImpl::GetOrCreateForWebContents() to // create an instance. - CONTENT_EXPORT explicit PictureInPictureWindowControllerImpl( - WebContents* initiator); + explicit PictureInPictureWindowControllerImpl(WebContents* web_contents); // Signal to the media player that |this| is leaving Picture-in-Picture mode. void OnLeavingPictureInPicture(bool should_pause_video); @@ -112,15 +136,10 @@ class PictureInPictureWindowControllerImpl // always_hide_play_pause_button_ is false. void UpdatePlayPauseButtonVisibility(); - std::unique_ptr<OverlayWindow> window_; + // Returns the web_contents() as a WebContentsImpl*. + WebContentsImpl* GetWebContentsImpl(); - // TODO(929156): remove this as it should be accessible via `web_contents()`. - WebContentsImpl* const initiator_; - - // Used to determine the state of the media player and route messages to - // the corresponding media player with id |media_player_id_|. - MediaWebContentsObserver* media_web_contents_observer_; - base::Optional<MediaPlayerId> media_player_id_; + std::unique_ptr<OverlayWindow> window_; viz::SurfaceId surface_id_; @@ -135,13 +154,13 @@ class PictureInPictureWindowControllerImpl // Used to hide play/pause button if video is a MediaStream or has infinite // duration. Play/pause button visibility can be overridden by the Media // Session API in UpdatePlayPauseButtonVisibility(). - bool always_hide_play_pause_button_ = false; + bool always_show_play_pause_button_ = false; // Session currently associated with the Picture-in-Picture window. The // session object makes the bridge with the renderer process by handling // requests and holding states such as the active player id. // The session will be nullptr when there is no active session. - PictureInPictureSession* active_session_ = nullptr; + std::unique_ptr<PictureInPictureSession> active_session_; WEB_CONTENTS_USER_DATA_KEY_DECL(); |