Chrome · UI › Video pip
CVE-2026-106320
Logic Error in UI (Video pip)
Overview
Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Files Changed
content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cccontent/browser/picture_in_picture/picture_in_picture_service_impl.cctools/metrics/histograms/enums.xmlcontent/public/browser/disallow_activation_reason.h
Patch
From 1f0c7a5cd9d5697ad9d8d186cd4252679a961873 Mon Sep 17 00:00:00 2001 From: Tommy Steimel <[email protected]> Date: Thu, 03 Sep 2026 14:50:11 -0700 Subject: [PATCH] [video pip] Prevent video pip from pending deletion/cached tabs The PictureInPictureServiceImpl currently does not check to make sure the requesting RenderFrameHost is active, which means an inactive one (e.g. in back-forward cache) could open a video picture-in-picture window. This CL adds a check to ensure the requesting RenderFrameHost is active when opening a video picture-in-picture window. Bug: 553929758 Change-Id: I73de1684adbf7d54400e57247f240f1c8bf044b9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8313010 Commit-Queue: Tommy Steimel <[email protected]> Reviewed-by: Benjamin Keen <[email protected]> Reviewed-by: Dave Tapuska <[email protected]> Cr-Commit-Position: refs/heads/main@{#1691956} --- diff --git a/content/browser/picture_in_picture/picture_in_picture_service_impl.cc b/content/browser/picture_in_picture/picture_in_picture_service_impl.cc index c9ee3da..4cd4319c 100644 --- a/content/browser/picture_in_picture/picture_in_picture_service_impl.cc +++ b/content/browser/picture_in_picture/picture_in_picture_service_impl.cc @@ -9,12 +9,21 @@ #include "base/functional/bind.h" #include "content/browser/picture_in_picture/video_picture_in_picture_window_controller_impl.h" #include "content/browser/renderer_host/render_frame_host_impl.h" +#include "content/public/browser/disallow_activation_reason.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_delegate.h" #include "media/base/video_spatial_format.h" #include "mojo/public/cpp/bindings/callback_helpers.h" namespace content { +namespace { + +bool IsFrameActive(RenderFrameHost& rfh) { + return !rfh.IsInactiveAndDisallowActivation( + DisallowActivationReasonId::kPictureInPictureService); +} + +} // namespace struct PictureInPictureServiceImpl::PendingSession { PendingSession( @@ -94,6 +103,10 @@ std::unique_ptr<PictureInPictureServiceImpl::PendingSession> pending_session, std::optional<ImmersiveOptions> immersive_options) { + if (!IsFrameActive(render_frame_host())) { + return; + } + gfx::Size window_size; mojo::PendingRemote<blink::mojom::PictureInPictureSession> session_remote; @@ -122,6 +135,10 @@ void PictureInPictureServiceImpl::StartSessionImmersive( std::unique_ptr<PictureInPictureServiceImpl::PendingSession> pending_session) { + if (!IsFrameActive(render_frame_host())) { + return; + } + // Immersive playback confirmation flow can only be requested in a // browser-native fullscreen state. auto* web_contents = WebContents::FromRenderFrameHost(&render_frame_host()); diff --git a/content/public/browser/disallow_activation_reason.h b/content/public/browser/disallow_activation_reason.h index 4c975c8b..21a9608 100644 --- a/content/public/browser/disallow_activation_reason.h +++ b/content/public/browser/disallow_activation_reason.h @@ -70,6 +70,7 @@ kWindowPlacement = 46, kClipboard = 47, kShowCaptionSettings = 48, + kPictureInPictureService = 49, // New entries go above here. New entries should be added to // tools/metrics/histograms/enums.xml . kMinEmbedderDisallowActivationReason = 2 << 16, diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index efe248f..89cdff7f 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -5932,6 +5932,10 @@ A request to open the caption settings arrived for a frame in BFCache or pending deletion. </int> + <int value="49" label="kPictureInPictureService"> + A PictureInPictureService IPC arrived for a frame in BFCache or pending + deletion. + </int> </enum> <enum name="IncidentType"> diff --git a/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc b/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc index fc76188..aa92fa33 100644 --- a/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc +++ b/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc @@ -790,4 +790,196 @@ EXPECT_FALSE(first_session_remote); } +TEST_F(PictureInPictureServiceImplTest, + StartSession_InactiveFrameRunningUnloadHandlers) { + main_test_rfh()->SetLifecycleState( + RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers); + ASSERT_FALSE(main_test_rfh()->IsActive()); + + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> + observer_remote; + mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out; + gfx::Size window_size_out; + service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(), + surface_id(), window_size(), show_play_pause_button(), + std::move(observer_remote), source_bounds(), + /*request_immersive=*/false, spatial_format(), + BindSession(session_remote_out, window_size_out)); + + EXPECT_FALSE(GetController()->active_session_for_testing()); + EXPECT_FALSE(session_remote_out); + EXPECT_EQ(gfx::Size(), window_size_out); +} + +TEST_F(PictureInPictureServiceImplTest, + StartSession_InactiveFrameInBackForwardCache) { + main_test_rfh()->SetLifecycleState( + RenderFrameHostImpl::LifecycleStateImpl::kInBackForwardCache); + ASSERT_FALSE(main_test_rfh()->IsActive()); + + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> + observer_remote; + mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out; + gfx::Size window_size_out; + service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(), + surface_id(), window_size(), show_play_pause_button(), + std::move(observer_remote), source_bounds(), + /*request_immersive=*/false, spatial_format(), + BindSession(session_remote_out, window_size_out)); + + EXPECT_FALSE(GetController()->active_session_for_testing()); + EXPECT_FALSE(session_remote_out); + EXPECT_EQ(gfx::Size(), window_size_out); +} + +TEST_F(PictureInPictureServiceImplTest, + StartSessionImmersive_InactiveFrameFails) { + EnterFullscreen(); + + EXPECT_CALL(delegate(), IsImmersivePlaybackEnabled()) + .WillRepeatedly(testing::Return(true)); + + main_test_rfh()->SetLifecycleState( + RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers); + ASSERT_FALSE(main_test_rfh()->IsActive()); + + // Delegate confirmation should never be requested for an inactive frame. + EXPECT_CALL(delegate(), RequestImmersivePlaybackConfirmation(_, _)).Times(0); + + DummyPictureInPictureSessionObserver observer; + mojo::Receiver<blink::mojom::PictureInPictureSessionObserver> + observer_receiver(&observer); + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> + observer_remote; + observer_receiver.Bind(observer_remote.InitWithNewPipeAndPassReceiver()); + + mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out; + gfx::Size window_size_out; + service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(), + surface_id(), window_size(), show_play_pause_button(), + std::move(observer_remote), source_bounds(), + /*request_immersive=*/true, spatial_format(), + BindSession(session_remote_out, window_size_out)); + + EXPECT_FALSE(GetController()->active_session_for_testing()); + EXPECT_FALSE(session_remote_out); + EXPECT_EQ(gfx::Size(), window_size_out); +} + +TEST_F(PictureInPictureServiceImplTest, + StartSessionImmersive_BecomesInactiveDuringConfirmationFails) { + EnterFullscreen(); + + EXPECT_CALL(delegate(), IsImmersivePlaybackEnabled()) + .WillRepeatedly(testing::Return(true)); + + base::OnceCallback<void(ImmersivePlaybackConfirmationResult)> + confirm_callback; + EXPECT_CALL(delegate(), RequestImmersivePlaybackConfirmation(_, _)) + .WillOnce( + [&confirm_callback]( + const ImmersiveOptions& options, + base::OnceCallback<void(ImmersivePlaybackConfirmationResult)> + callback) { confirm_callback = std::move(callback); }); + + DummyPictureInPictureSessionObserver observer; + mojo::Receiver<blink::mojom::PictureInPictureSessionObserver> + observer_receiver(&observer); + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> + observer_remote; + observer_receiver.Bind(observer_remote.InitWithNewPipeAndPassReceiver()); + + mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out; + gfx::Size window_size_out; + service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(), + surface_id(), window_size(), show_play_pause_button(), + std::move(observer_remote), source_bounds(), + /*request_immersive=*/true, spatial_format(), + BindSession(session_remote_out, window_size_out)); + + ASSERT_TRUE(confirm_callback); + EXPECT_FALSE(GetController()->active_session_for_testing()); + + // Frame becomes inactive before user confirmation finishes. + main_test_rfh()->SetLifecycleState( + RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers); + ASSERT_FALSE(main_test_rfh()->IsActive()); + + ImmersivePlaybackConfirmationResult result; + result.status = ImmersivePlaybackConfirmationStatus::kConfirmed; + result.options = default_immersive_options(); + + std::move(confirm_callback).Run(std::move(result)); + + EXPECT_FALSE(GetController()->active_session_for_testing()); + EXPECT_FALSE(session_remote_out); + EXPECT_EQ(gfx::Size(), window_size_out); +} + +TEST_F(PictureInPictureServiceImplTest, + StartSession_InactiveFrameInvalidatesPendingImmersiveConfirmation) { + EnterFullscreen(); + + EXPECT_CALL(delegate(), IsImmersivePlaybackEnabled()) + .WillRepeatedly(testing::Return(true)); + + base::OnceCallback<void(ImmersivePlaybackConfirmationResult)> + confirm_callback; + EXPECT_CALL(delegate(), RequestImmersivePlaybackConfirmation(_, _)) + .WillOnce( + [&confirm_callback]( + const ImmersiveOptions& options, + base::OnceCallback<void(ImmersivePlaybackConfirmationResult)> + callback) { confirm_callback = std::move(callback); }); + + DummyPictureInPictureSessionObserver observer; + mojo::Receiver<blink::mojom::PictureInPictureSessionObserver> + observer_receiver(&observer); + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> + observer_remote; + observer_receiver.Bind(observer_remote.InitWithNewPipeAndPassReceiver()); + + mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out; + gfx::Size window_size_out; + service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(), + surface_id(), window_size(), show_play_pause_button(), + std::move(observer_remote), source_bounds(), + /*request_immersive=*/true, spatial_format(), + BindSession(session_remote_out, window_size_out)); + + ASSERT_TRUE(confirm_callback); + + // Now frame becomes inactive and sends another StartSession call. + main_test_rfh()->SetLifecycleState( + RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers); + ASSERT_FALSE(main_test_rfh()->IsActive()); + + DummyPictureInPictureSessionObserver observer2; + mojo::Receiver<blink::mojom::PictureInPictureSessionObserver> + observer_receiver2(&observer2); + mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver> + observer_remote2; + observer_receiver2.Bind(observer_remote2.InitWithNewPipeAndPassReceiver()); + + mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out2; + gfx::Size window_size_out2; + service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(), + surface_id(), window_size(), show_play_pause_button(), + std::move(observer_remote2), source_bounds(), + /*request_immersive=*/false, spatial_format(), + BindSession(session_remote_out2, window_size_out2)); + + EXPECT_FALSE(session_remote_out2); + + // Later, the first confirmation runs, but its weak ptr was invalidated by + // the StartSession call. + ImmersivePlaybackConfirmationResult result; + result.status = ImmersivePlaybackConfirmationStatus::kConfirmed; + result.options = default_immersive_options(); + std::move(confirm_callback).Run(std::move(result)); + + EXPECT_FALSE(GetController()->active_session_for_testing()); + EXPECT_FALSE(session_remote_out); +} + } // namespace content
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc b/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc
index fc76188..aa92fa33 100644
--- a/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc
+++ b/content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc
@@ -790,4 +790,196 @@
EXPECT_FALSE(first_session_remote);
}
+TEST_F(PictureInPictureServiceImplTest,
+ StartSession_InactiveFrameRunningUnloadHandlers) {
+ main_test_rfh()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers);
+ ASSERT_FALSE(main_test_rfh()->IsActive());
+
+ mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>
+ observer_remote;
+ mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out;
+ gfx::Size window_size_out;
+ service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(),
+ surface_id(), window_size(), show_play_pause_button(),
+ std::move(observer_remote), source_bounds(),
+ /*request_immersive=*/false, spatial_format(),
+ BindSession(session_remote_out, window_size_out));
+
+ EXPECT_FALSE(GetController()->active_session_for_testing());
+ EXPECT_FALSE(session_remote_out);
+ EXPECT_EQ(gfx::Size(), window_size_out);
+}
+
+TEST_F(PictureInPictureServiceImplTest,
+ StartSession_InactiveFrameInBackForwardCache) {
+ main_test_rfh()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kInBackForwardCache);
+ ASSERT_FALSE(main_test_rfh()->IsActive());
+
+ mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>
+ observer_remote;
+ mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out;
+ gfx::Size window_size_out;
+ service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(),
+ surface_id(), window_size(), show_play_pause_button(),
+ std::move(observer_remote), source_bounds(),
+ /*request_immersive=*/false, spatial_format(),
+ BindSession(session_remote_out, window_size_out));
+
+ EXPECT_FALSE(GetController()->active_session_for_testing());
+ EXPECT_FALSE(session_remote_out);
+ EXPECT_EQ(gfx::Size(), window_size_out);
+}
+
+TEST_F(PictureInPictureServiceImplTest,
+ StartSessionImmersive_InactiveFrameFails) {
+ EnterFullscreen();
+
+ EXPECT_CALL(delegate(), IsImmersivePlaybackEnabled())
+ .WillRepeatedly(testing::Return(true));
+
+ main_test_rfh()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers);
+ ASSERT_FALSE(main_test_rfh()->IsActive());
+
+ // Delegate confirmation should never be requested for an inactive frame.
+ EXPECT_CALL(delegate(), RequestImmersivePlaybackConfirmation(_, _)).Times(0);
+
+ DummyPictureInPictureSessionObserver observer;
+ mojo::Receiver<blink::mojom::PictureInPictureSessionObserver>
+ observer_receiver(&observer);
+ mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>
+ observer_remote;
+ observer_receiver.Bind(observer_remote.InitWithNewPipeAndPassReceiver());
+
+ mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out;
+ gfx::Size window_size_out;
+ service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(),
+ surface_id(), window_size(), show_play_pause_button(),
+ std::move(observer_remote), source_bounds(),
+ /*request_immersive=*/true, spatial_format(),
+ BindSession(session_remote_out, window_size_out));
+
+ EXPECT_FALSE(GetController()->active_session_for_testing());
+ EXPECT_FALSE(session_remote_out);
+ EXPECT_EQ(gfx::Size(), window_size_out);
+}
+
+TEST_F(PictureInPictureServiceImplTest,
+ StartSessionImmersive_BecomesInactiveDuringConfirmationFails) {
+ EnterFullscreen();
+
+ EXPECT_CALL(delegate(), IsImmersivePlaybackEnabled())
+ .WillRepeatedly(testing::Return(true));
+
+ base::OnceCallback<void(ImmersivePlaybackConfirmationResult)>
+ confirm_callback;
+ EXPECT_CALL(delegate(), RequestImmersivePlaybackConfirmation(_, _))
+ .WillOnce(
+ [&confirm_callback](
+ const ImmersiveOptions& options,
+ base::OnceCallback<void(ImmersivePlaybackConfirmationResult)>
+ callback) { confirm_callback = std::move(callback); });
+
+ DummyPictureInPictureSessionObserver observer;
+ mojo::Receiver<blink::mojom::PictureInPictureSessionObserver>
+ observer_receiver(&observer);
+ mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>
+ observer_remote;
+ observer_receiver.Bind(observer_remote.InitWithNewPipeAndPassReceiver());
+
+ mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out;
+ gfx::Size window_size_out;
+ service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(),
+ surface_id(), window_size(), show_play_pause_button(),
+ std::move(observer_remote), source_bounds(),
+ /*request_immersive=*/true, spatial_format(),
+ BindSession(session_remote_out, window_size_out));
+
+ ASSERT_TRUE(confirm_callback);
+ EXPECT_FALSE(GetController()->active_session_for_testing());
+
+ // Frame becomes inactive before user confirmation finishes.
+ main_test_rfh()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers);
+ ASSERT_FALSE(main_test_rfh()->IsActive());
+
+ ImmersivePlaybackConfirmationResult result;
+ result.status = ImmersivePlaybackConfirmationStatus::kConfirmed;
+ result.options = default_immersive_options();
+
+ std::move(confirm_callback).Run(std::move(result));
+
+ EXPECT_FALSE(GetController()->active_session_for_testing());
+ EXPECT_FALSE(session_remote_out);
+ EXPECT_EQ(gfx::Size(), window_size_out);
+}
+
+TEST_F(PictureInPictureServiceImplTest,
+ StartSession_InactiveFrameInvalidatesPendingImmersiveConfirmation) {
+ EnterFullscreen();
+
+ EXPECT_CALL(delegate(), IsImmersivePlaybackEnabled())
+ .WillRepeatedly(testing::Return(true));
+
+ base::OnceCallback<void(ImmersivePlaybackConfirmationResult)>
+ confirm_callback;
+ EXPECT_CALL(delegate(), RequestImmersivePlaybackConfirmation(_, _))
+ .WillOnce(
+ [&confirm_callback](
+ const ImmersiveOptions& options,
+ base::OnceCallback<void(ImmersivePlaybackConfirmationResult)>
+ callback) { confirm_callback = std::move(callback); });
+
+ DummyPictureInPictureSessionObserver observer;
+ mojo::Receiver<blink::mojom::PictureInPictureSessionObserver>
+ observer_receiver(&observer);
+ mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>
+ observer_remote;
+ observer_receiver.Bind(observer_remote.InitWithNewPipeAndPassReceiver());
+
+ mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out;
+ gfx::Size window_size_out;
+ service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(),
+ surface_id(), window_size(), show_play_pause_button(),
+ std::move(observer_remote), source_bounds(),
+ /*request_immersive=*/true, spatial_format(),
+ BindSession(session_remote_out, window_size_out));
+
+ ASSERT_TRUE(confirm_callback);
+
+ // Now frame becomes inactive and sends another StartSession call.
+ main_test_rfh()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kRunningUnloadHandlers);
+ ASSERT_FALSE(main_test_rfh()->IsActive());
+
+ DummyPictureInPictureSessionObserver observer2;
+ mojo::Receiver<blink::mojom::PictureInPictureSessionObserver>
+ observer_receiver2(&observer2);
+ mojo::PendingRemote<blink::mojom::PictureInPictureSessionObserver>
+ observer_remote2;
+ observer_receiver2.Bind(observer_remote2.InitWithNewPipeAndPassReceiver());
+
+ mojo::Remote<blink::mojom::PictureInPictureSession> session_remote_out2;
+ gfx::Size window_size_out2;
+ service().StartSession(player_id(), BindMediaPlayerReceiverAndPassRemote(),
+ surface_id(), window_size(), show_play_pause_button(),
+ std::move(observer_remote2), source_bounds(),
+ /*request_immersive=*/false, spatial_format(),
+ BindSession(session_remote_out2, window_size_out2));
+
+ EXPECT_FALSE(session_remote_out2);
+
+ // Later, the first confirmation runs, but its weak ptr was invalidated by
+ // the StartSession call.
+ ImmersivePlaybackConfirmationResult result;
+ result.status = ImmersivePlaybackConfirmationStatus::kConfirmed;
+ result.options = default_immersive_options();
+ std::move(confirm_callback).Run(std::move(result));
+
+ EXPECT_FALSE(GetController()->active_session_for_testing());
+ EXPECT_FALSE(session_remote_out);
+}
+
} // namespace content
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.
References
On This Page