New issue
Advanced search Search tips

Issue 607755 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

mash: Refactor calls to RootWindowController::ForWindow()

Project Member Reported by jamescook@chromium.org, Apr 28 2016

Issue description

Several places in the ash code assume that they can call RootWindowController::ForWindow(random_window) and get the ash::RootWindowController for the display. In mash, this often returns null, because random_window may be the child of a widget that has its own RootWindow without a RootWindowController.

This was the cause of a crash when opening the overflow bubble,  issue 595851 

There are a couple approaches here:
1. Rewrite RootWindowController::ForWindow(random_window) so it returns the master RootWindowController for the display for random_window
2. Eliminate the method and refactor the call sites to get what they need via another route

I'm partial to trying (2), since it reduces the confusion about mash having ash::RootWindowControllers for arbitrary windows. WDYT?

 
Labels: -mustash mash
Project Member

Comment 2 by bugdroid1@chromium.org, May 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d74ac0d16386f72e1426107d78aba51c221c230

commit 5d74ac0d16386f72e1426107d78aba51c221c230
Author: jamescook <jamescook@chromium.org>
Date: Thu May 12 19:57:12 2016

mus: Fix ash::GetRootWindowController for child windows/widgets

Under mus there is one RootWindow per top-level widget, similar to desktop
aura. This means we can't just use the aura::Window's root to find the
RootWindowController -- sometimes it will be null. Instead we need to find
the display's root window's RootWindowController.

This has been a cause of crashes under mus. I looked into refactoring the
callers of GetRootWindowController(), but there are ~100 places in the code
that call into it, either directly or indirectly, and it seemed better to
fix it here.

BUG= 607755 
TEST=RootWindowControolerTest.GetRootWindowController, existing ash_unittests

Review-Url: https://codereview.chromium.org/1975433002
Cr-Commit-Position: refs/heads/master@{#393337}

[modify] https://crrev.com/5d74ac0d16386f72e1426107d78aba51c221c230/ash/root_window_controller.cc
[modify] https://crrev.com/5d74ac0d16386f72e1426107d78aba51c221c230/ash/root_window_controller.h
[modify] https://crrev.com/5d74ac0d16386f72e1426107d78aba51c221c230/ash/root_window_controller_unittest.cc

Status: Fixed (was: Assigned)
Components: -MUS Internals>Services>WindowService

Sign in to add a comment