diff --git a/chrome/browser/ui/exclusive_access/BUILD.gn b/chrome/browser/ui/exclusive_access/BUILD.gn index e49cb4b123c4f..29e6be6a34900 100644 --- a/chrome/browser/ui/exclusive_access/BUILD.gn +++ b/chrome/browser/ui/exclusive_access/BUILD.gn @@ -104,6 +104,7 @@ if (!is_android) { sources = [ "exclusive_access_bubble_type_unittest.cc", "exclusive_access_bubble_unittest.cc", + "exclusive_access_manager_lifetime_unittest.cc", "exclusive_access_permission_manager_unittest.cc", "fullscreen_controller_state_unittest.cc", ] diff --git a/chrome/browser/ui/exclusive_access/exclusive_access_manager_lifetime_unittest.cc b/chrome/browser/ui/exclusive_access/exclusive_access_manager_lifetime_unittest.cc new file mode 100644 index 0000000000000..d19d9d2b533de --- /dev/null +++ b/chrome/browser/ui/exclusive_access/exclusive_access_manager_lifetime_unittest.cc @@ -0,0 +1,126 @@ +// Copyright 2025 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "base/memory/raw_ptr.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_tabstrip.h" +#include "chrome/browser/ui/exclusive_access/exclusive_access_context.h" +#include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h" +#include "chrome/browser/ui/exclusive_access/fullscreen_controller.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/test/base/browser_with_test_window_test.h" +#include "content/public/browser/web_contents.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// An ExclusiveAccessContext whose ExitFullscreen() simulates the documented +// Widget::SetFullscreen() destruction hazard. Chromium documents in-tree that +// the BrowserWidget can be destroyed synchronously inside SetFullscreen() +// (see the WeakPtr early-returns in ui/views/widget/widget.cc SetFullscreen, +// ui/views/win/hwnd_message_handler.cc SetFullscreen, +// ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc +// SetFullscreen, ui/ozone/platform/x11/x11_window.cc SetFullscreen, and the +// "This may spin the message loop and destroy this object" comment in +// content/browser/web_contents/web_contents_impl.cc ExitFullscreenModeForTab). +// When that happens in production, ~BrowserWidget() runs +// BrowserWindowFeatures::TearDownPreBrowserWindowDestruction() which resets +// the heap-owned ExclusiveAccessManager. +class HazardContext : public ExclusiveAccessContext { + public: + explicit HazardContext(Browser* browser) : browser_(browser) {} + + // ExclusiveAccessContext: + Profile* GetProfile() override { return browser_->profile(); } + bool IsFullscreen() const override { return is_fullscreen_; } + void EnterFullscreen(const url::Origin&, + ExclusiveAccessBubbleType, + FullscreenTabParams) override { + is_fullscreen_ = true; + } + void ExitFullscreen() override { + is_fullscreen_ = false; + if (manager_owner_ && *manager_owner_) { + // Simulates ~BrowserWidget -> TearDownPreBrowserWindowDestruction() + // -> exclusive_access_manager_.reset(), which the in-tree WeakPtr + // guards lower on this same call stack expect to be possible. + manager_owner_->reset(); + } + } + void UpdateExclusiveAccessBubble( + const ExclusiveAccessBubbleParams&, + ExclusiveAccessBubbleHideCallback) override {} + bool IsExclusiveAccessBubbleDisplayed() const override { return false; } + void OnExclusiveAccessUserInput() override {} + content::WebContents* GetWebContentsForExclusiveAccess() override { + return browser_->tab_strip_model()->GetActiveWebContents(); + } + bool CanUserEnterFullscreen() const override { return true; } + bool CanUserExitFullscreen() const override { return true; } + + void set_manager_owner(std::unique_ptr* owner) { + manager_owner_ = owner; + } + + private: + const raw_ptr browser_; + raw_ptr> manager_owner_ = nullptr; + bool is_fullscreen_ = false; +}; + +class ExclusiveAccessManagerLifetimeTest : public BrowserWithTestWindowTest {}; + +// ExclusiveAccessManager::OnTabClosing() (and six sibling fan-out loops) +// range-iterates |exclusive_access_controllers_|, a base::flat_set whose +// backing vector lives on the heap. Iteration #1 dispatches to +// FullscreenController, which reaches ExclusiveAccessContext::ExitFullscreen() +// -> Widget::SetFullscreen(). The in-tree WeakPtr guards on that path +// document that the BrowserWidget (and thus the heap-owned +// ExclusiveAccessManager) can be destroyed synchronously there. None of the +// frames above SetFullscreen take a WeakPtr, so on return: +// - FullscreenController::ExitFullscreenModeInternal() writes to freed +// `this->extension_url_` and reads freed `this->manager_`; +// - ExclusiveAccessControllerBase::OnTabClosing() does a virtual call on +// freed `this`; +// - ExclusiveAccessManager::OnTabClosing() advances and dereferences a +// range-for iterator into the freed flat_set backing vector, then makes +// a virtual call through the (freed, sprayable) element. +TEST_F(ExclusiveAccessManagerLifetimeTest, OnTabClosingSetFullscreenHazard) { + AddTab(browser(), GURL("about:blank")); + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + ASSERT_TRUE(web_contents); + + HazardContext context(browser()); + std::unique_ptr manager = + std::make_unique(&context); + + // Enter tab-fullscreen so iteration #1 will reach + // FullscreenController::ExitFullscreenModeInternal(). + manager->fullscreen_controller()->EnterFullscreenModeForTab( + web_contents->GetPrimaryMainFrame()); + ASSERT_TRUE( + manager->fullscreen_controller()->IsWindowFullscreenForTabOrPending()); + ASSERT_TRUE(context.IsFullscreen()); + + // Arm the hazard: ExitFullscreen() will now destroy |manager|, exactly as + // ~BrowserWidget -> TearDownPreBrowserWindowDestruction() does in + // production when the widget dies inside Widget::SetFullscreen(). + context.set_manager_owner(&manager); + + // Range-for iteration #1 reaches ExitFullscreen() (which frees |manager| + // and the flat_set backing vector). The unguarded tail of + // FullscreenController::ExitFullscreenModeInternal() and the remaining + // loop iterations then operate on freed heap. + ExclusiveAccessManager* manager_raw = manager.get(); + manager_raw->OnTabClosing(web_contents); + + // If the lifetime is properly guarded, |manager| was reset and execution + // returned safely. + EXPECT_FALSE(manager); +} + +} // namespace