From 7fe9ada0ba8401ffe390c0e40419c0ce00b77fa0 Mon Sep 17 00:00:00 2001 From: Yongshun Liu Date: Wed, 14 Dec 2022 00:03:36 +0000 Subject: [PATCH] [Merge to M109] ash: Speculative fix for destructor of `ScopedOverviewHideWindows` For now, overview session uses `ScopedOverviewHideWindows` to manage the visibility of a bunch of windows for saved desk library including the overview item window. But when overview is being shutdown, the widget of overview item window may be destroyed before being removed from `ScopedOverviewHideWindows`. The fix is to remove the overview item window from `ScopedOverviewHideWindows` without showing during shutdown. (cherry picked from commit 65544fb8c0a037cb8f0d66c4a15ea2dfdc35478a) Test: Manual Bug: b/260001863 Change-Id: Id55fa5fe79dd3a606bb66e77d9971a3c15538fe6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4099109 Reviewed-by: Sammie Quon Commit-Queue: Yongshun Liu Cr-Original-Commit-Position: refs/heads/main@{#1082695} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4104482 Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/branch-heads/5414@{#706} Cr-Branched-From: 4417ee59d7bf6df7a9c9ea28f7722d2ee6203413-refs/heads/main@{#1070088} --- ash/wm/overview/overview_item.cc | 19 ++++++++++++++++--- .../overview/scoped_overview_hide_windows.cc | 7 ++++--- .../overview/scoped_overview_hide_windows.h | 2 +- .../scoped_overview_transform_window.cc | 2 +- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/ash/wm/overview/overview_item.cc b/ash/wm/overview/overview_item.cc index ba562947062e57..e51f9575f0282a 100644 --- a/ash/wm/overview/overview_item.cc +++ b/ash/wm/overview/overview_item.cc @@ -329,6 +329,16 @@ void OverviewItem::EnsureVisible() { } void OverviewItem::Shutdown() { + // If `hide_windows` still manages the visibility of this overview item + // window, remove it from the list without showing. + ScopedOverviewHideWindows* hide_windows = + overview_session_->hide_windows_for_saved_desks_grid(); + if (item_widget_ && hide_windows && + hide_windows->HasWindow(item_widget_->GetNativeWindow())) { + hide_windows->RemoveWindow(item_widget_->GetNativeWindow(), + /*show_window=*/false); + } + item_widget_.reset(); overview_item_view_ = nullptr; } @@ -1486,13 +1496,16 @@ void OverviewItem::ShowWindowInOverview() { if (hide_windows->HasWindow(GetWindow())) { const bool ignore_activations = overview_session_->ignore_activations(); overview_session_->set_ignore_activations(true); - hide_windows->RemoveWindow(GetWindow()); + hide_windows->RemoveWindow(GetWindow(), /*show_window=*/true); overview_session_->set_ignore_activations(ignore_activations); } // Show the overview item window. - if (item_widget_ && hide_windows->HasWindow(item_widget_->GetNativeWindow())) - hide_windows->RemoveWindow(item_widget_->GetNativeWindow()); + if (item_widget_ && + hide_windows->HasWindow(item_widget_->GetNativeWindow())) { + hide_windows->RemoveWindow(item_widget_->GetNativeWindow(), + /*show_window=*/true); + } } } // namespace ash diff --git a/ash/wm/overview/scoped_overview_hide_windows.cc b/ash/wm/overview/scoped_overview_hide_windows.cc index b8bc893d16ed42..4a5ef97ee6bf74 100644 --- a/ash/wm/overview/scoped_overview_hide_windows.cc +++ b/ash/wm/overview/scoped_overview_hide_windows.cc @@ -43,10 +43,11 @@ void ScopedOverviewHideWindows::AddWindow(aura::Window* window) { window->Hide(); } -void ScopedOverviewHideWindows::RemoveWindow(aura::Window* window) { +void ScopedOverviewHideWindows::RemoveWindow(aura::Window* window, + bool show_window) { DCHECK(HasWindow(window)); window->RemoveObserver(this); - if (!window->is_destroying() && window_visibility_[window]) + if (!window->is_destroying() && window_visibility_[window] && show_window) window->Show(); window_visibility_.erase(window); } @@ -57,7 +58,7 @@ void ScopedOverviewHideWindows::RemoveAllWindows() { for (const auto& element : window_visibility_) windows_to_remove.push_back(element.first); for (auto* window : base::Reversed(windows_to_remove)) - RemoveWindow(window); + RemoveWindow(window, /*show_window=*/true); } void ScopedOverviewHideWindows::OnWindowDestroying(aura::Window* window) { diff --git a/ash/wm/overview/scoped_overview_hide_windows.h b/ash/wm/overview/scoped_overview_hide_windows.h index 5749e7416991a0..cf9d2ea7e06d6b 100644 --- a/ash/wm/overview/scoped_overview_hide_windows.h +++ b/ash/wm/overview/scoped_overview_hide_windows.h @@ -31,7 +31,7 @@ class ScopedOverviewHideWindows : public aura::WindowObserver { bool HasWindow(aura::Window* window) const; void AddWindow(aura::Window* window); - void RemoveWindow(aura::Window* window); + void RemoveWindow(aura::Window* window, bool show_window); void RemoveAllWindows(); // aura::WindowObserver: diff --git a/ash/wm/overview/scoped_overview_transform_window.cc b/ash/wm/overview/scoped_overview_transform_window.cc index e9451f56b4d46b..6cedd0044c4004 100644 --- a/ash/wm/overview/scoped_overview_transform_window.cc +++ b/ash/wm/overview/scoped_overview_transform_window.cc @@ -600,7 +600,7 @@ void ScopedOverviewTransformWindow::OnWindowPropertyChanged( if (current_value) { AddHiddenTransientWindows({window}); } else { - hidden_transient_children_->RemoveWindow(window); + hidden_transient_children_->RemoveWindow(window, /*show_window=*/true); } }