New issue
Advanced search Search tips

Issue 629990 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 615155



Sign in to add a comment

mash shelf: Move ShelfItemDelegateManager access to WmShell

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

Issue description

It's a source of ash::Shell::GetInstance() calls in the shelf

 
Blocking: 615155
Project Member

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

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

commit b5974f28ede18239045b4e3c17324e05b5a0c8fa
Author: jamescook <jamescook@chromium.org>
Date: Fri Jul 22 17:59:33 2016

mash: Fold ShelfItemDelegateManager into ShelfModel

This eliminates some accesses of ash::Shell inside the shelf code, which is
necessary for porting the shelf to mus/mustash.

* ShelfModel now owns the ShelfItemDelegates
* Explicitly delete the ShelfItemDelegates during shutdown, because some of
the subclasses reach into ShelfModel during their destructors
* Move AppListShelfItemDelegate earlier, to WmShell::Initialize phase
* Fix ShelfAppBrowserTest.LauncherContextMenuVerifyCloseItemAppearance to use
the actual shelf ID of the browser shortcut, rather than using the magic
number "1" that can sometimes mean the app list item.

This also simplifies some of the unit tests for ChromeLauncherController
because they don't need to construct their own ShelfItemDelegateManager.

BUG= 629990 , 429870 
TEST=ash_unittests, chrome unit_tests, manually test shelf pin/unpin/etc.

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

[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/ash.gyp
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/common/shelf/app_list_shelf_item_delegate.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/common/shelf/app_list_shelf_item_delegate.h
[delete] https://crrev.com/eed46f8917f47c85ac37c8f04267058905beda56/ash/common/shelf/shelf_item_delegate_manager.cc
[delete] https://crrev.com/eed46f8917f47c85ac37c8f04267058905beda56/ash/common/shelf/shelf_item_delegate_manager.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/common/shelf/shelf_model.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/common/shelf/shelf_model.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/common/shelf/shelf_model_observer.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/common/shelf/shelf_model_unittest.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/common/wm_shell.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf_unittest.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf_view.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf_view.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf_window_watcher.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shelf/shelf_window_watcher.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shell.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shell.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/shell/window_watcher.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/sysui/shelf_delegate_mus.cc
[delete] https://crrev.com/eed46f8917f47c85ac37c8f04267058905beda56/ash/test/shelf_item_delegate_manager_test_api.cc
[delete] https://crrev.com/eed46f8917f47c85ac37c8f04267058905beda56/ash/test/shelf_item_delegate_manager_test_api.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/ash/test/test_shelf_delegate.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/launcher_context_menu.cc
[modify] https://crrev.com/b5974f28ede18239045b4e3c17324e05b5a0c8fa/chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc

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

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

Labels: VerifyIn-55

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

Labels: VerifyIn-56

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

Labels: VerifyIn-57

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

Labels: VerifyIn-58

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

Labels: VerifyIn-59

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

Labels: VerifyIn-60
Labels: VerifyIn-61

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

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

Sign in to add a comment