Chrome · Fullscreen
CVE-2026-5882
Logic Error in Fullscreen
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcontent/browser/web_contents/web_contents_impl.cc |
modified | |
fullscreen_frames_for_testingcontent/browser/web_contents/web_contents_impl.h |
modified |
Files Changed
content/browser/renderer_host/navigator.cccontent/browser/renderer_host/navigator_delegate.hcontent/browser/web_contents/web_contents_impl.cccontent/browser/web_contents/web_contents_impl.hcontent/browser/web_contents/web_contents_impl_browsertest.cc
Patch
From 9f026d281411d1a5d8996656192c4765955befb0 Mon Sep 17 00:00:00 2001 From: Muyao Xu <[email protected]> Date: Fri, 06 Mar 2026 16:26:08 -0800 Subject: [PATCH] [Fullscreen] Exit fullscreen on iframe navigation Currently, the browser only exits fullscreen mode when the main frame navigates away. This CL extends this protection to all frames by moving the fullscreen exit logic into a new `DidNavigateAnyFramePreCommit` function. Fullscreen will exit before committing a cross-document navigation if: - The navigating frame is the primary main frame. - The navigating frame is a subframe, and that subframe (or one of its descendants) is currently the frame that requested fullscreen. Fullscreen will NOT exit if: - The main frame is in fullscreen, and a subframe navigates. - The navigation is a same-document navigation. Bug: 480993682 Change-Id: I0c5166ab3e653dfb2e0b2f6a27577ac51f9c966b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7634575 Reviewed-by: Nasko Oskov <[email protected]> Commit-Queue: Muyao Xu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1595785} --- diff --git a/content/browser/renderer_host/navigator.cc b/content/browser/renderer_host/navigator.cc index 12bf5f4..c63cd9d 100644 --- a/content/browser/renderer_host/navigator.cc +++ b/content/browser/renderer_host/navigator.cc @@ -525,8 +525,11 @@ } #endif // BUILDFLAG(IS_ANDROID) + // Run tasks that must execute just before the commit. + delegate_->DidNavigateAnyFramePreCommit(navigation_request.get(), + was_within_same_document); + if (ui::PageTransitionIsMainFrame(params.transition)) { - // Run tasks that must execute just before the commit. delegate_->DidNavigateMainFramePreCommit(navigation_request.get(), was_within_same_document); } diff --git a/content/browser/renderer_host/navigator_delegate.h b/content/browser/renderer_host/navigator_delegate.h index b81c776e..cb801508 100644 --- a/content/browser/renderer_host/navigator_delegate.h +++ b/content/browser/renderer_host/navigator_delegate.h @@ -101,6 +101,8 @@ virtual void DidNavigateMainFramePreCommit( NavigationHandle* navigation_handle, bool navigation_is_within_page) = 0; + virtual void DidNavigateAnyFramePreCommit(NavigationHandle* navigation_handle, + bool navigation_is_within_page) = 0; // Handles post-navigation tasks in navigation AFTER the entry has been // committed to the NavigationController. Note that the NavigationEntry is diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 56bf36a..382b9ad9 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -7736,28 +7736,57 @@ } #endif - // Ensure fullscreen mode is exited before committing the navigation to a - // different page. The next page will not start out assuming it is in - // fullscreen mode. if (navigation_is_within_page) { - // No page change? Then, the renderer and browser can remain in fullscreen. return; } - if (IsFullscreen()) { - ExitFullscreen(false); - } - auto* rwhvb = static_cast<RenderWidgetHostViewBase*>( frame_tree_node->current_frame_host()->GetView()); if (rwhvb) { rwhvb->OnOldViewDidNavigatePreCommit(); } - // Clean up keyboard lock state when navigating. CancelKeyboardLock(keyboard_lock_widget_); } +void WebContentsImpl::DidNavigateAnyFramePreCommit( + NavigationHandle* navigation_handle, + bool navigation_is_within_page) { + // Ensure fullscreen mode is exited before committing the navigation to a + // different page. The next page will not start out assuming it is in + // fullscreen mode. + if (navigation_is_within_page || !IsFullscreen()) { + return; + } + + bool should_exit_fullscreen = false; + if (navigation_handle->IsInPrimaryMainFrame()) { + should_exit_fullscreen = true; + } else { + // For iframe navigation, exit if the fullscreen was requested by the + // iframe or one of its descendants. + const FrameTreeNodeId navigating_id = + navigation_handle->GetFrameTreeNodeId(); + should_exit_fullscreen = + std::any_of(fullscreen_frames_.begin(), fullscreen_frames_.end(), + [navigating_id](RenderFrameHostImpl* rfh) { + for (RenderFrameHostImpl* current = rfh; current; + current = current->GetParentOrOuterDocument()) { + if (current->frame_tree_node()->frame_tree_node_id() == + navigating_id) { + return true; + } + } + return false; + }); + } + + if (should_exit_fullscreen) { + ExitFullscreen(false); + CancelKeyboardLock(keyboard_lock_widget_); + } +} + void WebContentsImpl::DidNavigateMainFramePostCommit( RenderFrameHostImpl* render_frame_host, const LoadCommittedDetails& details) { diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 6db10482..e94fdbc 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -1062,6 +1062,8 @@ NavigationHandle* navigation_handle) override; void DidNavigateMainFramePreCommit(NavigationHandle* navigation_handle, bool navigation_is_within_page) override; + void DidNavigateAnyFramePreCommit(NavigationHandle* navigation_handle, + bool navigation_is_within_page) override; void DidNavigateMainFramePostCommit( RenderFrameHostImpl* render_frame_host, const LoadCommittedDetails& details) override; @@ -1598,6 +1600,15 @@ return pointer_lock_widget_; } + const std::set<raw_ptr<RenderFrameHostImpl, SetExperimental>>& + fullscreen_frames_for_testing() const { + return fullscreen_frames_; + } + + GlobalRenderFrameHostId current_fullscreen_frame_id_for_testing() const { + return current_fullscreen_frame_id_; + } + ui::mojom::VirtualKeyboardMode GetVirtualKeyboardMode() const; const std::optional<base::Location>& ownership_location() const { @@ -1649,8 +1660,6 @@ FRIEND_TEST_ALL_PREFIXES(WebContentsImplBrowserTest, NotifyFullscreenAcquired_Navigate); FRIEND_TEST_ALL_PREFIXES(WebContentsImplBrowserTest, - NotifyFullscreenAcquired_SameOrigin); - FRIEND_TEST_ALL_PREFIXES(WebContentsImplBrowserTest, PropagateFullscreenOptions); FRIEND_TEST_ALL_PREFIXES(WebContentsImplBrowserTest, FullscreenAfterFrameUnload); diff --git a/content/browser/web_contents/web_contents_impl_browsertest.cc b/content/browser/web_contents/web_contents_impl_browsertest.cc index e8f40f2..7a97ad5 100644 --- a/content/browser/web_contents/web_contents_impl_browsertest.cc +++ b/content/browser/web_contents/web_contents_impl_browsertest.cc @@ -170,7 +170,7 @@ bool IsInFullscreen() { WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(shell()->web_contents()); - return !!web_contents->current_fullscreen_frame_id_; + return !!web_contents->current_fullscreen_frame_id_for_testing(); } protected: @@ -3919,7 +3919,7 @@ static_cast<RenderFrameHostImpl*>(ChildFrameAt(main_frame, 0)); std::set<raw_ptr<RenderFrameHostImpl, SetExperimental>> fullscreen_frames; - EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_); + EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing()); EXPECT_FALSE(IsInFullscreen()); // Make the top page fullscreen. @@ -3930,9 +3930,9 @@ } fullscreen_frames.insert(main_frame); - EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_); + EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing()); EXPECT_EQ(main_frame->GetGlobalId(), - web_contents->current_fullscreen_frame_id_); + web_contents->current_fullscreen_frame_id_for_testing()); // Make the child frame fullscreen. {
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/content/browser/web_contents/web_contents_impl_browsertest.cc b/content/browser/web_contents/web_contents_impl_browsertest.cc
index e8f40f2..7a97ad5 100644
--- a/content/browser/web_contents/web_contents_impl_browsertest.cc
+++ b/content/browser/web_contents/web_contents_impl_browsertest.cc
@@ -170,7 +170,7 @@
bool IsInFullscreen() {
WebContentsImpl* web_contents =
static_cast<WebContentsImpl*>(shell()->web_contents());
- return !!web_contents->current_fullscreen_frame_id_;
+ return !!web_contents->current_fullscreen_frame_id_for_testing();
}
protected:
@@ -3919,7 +3919,7 @@
static_cast<RenderFrameHostImpl*>(ChildFrameAt(main_frame, 0));
std::set<raw_ptr<RenderFrameHostImpl, SetExperimental>> fullscreen_frames;
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing());
EXPECT_FALSE(IsInFullscreen());
// Make the top page fullscreen.
@@ -3930,9 +3930,9 @@
}
fullscreen_frames.insert(main_frame);
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(main_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
// Make the child frame fullscreen.
{
@@ -3943,9 +3943,9 @@
}
fullscreen_frames.insert(child_frame);
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(child_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
// Exit fullscreen on the child frame.
// This will not work with --site-per-process until crbug.com/617369
@@ -3958,9 +3958,9 @@
}
fullscreen_frames.erase(child_frame);
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(main_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
}
}
@@ -4012,13 +4012,13 @@
EXPECT_TRUE(NavigateToURL(shell(), url_a));
RenderFrameHostImpl* main_frame =
static_cast<RenderFrameHostImpl*>(web_contents->GetPrimaryMainFrame());
- EXPECT_EQ(0u, web_contents->fullscreen_frames_.size());
+ EXPECT_EQ(0u, web_contents->fullscreen_frames_for_testing().size());
// 2) Make it fullscreen.
FullscreenWebContentsObserver observer(web_contents, main_frame);
EXPECT_TRUE(ExecJs(main_frame, "document.body.webkitRequestFullscreen();"));
observer.Wait();
- EXPECT_EQ(1u, web_contents->fullscreen_frames_.size());
+ EXPECT_EQ(1u, web_contents->fullscreen_frames_for_testing().size());
// 3) Navigate cross origin. Act as if the old frame was very slow delivering
// the unload ack and stayed in pending deletion for a while. Even if the
@@ -4028,7 +4028,7 @@
main_frame->SetUnloadACKCallbackForTesting(unload_ack_filter);
main_frame->DisableUnloadTimerForTesting();
EXPECT_TRUE(NavigateToURL(shell(), url_b));
- EXPECT_EQ(0u, web_contents->fullscreen_frames_.size());
+ EXPECT_EQ(0u, web_contents->fullscreen_frames_for_testing().size());
}
IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
@@ -4047,7 +4047,7 @@
static_cast<RenderFrameHostImpl*>(ChildFrameAt(main_frame, 0));
std::set<raw_ptr<RenderFrameHostImpl, SetExperimental>> nodes;
- EXPECT_EQ(nodes, web_contents->fullscreen_frames_);
+ EXPECT_EQ(nodes, web_contents->fullscreen_frames_for_testing());
EXPECT_FALSE(IsInFullscreen());
// Make the top page fullscreen.
@@ -4058,9 +4058,9 @@
}
nodes.insert(main_frame);
- EXPECT_EQ(nodes, web_contents->fullscreen_frames_);
+ EXPECT_EQ(nodes, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(main_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
// Make the child frame fullscreen.
{
@@ -4071,19 +4071,148 @@
}
nodes.insert(child_frame);
- EXPECT_EQ(nodes, web_contents->fullscreen_frames_);
+ EXPECT_EQ(nodes, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(child_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
// Perform a cross origin navigation on the main frame.
EXPECT_TRUE(
NavigateToURL(shell(), embedded_test_server()->GetURL(
"c.com", "/cross_site_iframe_factory.html")));
- EXPECT_EQ(0u, web_contents->fullscreen_frames_.size());
+ EXPECT_EQ(0u, web_contents->fullscreen_frames_for_testing().size());
EXPECT_FALSE(IsInFullscreen());
}
IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
+ FullscreenNoExitOnIframeNavigate) {
+ ASSERT_TRUE(embedded_test_server()->Start());
+ WebContentsImpl* web_contents =
+ static_cast<WebContentsImpl*>(shell()->web_contents());
+ TestWCDelegateForDialogsAndFullscreen test_delegate(web_contents);
+
+ GURL url = embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(b{allowfullscreen})");
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ RenderFrameHostImpl* main_frame = web_contents->GetPrimaryMainFrame();
+
+ // Make the top page fullscreen.
+ {
+ FullscreenWebContentsObserver observer(web_contents, main_frame);
+ EXPECT_TRUE(ExecJs(main_frame, "document.body.requestFullscreen();"));
+ observer.Wait();
+ }
+ EXPECT_TRUE(web_contents->IsFullscreen());
+
+ // Navigate the iframe to a different page.
+ GURL url_c = embedded_test_server()->GetURL("c.com", "/title1.html");
+ EXPECT_TRUE(
+ content::NavigateToURLFromRenderer(ChildFrameAt(main_frame, 0), url_c));
+
+ // Fullscreen should NOT be exited.
+ EXPECT_TRUE(web_contents->IsFullscreen());
+ EXPECT_EQ(1u, web_contents->fullscreen_frames_for_testing().size());
+}
+
+IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
+ FullscreenNoExitOnIframeSameDocumentNavigate) {
+ ASSERT_TRUE(embedded_test_server()->Start());
+ WebContentsImpl* web_contents =
+ static_cast<WebContentsImpl*>(shell()->web_contents());
+ TestWCDelegateForDialogsAndFullscreen test_delegate(web_contents);
+
+ GURL url = embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(b{allowfullscreen})");
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ auto* main_frame = web_contents->GetPrimaryMainFrame();
+ auto* child_frame = ChildFrameAt(main_frame, 0);
+
+ // Make the top page fullscreen.
+ {
+ FullscreenWebContentsObserver observer(web_contents, main_frame);
+ EXPECT_TRUE(ExecJs(main_frame, "document.body.requestFullscreen();"));
+ observer.Wait();
+ }
+ EXPECT_TRUE(web_contents->IsFullscreen());
+
+ // Navigate the iframe same-document.
+ EXPECT_TRUE(ExecJs(child_frame, "location.hash = 'foo';"));
+ EXPECT_TRUE(WaitForLoadStop(web_contents));
+
+ // Fullscreen should NOT be exited.
+ EXPECT_TRUE(web_contents->IsFullscreen());
+ EXPECT_EQ(1u, web_contents->fullscreen_frames_for_testing().size());
+}
+
+IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
+ FullscreenExitOnIframeNavigateWhileIframeIsFullscreen) {
+ ASSERT_TRUE(embedded_test_server()->Start());
+ WebContentsImpl* web_contents =
+ static_cast<WebContentsImpl*>(shell()->web_contents());
+ TestWCDelegateForDialogsAndFullscreen test_delegate(web_contents);
+ test_delegate.WillWaitForFullscreenExit();
+
+ GURL url = embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(b{allowfullscreen})");
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ auto* main_frame = web_contents->GetPrimaryMainFrame();
+ auto* child_frame = ChildFrameAt(main_frame, 0);
+
+ // Make the child frame fullscreen.
+ {
+ FullscreenWebContentsObserver observer(web_contents, child_frame);
+ EXPECT_TRUE(ExecJs(child_frame, "document.body.requestFullscreen();"));
+ observer.Wait();
+ }
+ EXPECT_TRUE(web_contents->IsFullscreen());
+ EXPECT_EQ(child_frame->GetGlobalId(),
+ web_contents->current_fullscreen_frame_id_for_testing());
+
+ // Navigate the iframe to a different page.
+ GURL url_c = embedded_test_server()->GetURL("c.com", "/title1.html");
+ EXPECT_TRUE(
+ content::NavigateToURLFromRenderer(ChildFrameAt(main_frame, 0), url_c));
+
+ // Fullscreen should be exited.
+ EXPECT_FALSE(web_contents->IsFullscreen());
+ // On Android, it can take some time for `fullscreen_frames_` to get updated.
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ return web_contents->fullscreen_frames_for_testing().empty();
+ })) << "Timed out waiting for fullscreen frames to be cleared.";
+}
+
+IN_PROC_BROWSER_TEST_F(
+ WebContentsImplBrowserTest,
+ FullscreenExitOnMainFrameNavigateWhileIframeIsFullscreen) {
+ ASSERT_TRUE(embedded_test_server()->Start());
+ WebContentsImpl* web_contents =
+ static_cast<WebContentsImpl*>(shell()->web_contents());
+ TestWCDelegateForDialogsAndFullscreen test_delegate(web_contents);
+ test_delegate.WillWaitForFullscreenExit();
+
+ GURL url = embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(b{allowfullscreen})");
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ auto* main_frame = web_contents->GetPrimaryMainFrame();
+ auto* child_frame = ChildFrameAt(main_frame, 0);
+
+ // Make the child frame fullscreen.
+ {
+ FullscreenWebContentsObserver observer(web_contents, child_frame);
+ EXPECT_TRUE(ExecJs(child_frame, "document.body.requestFullscreen();"));
+ observer.Wait();
+ }
+ EXPECT_TRUE(web_contents->IsFullscreen());
+
+ // Navigate the main frame to a different page.
+ GURL url_c = embedded_test_server()->GetURL("c.com", "/title1.html");
+ EXPECT_TRUE(NavigateToURL(shell(), url_c));
+
+ // Fullscreen should be exited.
+ EXPECT_FALSE(web_contents->IsFullscreen());
+ EXPECT_EQ(0u, web_contents->fullscreen_frames_for_testing().size());
+}
+
+IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
NotifyFullscreenAcquired_SameOrigin) {
ASSERT_TRUE(embedded_test_server()->Start());
WebContentsImpl* web_contents =
@@ -4096,9 +4225,8 @@
RenderFrameHostImpl* main_frame = web_contents->GetPrimaryMainFrame();
RenderFrameHostImpl* child_frame =
static_cast<RenderFrameHostImpl*>(ChildFrameAt(main_frame, 0));
-
std::set<raw_ptr<RenderFrameHostImpl, SetExperimental>> fullscreen_frames;
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_TRUE(web_contents->fullscreen_frames_for_testing().empty());
EXPECT_FALSE(IsInFullscreen());
// Make the top page fullscreen.
@@ -4109,9 +4237,9 @@
}
fullscreen_frames.insert(main_frame);
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(main_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
// Make the child frame fullscreen.
{
@@ -4122,9 +4250,9 @@
}
fullscreen_frames.insert(child_frame);
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(child_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
// Exit fullscreen on the child frame.
{
@@ -4134,9 +4262,9 @@
}
fullscreen_frames.erase(child_frame);
- EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_);
+ EXPECT_EQ(fullscreen_frames, web_contents->fullscreen_frames_for_testing());
EXPECT_EQ(main_frame->GetGlobalId(),
- web_contents->current_fullscreen_frame_id_);
+ web_contents->current_fullscreen_frame_id_for_testing());
... (truncated)
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