Skip to content

Commit

Permalink
Properly filter out transient Crostini windows from the shelf
Browse files Browse the repository at this point in the history
Transient Crostini windows are supposed to be filtered from the shelf,
but we are checking for a transient parent on window Init, which is
before it actually gets set. This was causing a UAF on profile switching
as MultiUserWindowManagerChromeOS::SetWindowOwner() is not supposed to
be called with transient windows.

Bug: 845843
Change-Id: Iec7b48cb03fff22775c31ce08c398ba750836463
Reviewed-on: https://chromium-review.googlesource.com/c/1266475
Commit-Queue: Timothy Loh <[email protected]>
Reviewed-by: Michael Wasserman <[email protected]>
Cr-Commit-Position: refs/heads/master@{#599038}
  • Loading branch information
tim-loh authored and Commit Bot committed Oct 12, 2018
1 parent c6058c3 commit bf10ece
Showing 1 changed file with 13 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,13 @@ void CrostiniAppWindowShelfController::OnWindowInitialized(
aura::Window* window) {
// An Crostini window has type WINDOW_TYPE_NORMAL, a WindowDelegate and
// is a top level views widget. Tooltips, menus, and other kinds of transient
// windows that can't activate are filtered out.
// windows that can't activate are filtered out. The transient child is set
// up after window Init so add it here but remove it later.
if (window->type() != aura::client::WINDOW_TYPE_NORMAL || !window->delegate())
return;
views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
if (!widget || !widget->is_top_level())
return;
if (wm::GetTransientParent(window) != nullptr)
return;
if (!widget->CanActivate())
return;

Expand All @@ -155,6 +154,17 @@ void CrostiniAppWindowShelfController::OnWindowVisibilityChanging(
if (!visible)
return;

// Transient windows are set up after window Init, so remove them here.
if (wm::GetTransientParent(window)) {
DCHECK(aura_window_to_app_window_.find(window) ==
aura_window_to_app_window_.end());
auto it = observed_windows_.find(window);
DCHECK(it != observed_windows_.end());
observed_windows_.erase(it);
window->RemoveObserver(this);
return;
}

// Skip when this window has been handled. This can happen when the window
// becomes visible again.
auto app_window_it = aura_window_to_app_window_.find(window);
Expand Down

0 comments on commit bf10ece

Please sign in to comment.