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

Issue 635963 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ash: Clean up shelf IsHorizontalAlignment usage

Project Member Reported by jamescook@chromium.org, Aug 9 2016

Issue description

Callers should use WmShelf::IsHorizontalAlignment() instead of using the bare function.

Bonus points for eliminating places in the system tray code that cache their own copy of the alignment.

 
Status: Started (was: Assigned)
https://codereview.chromium.org/2676673005/ fixes most calls.

The status area is still caching its own copy of shelf alignment. I haven't looked into that yet. It might be a remnant from when the old Shelf object didn't exist before ShelfView creation. Ideally the status area would use WmShelf::GetAlignment().

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 3 2017

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

commit 9eb0f91135b98152c4dfa1b9779287ac2e5ac362
Author: jamescook <jamescook@chromium.org>
Date: Fri Feb 03 19:13:39 2017

ash: Clean up shelf GetAlignment() and IsHorizontalAlignment() calls

This resolves some TODOs left over from shelf refactoring.

Eliminate various GetAlignment() utility functions and instead call into
WmShelf::GetAlignment() directory.

Prefer WmShelf::IsHorizontalAlignment() to the bare utility fuction.

Fix IWYU violations.

BUG= 635963 
TEST=ash_unittests
TBR=dmazzoni@chromium.org for IWYU in c/b/chromeos/accessibility

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

[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/app_list/app_list_presenter_delegate.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/overflow_bubble.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/overflow_bubble_view.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/overflow_bubble_view.h
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/shelf_button.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/shelf_layout_manager.h
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/shelf_widget.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/shelf/shelf_widget.h
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/common/wm/panels/panel_layout_manager.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/9eb0f91135b98152c4dfa1b9779287ac2e5ac362/chrome/browser/chromeos/accessibility/accessibility_manager.cc

Status: Assigned (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 20 2017

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

commit e6704a0c2b29585fab489399ba0396104cff7d85
Author: mohsen <mohsen@chromium.org>
Date: Thu Apr 20 06:30:20 2017

Make LogoutButtonTray a regular View

Currently, LogoutButtonTray is a TrayBackgroundView, however, it does
not use most of its functionality (e.g., drawing and animating
background, handling bubbles, drawing separators and focus rings). This
CL makes LogoutButtonTray directly inherit from View and just adds the
padding using TrayContainer class.

This CL also removes shelf alignment caching in TrayBackgroundView,
TrayContainer, and StatusAreaWidgetDelegate and cleaned up some related
shelf alignment code.

BUG= 698134 , 635963 
TEST=StatusAreaWidgetTest.Basic in ash_unittests

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

[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/BUILD.gn
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/shelf/shelf_layout_manager_unittest.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/date/tray_system_info.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/date/tray_system_info.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/network/tray_vpn.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/network/tray_vpn.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/overview/overview_button_tray.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/overview/overview_button_tray.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/palette/palette_tray.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/session/logout_button_tray.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/session/logout_button_tray.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/status_area_widget.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/status_area_widget.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/status_area_widget_delegate.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/status_area_widget_delegate.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/system_tray.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/system_tray.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/system_tray_item.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/system_tray_item.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/tray_background_view.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/tray_background_view.h
[add] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/tray_container.cc
[add] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/tray_container.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/tray/tray_item_view.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/user/tray_user.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/user/tray_user.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/virtual_keyboard/virtual_keyboard_tray.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/virtual_keyboard/virtual_keyboard_tray.h
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/web_notification/web_notification_tray.cc
[modify] https://crrev.com/e6704a0c2b29585fab489399ba0396104cff7d85/ash/system/web_notification/web_notification_tray.h

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 26 2017

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

commit 7924a4308a1956e0c4b065c483de4da44cb6c383
Author: jamescook <jamescook@chromium.org>
Date: Wed Apr 26 00:52:05 2017

chromeos: Remove wm_shelf_util.h

Eliminate the one remaining caller of ash::IsHorizontalAlignment().
Everything uses WmShelf::IsHorizontalAlignment() now.

Just cleanup, no behavior changes.

BUG= 635963 
TEST=ash_unittests, manually test panels and shelf alignment

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

[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/BUILD.gn
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/shelf/shelf_widget.cc
[delete] https://crrev.com/0ce0ee1aa6d9d2db1a7cdc7f000cfe451378d6f2/ash/shelf/wm_shelf_util.cc
[delete] https://crrev.com/0ce0ee1aa6d9d2db1a7cdc7f000cfe451378d6f2/ash/shelf/wm_shelf_util.h
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/system/network/tray_network.cc
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/system/tray/system_tray.cc
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/system/tray/tray_image_item.cc
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/system/web_notification/web_notification_tray.cc
[modify] https://crrev.com/7924a4308a1956e0c4b065c483de4da44cb6c383/ash/wm/panels/panel_layout_manager.cc

Status: Fixed (was: Started)
Labels: code-change
Status: Verified (was: Fixed)

Sign in to add a comment