Skip to content

Commit

Permalink
[Merge to M109] ash: Speculative fix for destructor of `ScopedOvervie…
Browse files Browse the repository at this point in the history
…wHideWindows`

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 65544fb)

Test: Manual
Bug: b/260001863
Change-Id: Id55fa5fe79dd3a606bb66e77d9971a3c15538fe6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4099109
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Yongshun Liu <yongshun@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1082695}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4104482
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5414@{#706}
Cr-Branched-From: 4417ee5-refs/heads/main@{#1070088}
  • Loading branch information
Yongshun Liu authored and Chromium LUCI CQ committed Dec 14, 2022
1 parent f35a384 commit 7fe9ada
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 8 deletions.
19 changes: 16 additions & 3 deletions ash/wm/overview/overview_item.cc
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
7 changes: 4 additions & 3 deletions ash/wm/overview/scoped_overview_hide_windows.cc
Expand Up @@ -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);
}
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion ash/wm/overview/scoped_overview_hide_windows.h
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion ash/wm/overview/scoped_overview_transform_window.cc
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 7fe9ada

Please sign in to comment.