New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 615502 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

ash: Refactor access to shelf state

Project Member Reported by jamescook@chromium.org, May 27 2016

Issue description

As I work on converting the shelf to mustash's ash_sysui it's becoming obvious that the code is pretty messy. (See photo attached.) Part of this is history from harrym's various refactors that never quite got completed. I want to try to improve things a bit.

Part of the motivation here is to make it possible to do things with the shelf without looking it up via an aura::Window / root_window, since mustash will be using wm::WmWindow (to support both aura::Window and mus::Window).

Here are some things that I'd like to do:
1. Make it clear that "Shelf" is the controller for the shelf. All access into the shelf to set state (autohide, visibility, etc.) should go via "Shelf".
2. The primary (only?) way to get a Shelf is via RootWindowController.
3. Eliminate "helper" methods on Shell like SetShelfAlignment() that take an aura::Window.
4. Likewise eliminate "helper" methods on RootWindowController for shelf, replacing them with GetShelfController().
5. Change the ownership of ShelfWidget so it is owned by Shelf, not RootWindowController.
6. Accept the fact that the system tray widget will never be a child of the shelf. It's in its own container and too much of the system assumes it is there. It is also shown on the login/OOBE screens where the shelf is either not present or implemented as WebUI. Kill all the TODOs about making it a child.
7. Get access to the system tray via RootWindowController, not via Shelf/ShelfWidget.

I probably won't get to all of these, but I wanted to write them down somewhere.

Thoughts? Objections?

 
IMG_20160527_110149.jpg
685 KB View Download
Cc: tdander...@chromium.org varkha@chromium.org
+varkha, tdanderson as FYI, I'm doing some renaming/refactoring in the shelf code

https://codereview.chromium.org/2017413002/ and https://codereview.chromium.org/2020623004/ are out for review, which address #3 and fix a bunch of names

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 1 2016

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

commit 6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee
Author: jamescook <jamescook@chromium.org>
Date: Wed Jun 01 00:35:01 2016

ash: Move shelf alignment and auto-hide calls from Shell to Shelf

Shelf (aka ShelfController) should be the single point of access for alignment
and auto-hide state.

* Eliminate Shell::GetShelfAlignment and SetShelfAlignment
* Eliminate Shell::GetShelfAutoHideBehavior and SetShelfAutoHideBehavior
* Convert Shelf::GetAutoHideBehavior() to auto_hide_behavior()
* Use Shelf::ForPrimaryDisplay() in unit tests to avoid accessing the
  RootWindowController.

BUG= 615502 
TEST=existing ash_unittests and unit_tests

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

[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/content/keyboard_overlay/keyboard_overlay_delegate_unittest.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/root_window_controller.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/root_window_controller.h
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shelf/shelf.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shelf/shelf.h
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shelf/shelf_widget_unittest.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shell.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shell.h
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shell/context_menu.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/shell_unittest.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/system/web_notification/ash_popup_alignment_delegate_unittest.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/sysui/context_menu_mus.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/sysui/shelf_delegate_mus.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/wm/dock/docked_window_resizer_unittest.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/chrome/browser/chromeos/first_run/steps/tray_step.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/chrome/browser/chromeos/profiles/multiprofiles_session_aborted_dialog.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/chrome/browser/ui/ash/launcher/launcher_context_menu.cc
[modify] https://crrev.com/6afad6dbdc55b439182cadbd9cd08efd3f7ae6ee/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 1 2016

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

commit cf8b648fcece89a78eec21726a1aa48667778dbe
Author: jamescook <jamescook@chromium.org>
Date: Wed Jun 01 19:58:17 2016

ash: Fix variable names and setters in ShelfLayoutManager and tests

Calls to set shelf state should go through Shelf, not ShelfLayoutManager.
* Move all calls to SetAutoHideBehavior() and SetAlignment() to Shelf instead
of ShelfLayoutManager.
* Add Shelf::GetVisibilityState() and Shelf::GetAutoHideState() and prefer
using them to accessing ShelfLayoutManager directly.
* Remove ShelfLayoutManager::GetAutoHideBehavior().
* Rename variables mis-named "shelf" to shelf_widget, shelf_layout_manager, etc.
so they match their actual types.

No functional changes.

BUG= 615502 
TEST=ash_unittests

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

[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/root_window_controller_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/screen_util_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf.h
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/shelf/shelf_widget_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/system/toast/toast_manager_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/system/tray/system_tray_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/system/user/tray_user_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/wm/immersive_fullscreen_controller_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/wm/maximize_mode/maximize_mode_window_manager_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/wm/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/wm/workspace/workspace_window_resizer_unittest.cc
[modify] https://crrev.com/cf8b648fcece89a78eec21726a1aa48667778dbe/ash/wm/workspace_controller_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 17 2016

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

commit aada0db95278da7d4d74bae5d6dfb4480d5b3bee
Author: jamescook <jamescook@chromium.org>
Date: Fri Jun 17 22:10:02 2016

mash: Break ash system tray dependencies on ash::ShelfWidget

This is a step towards converting the system tray to wm common types, as well
as a step toward hiding the internal details of the shelf implementation from
external users. Long term, all access to shelf state should go via WmShelf.

* Plumb WmShelf into StatusAreaWidget.
* Remove ShelfWidget usage from //ash/system.
* Remove unnecessary includes.

BUG= 615502 , 619636 
TEST=ash_unittests

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

[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/aura/wm_shelf_aura.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/aura/wm_shelf_aura.h
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/focus_cycler_unittest.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/mus/bridge/wm_shelf_mus.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/mus/bridge/wm_shelf_mus.h
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/root_window_controller.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/shelf/shelf_widget.h
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/DEPS
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/chromeos/rotation/tray_rotation_lock_unittest.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/overview/overview_button_tray_unittest.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/status_area_widget.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/status_area_widget.h
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/tray/system_tray.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/tray/system_tray_unittest.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/tray/tray_background_view.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/tray/tray_background_view.h
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/tray/tray_details_view_unittest.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/web_notification/web_notification_tray.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/test/ash_test_base.cc
[modify] https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee/ash/test/ash_test_base.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 27 2016

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

commit 54a4d47fa4f8602431aa7b646fd7a77d34e96818
Author: jamescook <jamescook@chromium.org>
Date: Mon Jun 27 19:37:26 2016

mash: Convert all of //ash/system to use WmShelf

This breaks dependencies on //ash/shelf in favor of one on //ash/common.

* Add WmShelf::ForPrimaryDisplay().
* Remove AshTestBase::GetPrimaryShelf() in favor of above.
* Skip unnecessary setting of shelf visibility state and auto-hide state in tests.
* Remove unnecessary includes.

BUG= 615502 , 619636 
TEST=ash_unittests

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

[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/ash.gyp
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/aura/wm_shelf_aura.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/aura/wm_shelf_aura.h
[add] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/mus/bridge/wm_shelf_mus.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/mus/bridge/wm_shelf_mus.h
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/DEPS
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/cast/tray_cast.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/chromeos/network/tray_network.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/chromeos/screen_security/screen_tray_item.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/chromeos/virtual_keyboard/virtual_keyboard_tray.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/overview/overview_button_tray.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/toast/toast_manager_unittest.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/toast/toast_overlay.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/tray/system_tray_unittest.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/user/tray_user.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/user/tray_user_unittest.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/web_notification/ash_popup_alignment_delegate_unittest.cc
[modify] https://crrev.com/54a4d47fa4f8602431aa7b646fd7a77d34e96818/ash/system/web_notification/web_notification_tray_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 13 2016

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

commit 361185a2df3bfc21597d72dff241731e676885cd
Author: jamescook <jamescook@chromium.org>
Date: Sat Aug 13 00:12:09 2016

ash: Refactor Shelf ownership and ShelfView creation

This is a step toward replacing Shelf with WmShelf. See go/mash-shelf-refactor

* Move Shelf ownership to ash::RootWindowController
* Construct ShelfView in ShelfWidget
* Remove Shelf usage from ShelfWidget implementation
* Unfortunately, ShelfWidget::shelf() must remain for now as it is widely referenced. It will be easier to remove after alignment and autohide state move to WmShelf.
* Eliminate Shelf::SetVisible() helper

BUG= 615502 
TEST=existing ash_unittests and chrome unit_tests

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

[modify] https://crrev.com/361185a2df3bfc21597d72dff241731e676885cd/ash/root_window_controller.cc
[modify] https://crrev.com/361185a2df3bfc21597d72dff241731e676885cd/ash/root_window_controller.h
[modify] https://crrev.com/361185a2df3bfc21597d72dff241731e676885cd/ash/shelf/shelf.cc
[modify] https://crrev.com/361185a2df3bfc21597d72dff241731e676885cd/ash/shelf/shelf.h
[modify] https://crrev.com/361185a2df3bfc21597d72dff241731e676885cd/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/361185a2df3bfc21597d72dff241731e676885cd/ash/shelf/shelf_widget.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 22 2016

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

commit 37b9e6da90be32a6e163dbbdf6accb983203eff5
Author: jamescook <jamescook@chromium.org>
Date: Mon Aug 22 21:46:33 2016

ash: Remove unnecessary checks for null ShelfWidget in ShelfLayoutManager

The ShelfWidget exists for the lifetime of the ShelfLayoutManager. The
ShelfLayoutManager is created in the ShelfWidget's constructor, and exists
until the shelf container is destroyed at shutdown. Removing the checks makes
the lifetimes easier to understand.

BUG= 615502 
TEST=ash_unittests

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

[modify] https://crrev.com/37b9e6da90be32a6e163dbbdf6accb983203eff5/ash/shelf/shelf_layout_manager.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 22 2016

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

commit 44f141a103fe98f7c1be86ab8dfe80ce49204ca5
Author: jamescook <jamescook@chromium.org>
Date: Mon Aug 22 22:31:10 2016

ash: Remove unnecessary WmShelf::SchedulePaint() method

This method existed because the overflow bubble used it to cause a repaint of
the overflow button in the parent shelf view. Instead of repainting all the
buttons, just repaint the anchor directly.

(I am uncertain this code path ever gets called in production. It would only
run if some code directly destroyed the bubble widget instead of calling into
the shelf code to do so. This does not seem to happen at shutdown or when
disconnecting a display, even with the bubble visible. However, failure to
cleanup the cached pointers in OverflowBubble on widget destruction would
cause crashes, so I'm keeping the OnWidgetDestroying() method.)

BUG= 615502 
TEST=ash_browsertests, manual tests of opening and closing the overflow bubble
on multiple displays

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

[modify] https://crrev.com/44f141a103fe98f7c1be86ab8dfe80ce49204ca5/ash/common/shelf/overflow_bubble.cc
[modify] https://crrev.com/44f141a103fe98f7c1be86ab8dfe80ce49204ca5/ash/common/shelf/overflow_bubble.h
[modify] https://crrev.com/44f141a103fe98f7c1be86ab8dfe80ce49204ca5/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/44f141a103fe98f7c1be86ab8dfe80ce49204ca5/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/44f141a103fe98f7c1be86ab8dfe80ce49204ca5/ash/shelf/shelf.cc
[modify] https://crrev.com/44f141a103fe98f7c1be86ab8dfe80ce49204ca5/ash/shelf/shelf.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 23 2016

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 24 2016

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

commit a18fb174daa8e0c2770ce8f71807856de55f032c
Author: jamescook <jamescook@chromium.org>
Date: Wed Aug 24 05:29:42 2016

ash: Move alignment and autohide behavior from Shelf to WmShelf

This is a step toward replacing Shelf with WmShelf, which has a more sensible
lifetime. It does not change shelf behavior.

* Migrate the various getters and setters.
* Before the shelf is constructed (after login) default the alignment to
bottom-locked. After construction default to bottom. This is weird, but matches
what we had before.
* Add Shelf::wm_shelf() and use it in pieces of code that we know we'll have to
refactor anyway (chrome, some ash/wm pieces we're not converting).
* Move ShelfView::OnShelfAlignmentChanged() call into ShelfWidget. In general
I'm trying to reduce outside access into ShelfView.
* Convert Shelf* to WmShelf* where possible in tests.
* Make AshTestBase::GetPrimaryShelf() public so it can be used in test utility
functions outside of test classes.

BUG= 615502 
TEST=ash_unittests, chrome unit_tests
TBR=derat@chromium.org

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

[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/app_list/app_list_presenter_delegate.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/common/shelf/shelf.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/common/shelf/shelf.h
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/common/shelf/shelf_layout_manager.h
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/common/shelf/shelf_widget.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/content/keyboard_overlay/keyboard_overlay_delegate_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/shelf/shelf_widget_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/shell/context_menu.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/shell_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/sysui/context_menu_mus.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/test/ash_test_base.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/test/ash_test_base.h
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/dock/docked_window_resizer_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/immersive_fullscreen_controller_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/maximize_mode/maximize_mode_window_manager_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/window_animations.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/workspace/workspace_window_resizer_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/ash/wm/workspace_controller_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/chromeos/first_run/steps/tray_step.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/chromeos/login/supervised/supervised_user_creation_screen.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/chromeos/profiles/multiprofiles_session_aborted_dialog.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/ui/ash/launcher/launcher_context_menu.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.cc
[modify] https://crrev.com/a18fb174daa8e0c2770ce8f71807856de55f032c/chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 24 2016

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

commit 3ad0bed346b3bc79baa6bc79b20468930866a6cc
Author: jamescook <jamescook@chromium.org>
Date: Wed Aug 24 21:01:05 2016

ash: Convert ShelfDelegate to use WmShelf* instead of Shelf*

This is a step towards eliminating ash::Shelf.

Also remove ash::shell::ShelfDelegateImpl so we don't have to maintain
multiple stub implementations.

BUG= 615502 
TEST=ash_unittests

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

[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/ash.gyp
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/common/shelf/shelf.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/common/shelf/shelf_delegate.h
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/mus/bridge/wm_shelf_mus.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/root_window_controller.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/shelf/shelf_widget_unittest.cc
[delete] https://crrev.com/d63f07075f833e6dcce793b87a529569f3fe4827/ash/shell/shelf_delegate_impl.cc
[delete] https://crrev.com/d63f07075f833e6dcce793b87a529569f3fe4827/ash/shell/shelf_delegate_impl.h
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/test/test_shelf_delegate.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/ash/test/test_shelf_delegate.h
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
[modify] https://crrev.com/3ad0bed346b3bc79baa6bc79b20468930866a6cc/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 25 2016

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

commit 8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e
Author: jamescook <jamescook@chromium.org>
Date: Thu Aug 25 03:31:05 2016

ash: Move more code from Shelf to WmShelf

This is another step toward eliminating ash::Shelf.

* Move code that needs ShelfView into ShelfWidget to avoid exposing ShelfView
* Remove dead code (especially shelf_navigator.*)
* Make icon activation functions static

BUG= 615502 
TEST=ash_unittests, browser_tests

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

[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/accelerators/accelerator_controller_delegate_aura.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/ash.gyp
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/common/shelf/shelf.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/common/shelf/shelf.h
[delete] https://crrev.com/6c4ce51ea1886af904821bdd1dd0d6d3e6932d6f/ash/common/shelf/shelf_navigator.cc
[delete] https://crrev.com/6c4ce51ea1886af904821bdd1dd0d6d3e6932d6f/ash/common/shelf/shelf_navigator.h
[delete] https://crrev.com/6c4ce51ea1886af904821bdd1dd0d6d3e6932d6f/ash/common/shelf/shelf_navigator_unittest.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/common/shelf/shelf_widget.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/common/shelf/shelf_widget.h
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/common/wm/panels/panel_window_resizer.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/shelf/shelf_widget_unittest.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/ash/wm/window_animations.cc
[modify] https://crrev.com/8fd4ac5bbce4fbdc921a10ed70a7f6360de99c9e/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 30 2016

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

commit 577c95b3c9f75dbd71d60888d0f32f8a26d4e242
Author: jamescook <jamescook@chromium.org>
Date: Tue Aug 30 04:03:54 2016

ash: Move AppList support to ShelfWidget, expose ShelfWidget in WmShelf

This is another step toward eliminating ash::Shelf.

There are enough users of ShelfWidget from WmShelf that I decided to expose
ShelfWidget directly and remove GetShelfWidgetForTesting.

BUG= 615502 
TEST=ash_unittests

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

[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/app_list/app_list_presenter_delegate.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/common/shelf/shelf.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/common/shelf/shelf.h
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/common/shelf/shelf_widget.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/common/shelf/shelf_widget.h
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/first_run/first_run_helper_impl.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/shelf/shelf_widget_unittest.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/shell_unittest.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/wm/dock/docked_window_resizer_unittest.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/577c95b3c9f75dbd71d60888d0f32f8a26d4e242/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 31 2016

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

commit 1cad77e9db74d3826578d2f51b7474d86a765f40
Author: jamescook <jamescook@chromium.org>
Date: Wed Aug 31 00:02:26 2016

ash: Remove ash::Shelf in favor of ash::WmShelf

WmShelf exists for the lifetime of the RootWindowController, which makes it
easier to reason about. This also eliminates a wrapper layer in the shelf code.

* Move creation of ShelfView into WmShelf
* Move OnShelfCreated and OnShelfDestroyed notifications to WmShelf
* Rewrite ShelfWidgetTest.ShelfInitiallySizedAfterLogin to test both displays
* Fix IWYU for ShelfWidget
* Move FocusShelf and LaunchApp from AcceleratorControllerDelegateAura to the
common AcceleratorController, as they only use //ash/common

This does not yet change the lifetime or ownership of ShelfWidget, so there is
still some oddness around shelf "creation", which really means ShelfView
creation.

Pure refactor, no behavior changes.

BUG= 615502 
TEST=ash_unittests, unit_tests, manual tests of login with and without a
secondary display.

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

[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/accelerators/accelerator_controller_delegate_aura.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/ash.gyp
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/aura/wm_root_window_controller_aura.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/accelerators/accelerator_controller.cc
[delete] https://crrev.com/5b1e76e346a82ba506e0598f302838ebfdfc0680/ash/common/shelf/shelf.cc
[delete] https://crrev.com/5b1e76e346a82ba506e0598f302838ebfdfc0680/ash/common/shelf/shelf.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/shelf/shelf_button_pressed_metric_tracker_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/shelf/shelf_widget.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/shelf/shelf_widget.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/system/toast/toast_overlay.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/wm/dock/docked_window_layout_manager.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/wm/immersive_context_ash.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/wm/overview/window_grid.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/wm/overview/window_selector.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/wm/panels/panel_layout_manager.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/common/wm/workspace_controller.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/dip_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/display/root_window_transformers_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/display/window_tree_host_manager_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/first_run/first_run_helper_impl.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/focus_cycler_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/metrics/user_metrics_recorder.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/mus/accelerators/accelerator_controller_delegate_mus.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/mus/bridge/wm_shelf_mus.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/mus/bridge/wm_shelf_mus.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/mus/workspace/workspace_layout_manager_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/root_window_controller.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/root_window_controller.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/shelf/shelf_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/shelf/shelf_widget_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/shell.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/shell.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/shell/window_watcher.cc
[delete] https://crrev.com/5b1e76e346a82ba506e0598f302838ebfdfc0680/ash/test/shelf_test_api.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/wm/dock/docked_window_layout_manager_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/wm/overview/window_selector_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/wm/panels/attached_panel_window_targeter.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/wm/system_gesture_event_filter_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/ash/wm/window_cycle_controller_unittest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/chromeos/accessibility/chromevox_panel.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/chromeos/first_run/steps/tray_step.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/chromeos/login/supervised/supervised_user_creation_screen.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/launcher/chrome_launcher_controller_mus.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/launcher/chrome_launcher_controller_mus.h
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/ash/shelf_browsertest.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc
[modify] https://crrev.com/1cad77e9db74d3826578d2f51b7474d86a765f40/chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 1 2016

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

commit b551aba636f950844dad415bd73e08cdc815ebd6
Author: jamescook <jamescook@chromium.org>
Date: Thu Sep 01 01:00:16 2016

ash: Move ShelfWidget ownership to WmShelf and refactor access to it

This consolidates access to ShelfWidget in a single location.

* Move ShelfWidget ownership to WmShelf
* Remove RootWindowController::shelf_widget() and access it via WmShelf
cleanup
* Move HandleShowMessageCenterBubble to AcceleratorController because it no
longer depends on the aura-only ash::RootWindowController
* Eliminate WmShelf::SetShelfLayoutManager in favor of an explicit create,
shutdown, destroy cycle for ShelfWidget
* Move StatusAreaWidget creation slightly later, needed for above

There's still some ugliness here around ShelfWidget creation/destruction order
but it's better (or at least more explicit) than it was.

Pure refactor, no functional changes.

BUG= 615502 
TEST=ash_unittests, unit_tests, manual tests of login with and without a
secondary display.

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

[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/accelerators/accelerator_controller_delegate_aura.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/aura/wm_root_window_controller_aura.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/aura/wm_shelf_aura.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/aura/wm_shelf_aura.h
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/common/accelerators/accelerator_controller.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/common/shelf/shelf_widget.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/common/shelf/shelf_widget.h
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/mus/accelerators/accelerator_controller_delegate_mus.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/mus/bridge/wm_shelf_mus.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/mus/bridge/wm_shelf_mus.h
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/mus/root_window_controller.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/root_window_controller.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/root_window_controller.h
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/screen_util.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/shell.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/shell.h
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/shell/window_type_launcher.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/test/status_area_widget_test_helper.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/wm/overview/window_selector_unittest.cc
[modify] https://crrev.com/b551aba636f950844dad415bd73e08cdc815ebd6/ash/wm/workspace_controller_unittest.cc

Labels: Proj-Mustash-Mash
Status: Fixed (was: Started)
Calling this done for now.

* WmShelf is now the shelf controller. It has the same lifetime as WmRootWindowController, making it easy to add/remove observers at any time
* All shelf state (alignment, autohide) is owned by WmShelf, and almost all access is via WmShelf
* WmShelf owns ShelfWidget, and all access to ShelfWidget is via WmShelf
* ShelfView is no longer exposed outside ShelfWidget

And at long last the "Shelf" class in the middle of the diagram in comment 1 no longer exists.

Labels: VerifyIn-55

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

Labels: VerifyIn-56

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

Labels: VerifyIn-57

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

Labels: VerifyIn-58

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

Labels: VerifyIn-59

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

Labels: VerifyIn-60
Labels: VerifyIn-61

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

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

Sign in to add a comment