Skip to content

Commit

Permalink
Replace shelf_constants.h with a new ShelfConfig class.
Browse files Browse the repository at this point in the history
- Remove the shelf_constants.h file and replace it with a new
  ShelfConfig class used to access the current shelf values.

- Replace the usage of constants from shelf_constants.h with the use
  of ShelfConfig::Get(), encapsulating the constants.

- No visible UI changes can be seen with this change.

- ShelfConfig is owned and instantiated by the shell, so that in the
  future ShelfConfig can add and remove itself as an observer to
  things such as tabler_mode_controller.

- This change is in preparation for dense shelf. In the future,
  ShelfConfig will be used for transitioning between dense shelf and
  the standard (current) shelf during runtime. Classes that use
  changed ShelfConfig values will observe ShelfConfig and be notified
  by ShelfConfig to update their layout and use the new values.

Bug: 973483
Change-Id: I7c14ed3248b32e3a337b91e0822003894e7ba37e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1775390
Commit-Queue: Matthew Mourgos <mmourgos@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694923}
  • Loading branch information
Matthew Mourgos authored and Commit Bot committed Sep 9, 2019
1 parent 3288827 commit 7532756
Show file tree
Hide file tree
Showing 78 changed files with 755 additions and 551 deletions.
3 changes: 2 additions & 1 deletion ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ component("ash") {
"public/cpp/multi_user_window_manager_delegate.h",
"public/cpp/multi_user_window_manager_observer.h",
"public/cpp/overview_test_api.h",
"public/cpp/shelf_config.h",
"public/cpp/split_view_test_api.h",
"public/cpp/window_finder.h",
"public/cpp/window_tree_host_lookup.h",
"root_window_controller.h",
"screenshot_delegate.h",
"session/session_controller_impl.h",
"shelf/shelf.h",
"shelf/shelf_constants.h",
"shelf/shelf_widget.h",
"shell.h",
"shell_delegate.h",
Expand Down Expand Up @@ -587,6 +587,7 @@ component("ash") {
"shelf/shelf_button_delegate.h",
"shelf/shelf_button_pressed_metric_tracker.cc",
"shelf/shelf_button_pressed_metric_tracker.h",
"shelf/shelf_config.cc",
"shelf/shelf_container_view.cc",
"shelf/shelf_container_view.h",
"shelf/shelf_context_menu_model.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

#include <memory>

#include "ash/shelf/shelf_constants.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "ui/display/display.h"
Expand Down Expand Up @@ -130,7 +130,7 @@ TEST_F(AccessibilityPanelLayoutManagerTest, DisplayBoundsChange) {

gfx::Rect expected_work_area = screen->GetPrimaryDisplay().bounds();
expected_work_area.Inset(0, kDefaultPanelHeight, 0,
ShelfConstants::shelf_size());
ShelfConfig::Get()->shelf_size());
EXPECT_EQ(screen->GetPrimaryDisplay().work_area(), expected_work_area);
}

Expand Down
8 changes: 4 additions & 4 deletions ash/dip_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#include <algorithm>
#include <vector>

#include "ash/public/cpp/shelf_config.h"
#include "ash/shelf/shelf.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/shelf/shelf_widget.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
Expand All @@ -33,13 +33,13 @@ TEST_F(DIPTest, WorkArea) {
aura::Window* root = Shell::GetPrimaryRootWindow();
const display::Display display =
display::Screen::GetScreen()->GetDisplayNearestWindow(root);
const int shelf_inset = 900 - ShelfConstants::shelf_size();
const int shelf_inset = 900 - ShelfConfig::Get()->shelf_size();

EXPECT_EQ("0,0 1000x900", display.bounds().ToString());
gfx::Rect work_area = display.work_area();
EXPECT_EQ(gfx::Rect(0, 0, 1000, shelf_inset).ToString(),
work_area.ToString());
EXPECT_EQ(gfx::Insets(0, 0, ShelfConstants::shelf_size(), 0).ToString(),
EXPECT_EQ(gfx::Insets(0, 0, ShelfConfig::Get()->shelf_size(), 0).ToString(),
display.bounds().InsetsFrom(work_area).ToString());

UpdateDisplay("2000x1800*2.0f");
Expand All @@ -57,7 +57,7 @@ TEST_F(DIPTest, WorkArea) {
work_area = display_2x.work_area();
EXPECT_EQ(gfx::Rect(0, 0, 1000, shelf_inset).ToString(),
work_area.ToString());
EXPECT_EQ(gfx::Insets(0, 0, ShelfConstants::shelf_size(), 0).ToString(),
EXPECT_EQ(gfx::Insets(0, 0, ShelfConfig::Get()->shelf_size(), 0).ToString(),
display_2x.bounds().InsetsFrom(work_area).ToString());

// Sanity check if the workarea's inset hight is same as
Expand Down
8 changes: 4 additions & 4 deletions ash/display/display_move_window_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
#include "ash/accelerators/accelerator_controller_impl.h"
#include "ash/accelerators/accelerator_table.h"
#include "ash/accessibility/test_accessibility_controller_client.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/root_window_controller.h"
#include "ash/screen_util.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "ash/wm/mru_window_tracker.h"
Expand Down Expand Up @@ -244,7 +244,7 @@ TEST_F(DisplayMoveWindowUtilTest, KeepWindowBoundsIfNotChangedByUser) {
// | |
// +---+
UpdateDisplay("400x300,400x600");
const int shelf_inset = 300 - ShelfConstants::shelf_size();
const int shelf_inset = 300 - ShelfConfig::Get()->shelf_size();
// Create and activate window on display [1].
aura::Window* window =
CreateTestWindowInShellWithBounds(gfx::Rect(410, 20, 200, 400));
Expand Down Expand Up @@ -436,11 +436,11 @@ TEST_F(DisplayMoveWindowUtilTest, RestoreMaximizedWindowAfterMovement) {

WindowState* window_state = WindowState::Get(w);
window_state->Maximize();
EXPECT_EQ(gfx::Rect(0, 0, 400, 300 - ShelfConstants::shelf_size()),
EXPECT_EQ(gfx::Rect(0, 0, 400, 300 - ShelfConfig::Get()->shelf_size()),
w->GetBoundsInScreen());

PerformMoveWindowAccel();
EXPECT_EQ(gfx::Rect(400, 0, 400, 300 - ShelfConstants::shelf_size()),
EXPECT_EQ(gfx::Rect(400, 0, 400, 300 - ShelfConfig::Get()->shelf_size()),
w->GetBoundsInScreen());
window_state->Restore();
EXPECT_EQ(gfx::Rect(410, 20, 200, 100), w->GetBoundsInScreen());
Expand Down
6 changes: 3 additions & 3 deletions ash/display/window_tree_host_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
#include <memory>

#include "ash/display/display_util.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/screen_util.h"
#include "ash/shelf/shelf.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/shelf/shelf_widget.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
Expand Down Expand Up @@ -711,8 +711,8 @@ TEST_F(WindowTreeHostManagerTest, SwapPrimaryById) {
Shell::Get()->window_tree_host_manager();

UpdateDisplay("200x200,300x300");
const int shelf_inset_first = 200 - ShelfConstants::shelf_size();
const int shelf_inset_second = 300 - ShelfConstants::shelf_size();
const int shelf_inset_first = 200 - ShelfConfig::Get()->shelf_size();
const int shelf_inset_second = 300 - ShelfConfig::Get()->shelf_size();
display::Display primary_display =
display::Screen::GetScreen()->GetPrimaryDisplay();
display::Display secondary_display = display_manager()->GetSecondaryDisplay();
Expand Down
4 changes: 2 additions & 2 deletions ash/login/ui/login_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include "ash/login/ui/login_button.h"

#include "ash/shelf/shelf_constants.h"
#include "ash/public/cpp/shelf_config.h"
#include "ui/views/animation/flood_fill_ink_drop_ripple.h"
#include "ui/views/animation/ink_drop_highlight.h"
#include "ui/views/animation/ink_drop_impl.h"
Expand All @@ -28,7 +28,7 @@ LoginButton::LoginButton(views::ButtonListener* listener)
SetImageHorizontalAlignment(views::ImageButton::ALIGN_CENTER);
SetImageVerticalAlignment(views::ImageButton::ALIGN_MIDDLE);
SetInstallFocusRingOnFocus(true);
focus_ring()->SetColor(kShelfFocusBorderColor);
focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
SetInkDropMode(InkDropMode::ON);
set_has_ink_drop_action_on_click(true);
}
Expand Down
4 changes: 2 additions & 2 deletions ash/login/ui/login_pin_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
#include "ash/login/ui/login_button.h"
#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/login_constants.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/strings/grit/ash_strings.h"
#include "base/bind.h"
#include "base/callback.h"
Expand Down Expand Up @@ -108,7 +108,7 @@ class BasePinButton : public views::InkDropHostView {
SetInkDropMode(InkDropMode::ON_NO_GESTURE_HANDLER);

focus_ring_ = views::FocusRing::Install(this);
focus_ring_->SetColor(kShelfFocusBorderColor);
focus_ring_->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
}

~BasePinButton() override = default;
Expand Down
4 changes: 2 additions & 2 deletions ash/login/ui/login_user_menu_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "ash/login/ui/login_user_menu_view.h"
#include "ash/login/ui/non_accessible_view.h"
#include "ash/login/ui/views_utils.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/strings/grit/ash_strings.h"
#include "base/strings/utf_string_conversions.h"
#include "ui/accessibility/ax_node_data.h"
Expand Down Expand Up @@ -65,7 +65,7 @@ class RemoveUserButton : public views::Button {
gfx::Insets(kUserMenuMarginAroundRemoveUserButtonDp,
kUserMenuMarginAroundRemoveUserButtonDp)));
SetInstallFocusRingOnFocus(true);
focus_ring()->SetColor(kShelfFocusBorderColor);
focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
}

~RemoveUserButton() override = default;
Expand Down
8 changes: 4 additions & 4 deletions ash/login/ui/note_action_launch_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

#include <memory>

#include "ash/public/cpp/shelf_config.h"
#include "ash/public/mojom/tray_action.mojom.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/strings/grit/ash_strings.h"
#include "base/i18n/rtl.h"
#include "ui/base/l10n/l10n_util.h"
Expand Down Expand Up @@ -187,9 +187,9 @@ class NoteActionLaunchButton::ActionButton : public views::ImageButton,
event_targeter_delegate_(kLargeBubbleRadiusDp, kSmallBubbleRadiusDp) {
SetAccessibleName(
l10n_util::GetStringUTF16(IDS_ASH_STYLUS_TOOLS_CREATE_NOTE_ACTION));
SetImage(
views::Button::STATE_NORMAL,
CreateVectorIcon(kTrayActionNewLockScreenNoteIcon, kShelfIconColor));
SetImage(views::Button::STATE_NORMAL,
CreateVectorIcon(kTrayActionNewLockScreenNoteIcon,
ShelfConfig::Get()->shelf_icon_color()));
SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
SetFocusPainter(nullptr);
EnableCanvasFlippingForRTLUI(true);
Expand Down
4 changes: 2 additions & 2 deletions ash/login/ui/parent_access_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
#include "ash/login/ui/login_pin_view.h"
#include "ash/login/ui/non_accessible_view.h"
#include "ash/public/cpp/login_types.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/wallpaper/wallpaper_controller_impl.h"
Expand Down Expand Up @@ -231,7 +231,7 @@ class ParentAccessView::FocusableLabelButton : public views::LabelButton {
const base::string16& text)
: views::LabelButton(listener, text) {
SetInstallFocusRingOnFocus(true);
focus_ring()->SetColor(kShelfFocusBorderColor);
focus_ring()->SetColor(ShelfConfig::Get()->shelf_focus_border_color());
}
~FocusableLabelButton() override = default;

Expand Down
12 changes: 6 additions & 6 deletions ash/magnifier/docked_magnifier_controller_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
#include "ash/magnifier/magnifier_test_utils.h"
#include "ash/public/cpp/ash_pref_names.h"
#include "ash/public/cpp/ash_switches.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/session/session_controller_impl.h"
#include "ash/session/test_session_controller_client.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "ash/test/ash_test_helper.h"
Expand Down Expand Up @@ -241,7 +241,7 @@ TEST_P(DockedMagnifierTest, DisplaysWorkAreas) {
const gfx::Rect disp_1_bounds(0, 0, 800, 600);
EXPECT_EQ(disp_1_bounds, display_1.bounds());
gfx::Rect disp_1_workarea_no_magnifier = disp_1_bounds;
disp_1_workarea_no_magnifier.Inset(0, 0, 0, ShelfConstants::shelf_size());
disp_1_workarea_no_magnifier.Inset(0, 0, 0, ShelfConfig::Get()->shelf_size());
EXPECT_EQ(disp_1_workarea_no_magnifier, display_1.work_area());
// At this point, normal mouse cursor confinement should be used.
AshWindowTreeHost* host1 =
Expand All @@ -255,7 +255,7 @@ TEST_P(DockedMagnifierTest, DisplaysWorkAreas) {
const gfx::Rect disp_2_bounds(800, 0, 400, 300);
EXPECT_EQ(disp_2_bounds, display_2.bounds());
gfx::Rect disp_2_workarea_no_magnifier = disp_2_bounds;
disp_2_workarea_no_magnifier.Inset(0, 0, 0, ShelfConstants::shelf_size());
disp_2_workarea_no_magnifier.Inset(0, 0, 0, ShelfConfig::Get()->shelf_size());
EXPECT_EQ(disp_2_workarea_no_magnifier, display_2.work_area());
AshWindowTreeHost* host2 =
Shell::Get()
Expand Down Expand Up @@ -356,7 +356,7 @@ TEST_P(DockedMagnifierTest, DisplaysWorkAreasOverviewMode) {
const display::Display& display = display_manager()->GetDisplayAt(0);
gfx::Rect workarea = display.bounds();
const int magnifier_height = GetMagnifierHeight(display.bounds().height());
workarea.Inset(0, magnifier_height, 0, ShelfConstants::shelf_size());
workarea.Inset(0, magnifier_height, 0, ShelfConfig::Get()->shelf_size());
EXPECT_EQ(workarea, display.work_area());
EXPECT_EQ(workarea, window->bounds());
EXPECT_TRUE(WindowState::Get(window.get())->IsMaximized());
Expand Down Expand Up @@ -398,7 +398,7 @@ TEST_P(DockedMagnifierTest, DisplaysWorkAreasSingleSplitView) {
const display::Display& display = display_manager()->GetDisplayAt(0);
const int magnifier_height = GetMagnifierHeight(display.bounds().height());
gfx::Rect work_area = display.bounds();
work_area.Inset(0, magnifier_height, 0, ShelfConstants::shelf_size());
work_area.Inset(0, magnifier_height, 0, ShelfConfig::Get()->shelf_size());
EXPECT_EQ(work_area, display.work_area());
EXPECT_EQ(work_area, window->bounds());
EXPECT_TRUE(WindowState::Get(window.get())->IsMaximized());
Expand Down Expand Up @@ -440,7 +440,7 @@ TEST_P(DockedMagnifierTest, DisplaysWorkAreasDoubleSplitView) {
const display::Display& display = display_manager()->GetDisplayAt(0);
const int magnifier_height = GetMagnifierHeight(display.bounds().height());
gfx::Rect work_area = display.bounds();
work_area.Inset(0, magnifier_height, 0, ShelfConstants::shelf_size());
work_area.Inset(0, magnifier_height, 0, ShelfConfig::Get()->shelf_size());
EXPECT_EQ(work_area, display.work_area());
EXPECT_EQ(work_area.height(), window1->bounds().height());
EXPECT_EQ(work_area.height(), window2->bounds().height());
Expand Down
Loading

0 comments on commit 7532756

Please sign in to comment.