Skip to content

Commit

Permalink
Factor some constants out of ash_constants.h
Browse files Browse the repository at this point in the history
Some constants in ash_constants.h are used in
BrowserNonClientFrameViewAsh. In order to make this
class available to lacros, these constants need to be
factored out, once lacros chrome can not link to ash symbols.

The following constants are moved:

 ash::kDefaultFrameColor
 ash::kResizeAreaCornerSize
 ash::kResizeInsideBoundsSize
 ash::kResizeOutsideBoundsSize
 ash::kResizeOutsideBoundsScaleForTouch

now under the `chromeos` namespace.

Also, CL adds a README.md to chromeos/ui directory.

This is phase 2.2 on the design document [1].

[1] https://docs.google.com/document/d/1xHwcHrAiEaWuA4usGEqKZ9zm1H8SGk3WkS-Jf2Q_los/

BUG=1113900
R=jamescook@chromium.org

Change-Id: I01cf20ed5a77f752fee2c72df0d5cb0cf42125a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429441
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Antonio Gomes (GMT-4) <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#811027}
  • Loading branch information
tonikitoo authored and Commit Bot committed Sep 27, 2020
1 parent fd31ad1 commit 1f43abc
Show file tree
Hide file tree
Showing 20 changed files with 96 additions and 45 deletions.
2 changes: 2 additions & 0 deletions ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1690,6 +1690,7 @@ component("ash") {
"//chromeos/components/phonehub",
"//chromeos/components/quick_answers",
"//chromeos/components/quick_answers/public/cpp:prefs",
"//chromeos/ui",
"//services/viz/public/mojom",

# TODO(https://crbug.com/644336): Make CrasAudioHandler Chrome or Ash only.
Expand Down Expand Up @@ -2285,6 +2286,7 @@ test("ash_unittests") {
"//build:branding_buildflags",
"//chromeos:test_support",
"//chromeos/strings:strings_grit",
"//chromeos/ui",
"//chromeos/ui/vector_icons",

# TODO(https://crbug.com/644336): Make CrasAudioHandler Chrome or Ash only.
Expand Down
1 change: 1 addition & 0 deletions ash/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ include_rules = [
"+chromeos/system",
# Do not eliminate this.
"+chromeos/ui/vector_icons",
"+chromeos/ui/chromeos_ui_constants.h",

# ui/base/idle depends on SessionManagerClient so disallow it.
"-ui/base/idle"
Expand Down
1 change: 1 addition & 0 deletions ash/public/cpp/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ component("cpp") {
"//chromeos/services/assistant/public/cpp",
"//chromeos/services/cellular_setup:in_process_esim_manager",
"//chromeos/services/network_config:in_process_instance",
"//chromeos/ui",
"//chromeos/ui/vector_icons",
"//components/prefs",
"//components/sync:rest_of_sync",
Expand Down
1 change: 1 addition & 0 deletions ash/public/cpp/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_rules = [
"+chromeos/services",
"+chromeos/ui/chromeos_ui_constants.h",
"+cc/metrics",
"+components/prefs",
"+services/data_decoder/public",
Expand Down
15 changes: 0 additions & 15 deletions ash/public/cpp/ash_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@ namespace ash {
// Radius of the header's top corners when the window is restored.
constexpr int kTopCornerRadiusWhenRestored = 2;

// In the window corners, the resize areas don't actually expand bigger, but the
// 16 px at the end of each edge triggers diagonal resizing.
constexpr int kResizeAreaCornerSize = 16;

// Ash windows do not have a traditional visible window frame. Window content
// extends to the edge of the window. We consider a small region outside the
// window bounds and an even smaller region overlapping the window to be the
// "non-client" area and use it for resizing.
constexpr int kResizeOutsideBoundsSize = 6;
constexpr int kResizeOutsideBoundsScaleForTouch = 5;
constexpr int kResizeInsideBoundsSize = 1;

// Background color used for the Chrome OS boot splash screen.
constexpr SkColor kChromeOsBootColor = SkColorSetRGB(0xfe, 0xfe, 0xfe);

Expand Down Expand Up @@ -66,9 +54,6 @@ constexpr FloatingMenuPosition kDefaultAutoclickMenuPosition =
constexpr FloatingMenuPosition kDefaultFloatingMenuPosition =
FloatingMenuPosition::kSystemDefault;

// The default frame color.
constexpr SkColor kDefaultFrameColor = SkColorSetRGB(0xFD, 0xFE, 0xFF);

// Whether keyboard auto repeat is enabled by default.
constexpr bool kDefaultKeyAutoRepeatEnabled = true;

Expand Down
6 changes: 3 additions & 3 deletions ash/public/cpp/default_frame_header.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@

#include <memory>

#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/ash_public_export.h"
#include "ash/public/cpp/frame_header.h"
#include "base/compiler_specific.h" // override
#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "chromeos/ui/chromeos_ui_constants.h"

namespace ash {

Expand Down Expand Up @@ -49,8 +49,8 @@ class ASH_PUBLIC_EXPORT DefaultFrameHeader : public FrameHeader {

SkColor GetActiveFrameColorForPaintForTest();

SkColor active_frame_color_ = kDefaultFrameColor;
SkColor inactive_frame_color_ = kDefaultFrameColor;
SkColor active_frame_color_ = chromeos::kDefaultFrameColor;
SkColor inactive_frame_color_ = chromeos::kDefaultFrameColor;

int width_in_pixels_ = -1;

Expand Down
13 changes: 7 additions & 6 deletions ash/public/cpp/frame_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#include "ash/public/cpp/frame_utils.h"

#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/window_properties.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "ui/aura/env.h"
#include "ui/aura/window.h"
#include "ui/base/hit_test.h"
Expand All @@ -22,10 +22,10 @@ using WindowOpacity = views::Widget::InitParams::WindowOpacity;
int FrameBorderNonClientHitTest(views::NonClientFrameView* view,
const gfx::Point& point_in_widget) {
gfx::Rect expanded_bounds = view->bounds();
int outside_bounds = kResizeOutsideBoundsSize;
int outside_bounds = chromeos::kResizeOutsideBoundsSize;

if (aura::Env::GetInstance()->is_touch_down())
outside_bounds *= kResizeOutsideBoundsScaleForTouch;
outside_bounds *= chromeos::kResizeOutsideBoundsScaleForTouch;
expanded_bounds.Inset(-outside_bounds, -outside_bounds);

if (!expanded_bounds.Contains(point_in_widget))
Expand All @@ -37,14 +37,15 @@ int FrameBorderNonClientHitTest(views::NonClientFrameView* view,
bool can_ever_resize = widget->widget_delegate()->CanResize();
// Don't allow overlapping resize handles when the window is maximized or
// fullscreen, as it can't be resized in those states.
int resize_border = kResizeInsideBoundsSize;
int resize_border = chromeos::kResizeInsideBoundsSize;
if (widget->IsMaximized() || widget->IsFullscreen()) {
resize_border = 0;
can_ever_resize = false;
}
int frame_component = view->GetHTComponentForFrame(
point_in_widget, resize_border, resize_border, kResizeAreaCornerSize,
kResizeAreaCornerSize, can_ever_resize);
point_in_widget, resize_border, resize_border,
chromeos::kResizeAreaCornerSize, chromeos::kResizeAreaCornerSize,
can_ever_resize);
if (frame_component != HTNOWHERE)
return frame_component;

Expand Down
8 changes: 5 additions & 3 deletions ash/public/cpp/window_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

#include "ash/public/cpp/window_properties.h"

#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/immersive/immersive_fullscreen_controller.h"
#include "ash/public/cpp/shelf_types.h"
#include "ash/public/cpp/window_backdrop.h"
#include "ash/public/cpp/window_pin_type.h"
#include "ash/public/cpp/window_state_type.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "third_party/skia/include/core/SkRegion.h"
#include "ui/aura/window.h"
#include "ui/wm/core/window_properties.h"
Expand Down Expand Up @@ -70,10 +70,12 @@ DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(SkRegion,
DEFINE_UI_CLASS_PROPERTY_KEY(aura::Window*,
kTabDraggingSourceWindowKey,
nullptr)
DEFINE_UI_CLASS_PROPERTY_KEY(SkColor, kFrameActiveColorKey, kDefaultFrameColor)
DEFINE_UI_CLASS_PROPERTY_KEY(SkColor,
kFrameActiveColorKey,
chromeos::kDefaultFrameColor)
DEFINE_UI_CLASS_PROPERTY_KEY(SkColor,
kFrameInactiveColorKey,
kDefaultFrameColor)
chromeos::kDefaultFrameColor)
DEFINE_UI_CLASS_PROPERTY_KEY(bool, kFrameRestoreLookKey, false)
DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(base::string16,
kWindowOverviewTitleKey,
Expand Down
5 changes: 4 additions & 1 deletion ash/wm/resize_shadow_and_cursor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
// found in the LICENSE file.

#include "ash/frame/non_client_frame_view_ash.h"
#include "ash/public/cpp/ash_constants.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "ash/wm/cursor_manager_test_api.h"
#include "ash/wm/resize_shadow.h"
#include "ash/wm/resize_shadow_controller.h"
#include "ash/wm/window_state.h"
#include "base/bind.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "ui/aura/window_event_dispatcher.h"
#include "ui/base/cursor/cursor.h"
#include "ui/base/cursor/mojom/cursor_type.mojom-shared.h"
Expand All @@ -19,6 +19,9 @@
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_delegate.h"

using chromeos::kResizeInsideBoundsSize;
using chromeos::kResizeOutsideBoundsSize;

namespace ash {

namespace {
Expand Down
12 changes: 6 additions & 6 deletions ash/wm/window_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <memory>

#include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/public/cpp/tablet_mode_observer.h"
Expand All @@ -27,6 +26,7 @@
#include "ash/wm/window_positioning_utils.h"
#include "ash/wm/window_state.h"
#include "ash/wm/wm_event.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/client/capture_client.h"
#include "ui/aura/client/focus_client.h"
Expand Down Expand Up @@ -58,7 +58,7 @@ namespace {
class InteriorResizeHandleTargeter : public aura::WindowTargeter {
public:
InteriorResizeHandleTargeter() {
SetInsets(gfx::Insets(kResizeInsideBoundsSize));
SetInsets(gfx::Insets(chromeos::kResizeInsideBoundsSize));
}

~InteriorResizeHandleTargeter() override = default;
Expand Down Expand Up @@ -188,11 +188,11 @@ int GetNonClientComponent(aura::Window* window, const gfx::Point& location) {
}

void SetChildrenUseExtendedHitRegionForWindow(aura::Window* window) {
gfx::Insets mouse_extend(-kResizeOutsideBoundsSize, -kResizeOutsideBoundsSize,
-kResizeOutsideBoundsSize,
-kResizeOutsideBoundsSize);
gfx::Insets mouse_extend(
-chromeos::kResizeOutsideBoundsSize, -chromeos::kResizeOutsideBoundsSize,
-chromeos::kResizeOutsideBoundsSize, -chromeos::kResizeOutsideBoundsSize);
gfx::Insets touch_extend =
mouse_extend.Scale(kResizeOutsideBoundsScaleForTouch);
mouse_extend.Scale(chromeos::kResizeOutsideBoundsScaleForTouch);
window->SetEventTargeter(std::make_unique<::wm::EasyResizeWindowTargeter>(
mouse_extend, touch_extend));
}
Expand Down
5 changes: 4 additions & 1 deletion ash/wm/workspace/multi_window_resize_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "ash/wm/workspace/multi_window_resize_controller.h"

#include "ash/frame/non_client_frame_view_ash.h"
#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/shelf_config.h"
#include "ash/public/cpp/test/shell_test_api.h"
#include "ash/shell.h"
Expand All @@ -21,6 +20,7 @@
#include "ash/wm/workspace_controller_test_api.h"
#include "base/memory/ptr_util.h"
#include "base/stl_util.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/test/test_window_delegate.h"
#include "ui/aura/window.h"
Expand All @@ -30,6 +30,9 @@
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_delegate.h"

using chromeos::kResizeInsideBoundsSize;
using chromeos::kResizeOutsideBoundsSize;

namespace ash {

namespace {
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2622,6 +2622,7 @@ static_library("ui") {
"//chromeos/settings",
"//chromeos/strings",
"//chromeos/system",
"//chromeos/ui",
"//chromeos/ui/vector_icons",
"//components/arc",
"//components/assist_ranker",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "apps/ui/views/app_window_frame_view.h"
#include "ash/frame/non_client_frame_view_ash.h"
#include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/ash_switches.h"
#include "ash/public/cpp/immersive/immersive_fullscreen_controller.h"
#include "ash/public/cpp/shelf_types.h"
Expand All @@ -32,6 +31,7 @@
#include "chrome/browser/ui/views/exclusive_access_bubble_views.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/extensions/extension_constants.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "components/session_manager/core/session_manager.h"
#include "extensions/browser/app_window/app_delegate.h"
#include "extensions/common/constants.h"
Expand Down Expand Up @@ -151,9 +151,9 @@ ChromeNativeAppWindowViewsAuraAsh::CreateNonStandardAppFrame() {

// For Aura windows on the Ash desktop the sizes are different and the user
// can resize the window from slightly outside the bounds as well.
frame->SetResizeSizes(ash::kResizeInsideBoundsSize,
ash::kResizeOutsideBoundsSize,
ash::kResizeAreaCornerSize);
frame->SetResizeSizes(chromeos::kResizeInsideBoundsSize,
chromeos::kResizeOutsideBoundsSize,
chromeos::kResizeAreaCornerSize);
return frame;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <algorithm>

#include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/caption_buttons/frame_caption_button_container_view.h"
#include "ash/public/cpp/default_frame_header.h"
#include "ash/public/cpp/frame_utils.h"
Expand Down Expand Up @@ -39,6 +38,7 @@
#include "chrome/browser/ui/web_applications/app_browser_controller.h"
#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "content/public/browser/web_contents.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/accessibility/ax_enums.mojom.h"
Expand Down Expand Up @@ -216,7 +216,7 @@ SkColor BrowserNonClientFrameViewAsh::GetCaptionColor(
bool active = ShouldPaintAsActive(active_state);

SkColor active_color =
views::FrameCaptionButton::GetButtonColor(ash::kDefaultFrameColor);
views::FrameCaptionButton::GetButtonColor(chromeos::kDefaultFrameColor);

// Web apps apply a theme color if specified by the extension.
Browser* browser = browser_view()->browser();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include <string>

#include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/ash_switches.h"
#include "ash/public/cpp/caption_buttons/frame_caption_button_container_view.h"
#include "ash/public/cpp/default_frame_header.h"
Expand Down Expand Up @@ -74,6 +73,7 @@
#include "chrome/common/web_application_info.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#include "components/account_id/account_id.h"
#include "components/autofill/core/common/password_form.h"
#include "components/keep_alive_registry/keep_alive_types.h"
Expand Down Expand Up @@ -287,7 +287,7 @@ IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTestNoWebUiTabStrip,
EXPECT_EQ(HTTOP, frame_view->NonClientHitTest(top_edge));

// Click just below the resize handle hits the caption.
gfx::Point below_resize(kWindowWidth / 2, ash::kResizeInsideBoundsSize);
gfx::Point below_resize(kWindowWidth / 2, chromeos::kResizeInsideBoundsSize);
EXPECT_EQ(HTCAPTION, frame_view->NonClientHitTest(below_resize));

// Click in the top edge of a maximized window now hits the client area,
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/views/location_bar/custom_tab_bar_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
#include "url/gurl.h"

#if defined(OS_CHROMEOS)
#include "ash/public/cpp/ash_constants.h"
#include "chromeos/ui/chromeos_ui_constants.h"
#else
#include "chrome/browser/themes/theme_properties.h"
#endif
Expand Down Expand Up @@ -460,7 +460,7 @@ bool CustomTabBarView::IsShowingOriginForTesting() const {
SkColor CustomTabBarView::GetDefaultFrameColor() const {
#if defined(OS_CHROMEOS)
// Ash system frames differ from ChromeOS browser frames.
return ash::kDefaultFrameColor;
return chromeos::kDefaultFrameColor;
#else
return ThemeProperties::GetDefaultColor(
ThemeProperties::COLOR_FRAME_ACTIVE, false,
Expand Down
15 changes: 15 additions & 0 deletions chromeos/ui/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

assert(is_chromeos || chromeos_is_browser_only,
"Non-Chrome-OS or Lacros builds must not depend on //chromeos")

# C++ headers and sources that can be used by both ash and lacros builds.
component("ui") {
sources = [ "chromeos_ui_constants.h" ]

output_name = "chromeos_ui"

deps = [ "//skia" ]
}
3 changes: 3 additions & 0 deletions chromeos/ui/DEPS
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include_rules = [
"+third_party/skia",
]
4 changes: 4 additions & 0 deletions chromeos/ui/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
This directory accommodates ChromeOS code that can be linked against both by
Ash and Lacros Chrome binaries. Some of the code residing here come originally,
but not exclusively, from //ash/public/cpp/. It includes classes, constants,
runtime switches, etc.
Loading

0 comments on commit 1f43abc

Please sign in to comment.