New issue
Advanced search Search tips

Issue 850184 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

mash: Remove ash::Shell access from BrowserStatusMonitor

Project Member Reported by jamescook@chromium.org, Jun 6 2018

Issue description

BrowserStatusMonitor tracks window activation by installing an observer on ash::Shell's global activation client. It uses window activation to update the shelf status of v1 (non-packaged) apps, especially those running in windows.

I think some of this code can be removed. In particular, ChromeLauncherController has:

  // Used to update the state of non plaform apps, as web contents change.
  enum AppState {
    APP_STATE_ACTIVE,
    APP_STATE_WINDOW_ACTIVE,
    APP_STATE_INACTIVE,
    APP_STATE_REMOVED
  };

I think the active vs. inactive states date back to when we drew the dot under a shelf item as bright vs. dim based on the window activation state. Today we just draw the dot with the same brightness all the time, so I think we can just track if the app is running or not.

If that's not enough to kill the shell dependency we might be able to track BrowserList activation instead of aura::Window activation.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 7 2018

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

commit 2712126a201347fa0560aa11fbbc8acc57f31033
Author: James Cook <jamescook@chromium.org>
Date: Thu Jun 07 00:27:14 2018

chromeos: Clean up BrowserStatusMonitor for ash shelf

For out-of-process ash code in //chrome/browser can't call into ash.
BrowserStatusMonitor was installing a global window activation observer
on ash::Shell. However, it doesn't need to do that anymore.

The code was written to change the brightness of the "running" dot we
show under the shelf icon for a v1 (non-packaged) app. However, with
the MD redesign of the shelf we always use the same dot so we don't
need to track activation state any more.

See removal of ash::STATUS_ACTIVE from enum ShelfItemStatus in
https://chromium-review.googlesource.com/c/chromium/src/+/758800

Bug:  850184 ,  557406 
Test: browser_tests --gtest_filter=*Launcher*
Change-Id: I38ed5e563d8b52386c53fafd17174608cd4ef8d6
Reviewed-on: https://chromium-review.googlesource.com/1089914
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565119}
[modify] https://crrev.com/2712126a201347fa0560aa11fbbc8acc57f31033/chrome/browser/ui/ash/launcher/DEPS
[modify] https://crrev.com/2712126a201347fa0560aa11fbbc8acc57f31033/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
[modify] https://crrev.com/2712126a201347fa0560aa11fbbc8acc57f31033/chrome/browser/ui/ash/launcher/browser_status_monitor.h
[modify] https://crrev.com/2712126a201347fa0560aa11fbbc8acc57f31033/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/2712126a201347fa0560aa11fbbc8acc57f31033/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
[modify] https://crrev.com/2712126a201347fa0560aa11fbbc8acc57f31033/chrome/browser/ui/ash/launcher/multi_profile_browser_status_monitor.cc

Status: Fixed (was: Started)

Sign in to add a comment