diff --git a/ui/base/x/BUILD.gn b/ui/base/x/BUILD.gn index 190c5c03abf15..2d8dcff05650c 100644 --- a/ui/base/x/BUILD.gn +++ b/ui/base/x/BUILD.gn @@ -167,6 +167,7 @@ source_set("unittests") { "x11_cursor_loader_unittest.cc", "x11_desktop_window_move_client_unittest.cc", "x11_display_util_unittest.cc", + "x11_whole_screen_move_loop_unittest.cc", ] deps = [ ":x", @@ -176,6 +177,7 @@ source_set("unittests") { "//testing/gtest", "//ui/base/cursor", "//ui/base/cursor/mojom:cursor_type_shared", + "//ui/events/platform/x11", "//ui/gfx/geometry", "//ui/gfx/x", ] diff --git a/ui/base/x/x11_whole_screen_move_loop_unittest.cc b/ui/base/x/x11_whole_screen_move_loop_unittest.cc new file mode 100644 index 0000000000000..55b7370d0dec9 --- /dev/null +++ b/ui/base/x/x11_whole_screen_move_loop_unittest.cc @@ -0,0 +1,98 @@ +// Copyright 2026 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/base/x/x11_whole_screen_move_loop.h" + +#include + +#include "base/functional/callback_helpers.h" +#include "base/location.h" +#include "base/memory/raw_ptr.h" +#include "base/task/single_thread_task_runner.h" +#include "base/test/task_environment.h" +#include "base/time/time.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/base/x/x11_cursor.h" +#include "ui/base/x/x11_move_loop_delegate.h" +#include "ui/events/platform/x11/x11_event_source.h" +#include "ui/gfx/geometry/point.h" +#include "ui/gfx/x/connection.h" + +namespace ui { + +namespace { + +// Delegate that models the X11Window ownership: the source X11Window owns the +// move loop via unique_ptr (X11Window::drag_loop_). When the source window is +// destroyed while EndMoveLoop() is on the stack, the unique_ptr is reset, +// which runs ~X11WholeScreenMoveLoop -> EndMoveLoop() re-entrantly and then +// frees the object. +class SelfDeletingDelegate : public X11MoveLoopDelegate { + public: + SelfDeletingDelegate() = default; + ~SelfDeletingDelegate() override = default; + + // X11MoveLoopDelegate: + void OnMouseMovement(const gfx::Point& screen_point, + int flags, + base::TimeTicks event_time) override {} + void OnMouseReleased() override {} + void OnMoveLoopEnded() override { + ++on_move_loop_ended_count_; + // Mirrors ~X11Window destroying its drag_loop_ member while the outer + // EndMoveLoop() frame is still live at the delegate_->OnMoveLoopEnded() + // callout. unique_ptr::reset() nulls the stored pointer first, so the + // recursive OnMoveLoopEnded() sees a null holder and does not double-free. + move_loop_holder_->reset(); + } + + void set_move_loop_holder( + std::unique_ptr* holder) { + move_loop_holder_ = holder; + } + + int on_move_loop_ended_count() const { return on_move_loop_ended_count_; } + + private: + raw_ptr> move_loop_holder_ = nullptr; + int on_move_loop_ended_count_ = 0; +}; + +} // namespace + +// Regression test: EndMoveLoop() must survive |this| being freed from within +// delegate_->OnMoveLoopEnded(). Without a WeakPtr guard after the delegate +// callout, the tail of EndMoveLoop() (grab_input_window_events_.Reset() ... +// std::move(quit_closure_).Run()) touches freed |this| and ASAN reports +// heap-use-after-free. +TEST(X11WholeScreenMoveLoopTest, EndMoveLoopSurvivesSelfDeletion) { + base::test::TaskEnvironment task_env{ + base::test::TaskEnvironment::MainThreadType::UI}; + X11EventSource event_source(x11::Connection::Get()); + + SelfDeletingDelegate delegate; + auto move_loop = std::make_unique(&delegate); + delegate.set_move_loop_holder(&move_loop); + X11WholeScreenMoveLoop* raw = move_loop.get(); + + // Once RunMoveLoop() enters its nested run loop, invoke EndMoveLoop() from a + // posted task, mirroring the Esc-key / CancelDrag entry points. + base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, base::BindOnce(&X11WholeScreenMoveLoop::EndMoveLoop, + base::Unretained(raw))); + + // can_grab_pointer=false so the pointer grab is skipped (no reliance on + // interactive X server state). + bool result = raw->RunMoveLoop( + /*can_grab_pointer=*/false, /*old_cursor=*/nullptr, + /*new_cursor=*/nullptr, /*started_callback=*/base::DoNothing()); + + // If we get here, RunMoveLoop's own |alive| guard caught the deletion. + EXPECT_FALSE(result); + EXPECT_FALSE(move_loop); + // Outer EndMoveLoop + recursive EndMoveLoop from the destructor. + EXPECT_EQ(2, delegate.on_move_loop_ended_count()); +} + +} // namespace ui