Mash browser frame: fix incognito text color (used in popup windows)
Remove incognito member variable from CustomFrameHeader and make the
text color part of the AppearanceProvider (in Mash, this relies on
window properties).
Also move some functionality from CustomFrameViewAsh to HeaderView so
that when BrowserNonClientFrameViewAsh uses HeaderView, it will benefit
from it as well.
Bug: 837705
Change-Id: I7cb4aa654b37a6a9f8f73864c745f15245a15fc3
Reviewed-on: https://chromium-review.googlesource.com/1117883
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575662}diff --git a/ash/frame/custom_frame_header.cc b/ash/frame/custom_frame_header.cc
index 9acdf28..feea8e5 100644
--- a/ash/frame/custom_frame_header.cc
+++ b/ash/frame/custom_frame_header.cc
@@ -26,10 +26,6 @@
namespace {
-// Color for the window title text.
-const SkColor kNormalWindowTitleTextColor = SkColorSetRGB(40, 40, 40);
-const SkColor kIncognitoWindowTitleTextColor = SK_ColorWHITE;
-
// Creates a path with rounded top corners.
SkPath MakeRoundRectPath(const gfx::Rect& bounds,
int top_left_corner_radius,
@@ -125,13 +121,11 @@
views::Widget* target_widget,
views::View* view,
AppearanceProvider* appearance_provider,
- bool incognito,
FrameCaptionButtonContainerView* caption_button_container)
: FrameHeader(target_widget, view) {
DCHECK(appearance_provider);
DCHECK(caption_button_container);
appearance_provider_ = appearance_provider;
- is_incognito_ = incognito;
SetCaptionButtonContainer(caption_button_container);
}
@@ -155,8 +149,7 @@
}
SkColor CustomFrameHeader::GetTitleColor() const {
- return is_incognito_ ? kIncognitoWindowTitleTextColor
- : kNormalWindowTitleTextColor;
+ return appearance_provider_->GetTitleColor();
}
SkColor CustomFrameHeader::GetCurrentFrameColor() const {
diff --git a/ash/frame/custom_frame_header.h b/ash/frame/custom_frame_header.h
index df5ab01..c54bb45 100644
--- a/ash/frame/custom_frame_header.h
+++ b/ash/frame/custom_frame_header.h
@@ -20,6 +20,7 @@
class AppearanceProvider {
public:
virtual ~AppearanceProvider() = default;
+ virtual SkColor GetTitleColor() = 0;
virtual SkColor GetFrameHeaderColor(bool active) = 0;
virtual gfx::ImageSkia GetFrameHeaderImage(bool active) = 0;
virtual gfx::ImageSkia GetFrameHeaderOverlayImage(bool active) = 0;
@@ -33,7 +34,6 @@
CustomFrameHeader(views::Widget* target_widget,
views::View* view,
AppearanceProvider* appearance_provider,
- bool incognito,
FrameCaptionButtonContainerView* caption_button_container);
~CustomFrameHeader() override;
@@ -53,9 +53,6 @@
AppearanceProvider* appearance_provider_ = nullptr;
- // Whether the header is for an incognito browser window.
- bool is_incognito_ = false;
-
DISALLOW_COPY_AND_ASSIGN(CustomFrameHeader);
};
diff --git a/ash/frame/custom_frame_view_ash.cc b/ash/frame/custom_frame_view_ash.cc
index 0a49a50..cd8e34aa 100644
--- a/ash/frame/custom_frame_view_ash.cc
+++ b/ash/frame/custom_frame_view_ash.cc
@@ -284,7 +284,6 @@
// overlay the web contents in immersive fullscreen.
frame->non_client_view()->SetOverlayView(overlay_view_);
frame_window->SetProperty(aura::client::kTopViewColor, kDefaultFrameColor);
- frame_window->AddObserver(this);
// A delegate for a more complex way of fullscreening the window may already
// be set. This is the case for packaged apps.
@@ -302,10 +301,6 @@
Shell::Get()->RemoveShellObserver(this);
if (Shell::Get()->split_view_controller())
Shell::Get()->split_view_controller()->RemoveObserver(this);
- if (frame_ && frame_->GetNativeWindow() &&
- frame_->GetNativeWindow()->HasObserver(this)) {
- frame_->GetNativeWindow()->RemoveObserver(this);
- }
}
void CustomFrameViewAsh::InitImmersiveFullscreenControllerForView(
@@ -464,21 +459,6 @@
InvalidateLayout();
}
-void CustomFrameViewAsh::OnWindowDestroying(aura::Window* window) {
- DCHECK_EQ(frame_->GetNativeWindow(), window);
- window->RemoveObserver(this);
-}
-
-void CustomFrameViewAsh::OnWindowPropertyChanged(aura::Window* window,
- const void* key,
- intptr_t old) {
- DCHECK_EQ(frame_->GetNativeWindow(), window);
- if (key == aura::client::kShowStateKey) {
- header_view_->OnShowStateChanged(
- window->GetProperty(aura::client::kShowStateKey));
- }
-}
-
const views::View* CustomFrameViewAsh::GetAvatarIconViewForTest() const {
return header_view_->avatar_icon();
}
diff --git a/ash/frame/custom_frame_view_ash.h b/ash/frame/custom_frame_view_ash.h
index 4886e8e..a593db51a 100644
--- a/ash/frame/custom_frame_view_ash.h
+++ b/ash/frame/custom_frame_view_ash.h
@@ -16,7 +16,6 @@
#include "base/macros.h"
#include "base/optional.h"
#include "third_party/skia/include/core/SkColor.h"
-#include "ui/aura/window_observer.h"
#include "ui/views/window/non_client_view.h"
namespace views {
@@ -38,8 +37,7 @@
// BrowserNonClientFrameViewAsh.
class ASH_EXPORT CustomFrameViewAsh : public views::NonClientFrameView,
public ShellObserver,
- public SplitViewController::Observer,
- public aura::WindowObserver {
+ public SplitViewController::Observer {
public:
// Internal class name.
static const char kViewClassName[];
@@ -104,12 +102,6 @@
void SchedulePaintInRect(const gfx::Rect& r) override;
void SetVisible(bool visible) override;
- // aura::WindowObserver:
- void OnWindowDestroying(aura::Window* window) override;
- void OnWindowPropertyChanged(aura::Window* window,
- const void* key,
- intptr_t old) override;
-
// If |paint| is false, we should not paint the header. Used for overview mode
// with OnOverviewModeStarting() and OnOverviewModeEnded() to hide/show the
// header of v2 and ARC apps.
diff --git a/ash/frame/header_view.cc b/ash/frame/header_view.cc
index 064bba4c..4689ebe 100644
--- a/ash/frame/header_view.cc
+++ b/ash/frame/header_view.cc
@@ -36,6 +36,10 @@
: window_(window) {}
~WindowPropertyAppearanceProvider() override = default;
+ SkColor GetTitleColor() override {
+ return window_->GetProperty(kFrameTextColorKey);
+ }
+
SkColor GetFrameHeaderColor(bool active) override {
return window_->GetProperty(active ? kFrameActiveColorKey
: kFrameInactiveColorKey);
@@ -127,9 +131,8 @@
DCHECK(!::features::IsAshInBrowserProcess());
appearance_provider_ = std::make_unique<WindowPropertyAppearanceProvider>(
target_widget_->GetNativeWindow());
- // TODO(estade): pass correct value for |incognito|.
auto frame_header = std::make_unique<CustomFrameHeader>(
- target_widget, this, appearance_provider_.get(), false,
+ target_widget, this, appearance_provider_.get(),
caption_button_container_);
frame_header_ = std::move(frame_header);
}
@@ -213,10 +216,6 @@
: views::PaintInfo::ScaleType::kScaleWithEdgeSnapping);
}
-void HeaderView::OnShowStateChanged(ui::WindowShowState show_state) {
- frame_header_->OnShowStateChanged(show_state);
-}
-
///////////////////////////////////////////////////////////////////////////////
// HeaderView, views::View overrides:
@@ -267,6 +266,9 @@
window->GetProperty(kFrameInactiveColorKey));
} else if (key == kFrameBackButtonStateKey) {
UpdateCaptionButtons();
+ } else if (key == aura::client::kShowStateKey) {
+ frame_header_->OnShowStateChanged(
+ window->GetProperty(aura::client::kShowStateKey));
}
}
diff --git a/ash/frame/header_view.h b/ash/frame/header_view.h
index e1914c0..3fb0ca8 100644
--- a/ash/frame/header_view.h
+++ b/ash/frame/header_view.h
@@ -81,9 +81,6 @@
void SetWidthInPixels(int width_in_pixels);
- // Called when the target widget show state changed.
- void OnShowStateChanged(ui::WindowShowState show_state);
-
// views::View:
void Layout() override;
void ChildPreferredSizeChanged(views::View* child) override;
diff --git a/ash/public/cpp/mus_property_mirror_ash.cc b/ash/public/cpp/mus_property_mirror_ash.cc
index f630ffb..b37cff70 100644
--- a/ash/public/cpp/mus_property_mirror_ash.cc
+++ b/ash/public/cpp/mus_property_mirror_ash.cc
@@ -90,6 +90,9 @@
} else if (key == kFrameIsThemedByHostedAppKey) {
root_window->SetProperty(kFrameIsThemedByHostedAppKey,
window->GetProperty(kFrameIsThemedByHostedAppKey));
+ } else if (key == kFrameTextColorKey) {
+ root_window->SetProperty(kFrameTextColorKey,
+ window->GetProperty(kFrameTextColorKey));
}
}
diff --git a/ash/public/cpp/window_properties.cc b/ash/public/cpp/window_properties.cc
index 8b73a12..9e09914 100644
--- a/ash/public/cpp/window_properties.cc
+++ b/ash/public/cpp/window_properties.cc
@@ -16,6 +16,7 @@
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/mus/property_converter.h"
#include "ui/aura/window.h"
+#include "ui/gfx/color_palette.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/image/image_skia.h"
#include "ui/wm/core/shadow_types.h"
@@ -63,6 +64,9 @@
kFrameIsThemedByHostedAppKey, mojom::kFrameIsThemedByHostedApp_Property,
aura::PropertyConverter::CreateAcceptAnyValueCallback());
property_converter->RegisterPrimitiveProperty(
+ kFrameTextColorKey, mojom::kFrameTextColor_Property,
+ aura::PropertyConverter::CreateAcceptAnyValueCallback());
+ property_converter->RegisterPrimitiveProperty(
kHideShelfWhenFullscreenKey, mojom::kHideShelfWhenFullscreen_Property,
aura::PropertyConverter::CreateAcceptAnyValueCallback());
property_converter->RegisterPrimitiveProperty(
@@ -154,6 +158,9 @@
kFrameInactiveColorKey,
kDefaultFrameColor);
DEFINE_UI_CLASS_PROPERTY_KEY(bool, kFrameIsThemedByHostedAppKey, false);
+DEFINE_UI_CLASS_PROPERTY_KEY(SkColor,
+ kFrameTextColorKey,
+ gfx::kPlaceholderColor);
DEFINE_UI_CLASS_PROPERTY_KEY(mojom::WindowPinType,
kWindowPinTypeKey,
mojom::WindowPinType::NONE);
diff --git a/ash/public/cpp/window_properties.h b/ash/public/cpp/window_properties.h
index 4ab5200c..45d882913 100644
--- a/ash/public/cpp/window_properties.h
+++ b/ash/public/cpp/window_properties.h
@@ -164,6 +164,10 @@
ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
kFrameIsThemedByHostedAppKey;
+// A property that controls the color of text rendered on a browser frame.
+ASH_PUBLIC_EXPORT extern const aura::WindowProperty<SkColor>* const
+ kFrameTextColorKey;
+
// A property key to store ash::WindowPinType for a window.
// When setting this property to PINNED or TRUSTED_PINNED, the window manager
// will try to fullscreen the window and pin it on the top of the screen. If the
diff --git a/ash/public/interfaces/window_properties.mojom b/ash/public/interfaces/window_properties.mojom
index fdc8524..05e6383f 100644
--- a/ash/public/interfaces/window_properties.mojom
+++ b/ash/public/interfaces/window_properties.mojom
@@ -31,6 +31,10 @@
const string kFrameIsThemedByHostedApp_Property =
"ash:frame-is-themed-by-hosted-app";
+// The color of the text drawn on the frame (i.e. the window title). Only used
+// for tabless browser windows.
+const string kFrameTextColor_Property = "ash:frame-text-color";
+
// True if the shelf should be hidden when this window is put into fullscreen.
// Exposed because some windows want to explicitly opt-out of this.
const string kHideShelfWhenFullscreen_Property =
diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
index 0ae0b376..76198c2 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
@@ -72,6 +72,10 @@
// the Settings window.
constexpr SkColor kMdWebUiFrameColor = SkColorSetARGB(0xff, 0x25, 0x4f, 0xae);
+// Color for the window title text.
+constexpr SkColor kNormalWindowTitleTextColor = SkColorSetRGB(40, 40, 40);
+constexpr SkColor kIncognitoWindowTitleTextColor = SK_ColorWHITE;
+
bool IsMash() {
return !features::IsAshInBrowserProcess();
}
@@ -218,6 +222,8 @@
// TODO(estade): how much of the rest of this needs porting to Mash?
if (IsMash()) {
+ frame()->GetNativeWindow()->SetProperty(ash::kFrameTextColorKey,
+ GetTitleColor());
OnThemeChanged();
return;
}
@@ -689,6 +695,12 @@
///////////////////////////////////////////////////////////////////////////////
// ash::CustomFrameHeader::AppearanceProvider:
+SkColor BrowserNonClientFrameViewAsh::GetTitleColor() {
+ return browser_view()->IsRegularOrGuestSession()
+ ? kNormalWindowTitleTextColor
+ : kIncognitoWindowTitleTextColor;
+}
+
SkColor BrowserNonClientFrameViewAsh::GetFrameHeaderColor(bool active) {
DCHECK(!IsMash());
return GetFrameColor(active);
@@ -827,8 +839,10 @@
///////////////////////////////////////////////////////////////////////////////
// aura::WindowObserver:
+// TODO(estade): remove this interface. Ash handles it for us with HeaderView.
void BrowserNonClientFrameViewAsh::OnWindowDestroying(aura::Window* window) {
+ DCHECK(!IsMash());
DCHECK_EQ(frame()->GetNativeWindow(), window);
window->RemoveObserver(this);
}
@@ -836,6 +850,7 @@
void BrowserNonClientFrameViewAsh::OnWindowPropertyChanged(aura::Window* window,
const void* key,
intptr_t old) {
+ DCHECK(!IsMash());
DCHECK_EQ(frame()->GetNativeWindow(), window);
if (key != aura::client::kShowStateKey)
return;
@@ -974,8 +989,7 @@
Browser* browser = browser_view()->browser();
if (!UsePackagedAppHeaderStyle(browser)) {
header = std::make_unique<ash::CustomFrameHeader>(
- frame(), this, this, !browser_view()->IsRegularOrGuestSession(),
- caption_button_container_);
+ frame(), this, this, caption_button_container_);
} else {
auto default_frame_header = std::make_unique<ash::DefaultFrameHeader>(
frame(), this, caption_button_container_);
diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
index bca5d45..eaef4c1 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
@@ -92,6 +92,7 @@
void ChildPreferredSizeChanged(views::View* child) override;
// ash::CustomFrameHeader::AppearanceProvider:
+ SkColor GetTitleColor() override;
SkColor GetFrameHeaderColor(bool active) override;
gfx::ImageSkia GetFrameHeaderImage(bool active) override;
gfx::ImageSkia GetFrameHeaderOverlayImage(bool active) override;
diff --git a/components/exo/shell_surface_base.cc b/components/exo/shell_surface_base.cc
index a3fa89d..8073d3c8 100644
--- a/components/exo/shell_surface_base.cc
+++ b/components/exo/shell_surface_base.cc
@@ -34,6 +34,7 @@
#include "ui/aura/client/cursor_client.h"
#include "ui/aura/window.h"
#include "ui/aura/window_event_dispatcher.h"
+#include "ui/aura/window_observer.h"
#include "ui/aura/window_targeter.h"
#include "ui/aura/window_tree_host.h"
#include "ui/base/accelerators/accelerator.h"
@@ -95,7 +96,8 @@
DISALLOW_COPY_AND_ASSIGN(ShellSurfaceWidget);
};
-class CustomFrameView : public ash::CustomFrameViewAsh {
+class CustomFrameView : public ash::CustomFrameViewAsh,
+ public aura::WindowObserver {
public:
using ShapeRects = std::vector<gfx::Rect>;
