|
|
@@ -0,0 +1,135 @@
|
|
|
+From 5e3a738b1204941aab9f15c0eb3d06e20fefd96e Mon Sep 17 00:00:00 2001
|
|
|
+From: Scott Violet <sky@chromium.org>
|
|
|
+Date: Mon, 8 Mar 2021 21:07:39 +0000
|
|
|
+Subject: [PATCH] x11/ozone: fix two edge cases
|
|
|
+
|
|
|
+WindowTreeHost::OnHostMovedInPixels() may trigger a nested message
|
|
|
+loop (tab dragging), which when the stack unravels means this may
|
|
|
+be deleted. This adds an early out if this happens.
|
|
|
+
|
|
|
+X11WholeScreenMoveLoop has a similar issue, in so far as notifying
|
|
|
+the delegate may delete this.
|
|
|
+
|
|
|
+BUG=1185482
|
|
|
+TEST=WindowTreeHostPlatform.DeleteHostFromOnHostMovedInPixels
|
|
|
+
|
|
|
+Change-Id: Ieca1c90b3e4358da50b332abe2941fdbb50c5c25
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2743555
|
|
|
+Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
|
|
|
+Commit-Queue: Scott Violet <sky@chromium.org>
|
|
|
+Cr-Commit-Position: refs/heads/master@{#860852}
|
|
|
+---
|
|
|
+ ui/aura/window_tree_host_platform.cc | 10 ++++-
|
|
|
+ ui/aura/window_tree_host_platform_unittest.cc | 40 ++++++++++++++++++-
|
|
|
+ ui/base/x/x11_whole_screen_move_loop.cc | 4 ++
|
|
|
+ 3 files changed, 51 insertions(+), 3 deletions(-)
|
|
|
+
|
|
|
+diff --git a/ui/aura/window_tree_host_platform.cc b/ui/aura/window_tree_host_platform.cc
|
|
|
+index ce8395fe07..7589542026 100644
|
|
|
+--- a/ui/aura/window_tree_host_platform.cc
|
|
|
++++ b/ui/aura/window_tree_host_platform.cc
|
|
|
+@@ -214,13 +214,21 @@ void WindowTreeHostPlatform::OnBoundsChanged(const gfx::Rect& new_bounds) {
|
|
|
+ float current_scale = compositor()->device_scale_factor();
|
|
|
+ float new_scale = ui::GetScaleFactorForNativeView(window());
|
|
|
+ gfx::Rect old_bounds = bounds_in_pixels_;
|
|
|
++ auto weak_ref = GetWeakPtr();
|
|
|
+ bounds_in_pixels_ = new_bounds;
|
|
|
+- if (bounds_in_pixels_.origin() != old_bounds.origin())
|
|
|
++ if (bounds_in_pixels_.origin() != old_bounds.origin()) {
|
|
|
+ OnHostMovedInPixels(bounds_in_pixels_.origin());
|
|
|
++ // Changing the bounds may destroy this.
|
|
|
++ if (!weak_ref)
|
|
|
++ return;
|
|
|
++ }
|
|
|
+ if (bounds_in_pixels_.size() != old_bounds.size() ||
|
|
|
+ current_scale != new_scale) {
|
|
|
+ pending_size_ = gfx::Size();
|
|
|
+ OnHostResizedInPixels(bounds_in_pixels_.size());
|
|
|
++ // Changing the size may destroy this.
|
|
|
++ if (!weak_ref)
|
|
|
++ return;
|
|
|
+ }
|
|
|
+ DCHECK_GT(on_bounds_changed_recursion_depth_, 0);
|
|
|
+ if (--on_bounds_changed_recursion_depth_ == 0) {
|
|
|
+diff --git a/ui/aura/window_tree_host_platform_unittest.cc b/ui/aura/window_tree_host_platform_unittest.cc
|
|
|
+index eda14e2f0c..4de039c88a 100644
|
|
|
+--- a/ui/aura/window_tree_host_platform_unittest.cc
|
|
|
++++ b/ui/aura/window_tree_host_platform_unittest.cc
|
|
|
+@@ -34,7 +34,7 @@ class TestWindowTreeHost : public WindowTreeHostPlatform {
|
|
|
+ // OnHostWill/DidProcessBoundsChange. Additionally, this triggers a bounds
|
|
|
+ // change from within OnHostResized(). Such a scenario happens in production
|
|
|
+ // code.
|
|
|
+-class TestWindowTreeHostObserver : public aura::WindowTreeHostObserver {
|
|
|
++class TestWindowTreeHostObserver : public WindowTreeHostObserver {
|
|
|
+ public:
|
|
|
+ TestWindowTreeHostObserver(WindowTreeHostPlatform* host,
|
|
|
+ ui::PlatformWindow* platform_window)
|
|
|
+@@ -51,7 +51,7 @@ class TestWindowTreeHostObserver : public aura::WindowTreeHostObserver {
|
|
|
+ return on_host_will_process_bounds_change_count_;
|
|
|
+ }
|
|
|
+
|
|
|
+- // aura::WindowTreeHostObserver:
|
|
|
++ // WindowTreeHostObserver:
|
|
|
+ void OnHostResized(WindowTreeHost* host) override {
|
|
|
+ if (!should_change_bounds_in_on_resized_)
|
|
|
+ return;
|
|
|
+@@ -92,5 +92,41 @@ TEST_F(WindowTreeHostPlatformTest, HostWillProcessBoundsChangeRecursion) {
|
|
|
+ EXPECT_EQ(1, observer.on_host_will_process_bounds_change_count());
|
|
|
+ }
|
|
|
+
|
|
|
++// Deletes WindowTreeHostPlatform from OnHostMovedInPixels().
|
|
|
++class DeleteHostWindowTreeHostObserver : public WindowTreeHostObserver {
|
|
|
++ public:
|
|
|
++ explicit DeleteHostWindowTreeHostObserver(
|
|
|
++ std::unique_ptr<TestWindowTreeHost> host)
|
|
|
++ : host_(std::move(host)) {
|
|
|
++ host_->AddObserver(this);
|
|
|
++ }
|
|
|
++ ~DeleteHostWindowTreeHostObserver() override = default;
|
|
|
++
|
|
|
++ TestWindowTreeHost* host() { return host_.get(); }
|
|
|
++
|
|
|
++ // WindowTreeHostObserver:
|
|
|
++ void OnHostMovedInPixels(WindowTreeHost* host,
|
|
|
++ const gfx::Point& new_origin_in_pixels) override {
|
|
|
++ host_->RemoveObserver(this);
|
|
|
++ host_.reset();
|
|
|
++ }
|
|
|
++
|
|
|
++ private:
|
|
|
++ std::unique_ptr<TestWindowTreeHost> host_;
|
|
|
++
|
|
|
++ DISALLOW_COPY_AND_ASSIGN(DeleteHostWindowTreeHostObserver);
|
|
|
++};
|
|
|
++
|
|
|
++// Verifies WindowTreeHostPlatform can be safely deleted when calling
|
|
|
++// OnHostMovedInPixels().
|
|
|
++// Regression test for https://crbug.com/1185482
|
|
|
++TEST_F(WindowTreeHostPlatformTest, DeleteHostFromOnHostMovedInPixels) {
|
|
|
++ std::unique_ptr<TestWindowTreeHost> host =
|
|
|
++ std::make_unique<TestWindowTreeHost>();
|
|
|
++ DeleteHostWindowTreeHostObserver observer(std::move(host));
|
|
|
++ observer.host()->SetBoundsInPixels(gfx::Rect(1, 2, 3, 4));
|
|
|
++ EXPECT_EQ(nullptr, observer.host());
|
|
|
++}
|
|
|
++
|
|
|
+ } // namespace
|
|
|
+ } // namespace aura
|
|
|
+diff --git a/ui/base/x/x11_whole_screen_move_loop.cc b/ui/base/x/x11_whole_screen_move_loop.cc
|
|
|
+index 5ed215db66..db678799db 100644
|
|
|
+--- a/ui/base/x/x11_whole_screen_move_loop.cc
|
|
|
++++ b/ui/base/x/x11_whole_screen_move_loop.cc
|
|
|
+@@ -78,9 +78,13 @@ X11WholeScreenMoveLoop::~X11WholeScreenMoveLoop() {
|
|
|
+ void X11WholeScreenMoveLoop::DispatchMouseMovement() {
|
|
|
+ if (!last_motion_in_screen_)
|
|
|
+ return;
|
|
|
++ auto weak_ref = weak_factory_.GetWeakPtr();
|
|
|
+ delegate_->OnMouseMovement(last_motion_in_screen_->root_location(),
|
|
|
+ last_motion_in_screen_->flags(),
|
|
|
+ last_motion_in_screen_->time_stamp());
|
|
|
++ // The delegate may delete this during dispatch.
|
|
|
++ if (!weak_ref)
|
|
|
++ return;
|
|
|
+ last_motion_in_screen_.reset();
|
|
|
+ }
|
|
|
+
|
jk