New issue
Advanced search Search tips

Issue 629257 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 615155



Sign in to add a comment

mash shelf: Convert ShelfWindowWatcher to wm common types

Project Member Reported by jamescook@chromium.org, Jul 18 2016

Issue description

ShelfWindowWatcher
* Heavy aura::Window use, but might be straightforward to convert to WmWindow

ShelfWindowWatcherItemDelegate
* Simple convert to WmWindow

 
Cc: -jamescook@chromium.org
Owner: jamescook@chromium.org
Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 26 2016

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

commit 04f84072e56506d52835408c0862a80865bde7b9
Author: jamescook <jamescook@chromium.org>
Date: Tue Jul 26 21:25:12 2016

Remove ash::ScopedObserverWithDuplicatedSources

It was added in http://crrev.com/21185004 to allow observation of multiple
ActivationClients in ash. However, ash only has a single ActivationClient, so
just observe that directly.

This eliminates ash::WindowTreeHostManager access in ShelfWindowWatcher, which
will simplify its conversion to wm types. It also simplifies
BrowserStatusMonitor, which no longer needs to observe root windows or displays
just to add activation client observers.

BUG= 629257 
TEST=ash_unittests, chrome unit_tests, manually activate windows and tabs in
ash and verify shelf light-bar appearance changes correctly

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

[modify] https://crrev.com/04f84072e56506d52835408c0862a80865bde7b9/ash/ash.gyp
[delete] https://crrev.com/d7003fd439ac29488126ea7bdd4731f92dcd06ee/ash/shelf/scoped_observer_with_duplicated_sources.h
[delete] https://crrev.com/d7003fd439ac29488126ea7bdd4731f92dcd06ee/ash/shelf/scoped_observer_with_duplicated_sources_unittest.cc
[modify] https://crrev.com/04f84072e56506d52835408c0862a80865bde7b9/ash/shelf/shelf_window_watcher.cc
[modify] https://crrev.com/04f84072e56506d52835408c0862a80865bde7b9/ash/shelf/shelf_window_watcher.h
[modify] https://crrev.com/04f84072e56506d52835408c0862a80865bde7b9/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
[modify] https://crrev.com/04f84072e56506d52835408c0862a80865bde7b9/chrome/browser/ui/ash/launcher/browser_status_monitor.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 27 2016

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

commit 47d35c03fb047de36bf7a26dc1beb80d572f7696
Author: jamescook <jamescook@chromium.org>
Date: Wed Jul 27 21:07:48 2016

mash: Convert ShelfWindowWatcher to wm common types

This also simplifies the behavior of ShelfWindowWatcher:
* It adds an icon to the shelf if a window with ShelfItemDetails is created in
the default window container.
* It leaves the icon there until the window is closed, even if the window is
reparented during a drag or dock operation.

This eliminates one set of window observers.

TODO: Change ownership to WmShell and move to //ash/common

BUG= 629257 
TEST=added to ash_unittests

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

[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/aura/wm_window_aura.cc
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/aura/wm_window_aura.h
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/common/wm_window.h
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/common/wm_window_property.h
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/mus/bridge/wm_window_mus.cc
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/mus/bridge/wm_window_mus.h
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/shelf/shelf_window_watcher.cc
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/shelf/shelf_window_watcher.h
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/shelf/shelf_window_watcher_item_delegate.cc
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/shelf/shelf_window_watcher_item_delegate.h
[modify] https://crrev.com/47d35c03fb047de36bf7a26dc1beb80d572f7696/ash/shelf/shelf_window_watcher_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 4 2016

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

commit 757a8f0c8a59d4dd16ad8821a19fa2f1609c0105
Author: msw <msw@chromium.org>
Date: Wed Aug 03 23:59:42 2016

mash: Move ShelfWindowWatcher[ItemDelegate] to ash/common.

James already converted these to ash common types in:
  https://codereview.chromium.org/2192553002

Convert the tests to common types and move all to ash/common.
Add WmWindow::[Set|Clear]ShelfItemDetails (Get* already added).

Move ShelfWindowWatcher ownership to WmShell.

BUG= 615155 , 629257 
TEST=Automated; no cros task manager (etc.) shelf item changes.
R=sky@chromium.org

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

[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/ash.gyp
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/aura/wm_window_aura.cc
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/aura/wm_window_aura.h
[rename] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/shelf/shelf_window_watcher.cc
[rename] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/shelf/shelf_window_watcher.h
[rename] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/shelf/shelf_window_watcher_item_delegate.cc
[rename] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/shelf/shelf_window_watcher_item_delegate.h
[add] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/shelf/shelf_window_watcher_unittest.cc
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/wm_shell.cc
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/wm_shell.h
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/common/wm_window.h
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/mus/bridge/wm_window_mus.cc
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/mus/bridge/wm_window_mus.h
[delete] https://crrev.com/96b9de712a092e59ccc38336c6f3fb3c14630399/ash/shelf/shelf_window_watcher_unittest.cc
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/shell.cc
[modify] https://crrev.com/757a8f0c8a59d4dd16ad8821a19fa2f1609c0105/ash/shell.h

Comment 5 by msw@chromium.org, Aug 4 2016

Status: Fixed (was: Started)
Labels: VerifyIn-54

Comment 7 by dchan@chromium.org, Oct 7 2016

Labels: VerifyIn-55

Comment 8 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56

Comment 9 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 10 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 11 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 12 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 14 by dchan@chromium.org, Oct 14 2017

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

Sign in to add a comment