From 428150f17b7a70e315d42017f80d99d476072a24 Mon Sep 17 00:00:00 2001 From: Peter Kasting Date: Mon, 21 Mar 2022 19:53:27 +0000 Subject: [PATCH] Use Color Pipeline machinery for PWA ContentsWebView backgrounds. Bug: 1059923 Change-Id: I70c87783f6880cf4320863bcb7df6c1f4396d45a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3537669 Auto-Submit: Peter Kasting Reviewed-by: David Black Reviewed-by: Jeffrey Young Commit-Queue: Peter Kasting Cr-Commit-Position: refs/heads/main@{#983456} --- ...onalization_app_wallpaper_provider_impl.cc | 2 +- chrome/browser/ui/color/chrome_color_id.h | 5 ++- chrome/browser/ui/color/chrome_color_mixer.cc | 3 ++ .../browser_non_client_frame_view_chromeos.cc | 33 ---------------- .../browser_non_client_frame_view_chromeos.h | 3 -- .../ui/views/frame/contents_web_view.cc | 38 +++++++------------ .../ui/views/frame/contents_web_view.h | 7 ++-- .../app_browser_controller.cc | 2 + 8 files changed, 26 insertions(+), 67 deletions(-) diff --git a/chrome/browser/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.cc b/chrome/browser/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.cc index 3426855b4873ee..9f65b4a44f93b1 100644 --- a/chrome/browser/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.cc +++ b/chrome/browser/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.cc @@ -132,7 +132,7 @@ void PersonalizationAppWallpaperProviderImpl::MakeTransparent() { static_cast(BrowserView::GetBrowserViewForNativeWindow( web_contents->GetTopLevelNativeWindow()) ->contents_web_view()) - ->SetBackgroundColorOverride(SK_ColorTRANSPARENT); + ->SetBackgroundVisible(false); } void PersonalizationAppWallpaperProviderImpl::FetchCollections( diff --git a/chrome/browser/ui/color/chrome_color_id.h b/chrome/browser/ui/color/chrome_color_id.h index d0819796d6c122..a1e68fe45e5289 100644 --- a/chrome/browser/ui/color/chrome_color_id.h +++ b/chrome/browser/ui/color/chrome_color_id.h @@ -340,7 +340,10 @@ ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR_FRAME_ACTIVE) \ E(kColorToolbarTopSeparatorFrameInactive, \ ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR_FRAME_INACTIVE) \ - /* Window control button background colors */ \ + /* Web contents colors. */ \ + E_CPONLY(kColorWebContentsBackground) \ + E_CPONLY(kColorWebContentsBackgroundLetterboxing) \ + /* Window control button background colors. */ \ E(kColorWindowControlButtonBackgroundActive, \ ThemeProperties::COLOR_WINDOW_CONTROL_BUTTON_BACKGROUND_ACTIVE) \ E(kColorWindowControlButtonBackgroundInactive, \ diff --git a/chrome/browser/ui/color/chrome_color_mixer.cc b/chrome/browser/ui/color/chrome_color_mixer.cc index 35eb80a3217b1b..5823295445e709 100644 --- a/chrome/browser/ui/color/chrome_color_mixer.cc +++ b/chrome/browser/ui/color/chrome_color_mixer.cc @@ -456,6 +456,9 @@ void AddChromeColorMixer(ui::ColorProvider* provider, mixer[kColorToolbarTopSeparatorFrameInactive] = GetToolbarTopSeparatorColorTransform(kColorToolbar, ui::kColorFrameInactive); + mixer[kColorWebContentsBackground] = {kColorNewTabPageBackground}; + mixer[kColorWebContentsBackgroundLetterboxing] = + ui::AlphaBlend(kColorWebContentsBackground, SK_ColorBLACK, 0x33); mixer[kColorWindowControlButtonBackgroundActive] = {ui::kColorFrameActive}; mixer[kColorWindowControlButtonBackgroundInactive] = { ui::kColorFrameInactive}; diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.cc b/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.cc index ea3aec1fb9bbef..5355c293fc445e 100644 --- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.cc +++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.cc @@ -496,7 +496,6 @@ gfx::Size BrowserNonClientFrameViewChromeOS::GetMinimumSize() const { void BrowserNonClientFrameViewChromeOS::OnThemeChanged() { OnUpdateFrameColor(); - OnUpdateBackgroundColor(); caption_button_container_->OnWindowControlsOverlayEnabledChanged( browser_view()->IsWindowControlsOverlayEnabled(), GetFrameHeaderColor(browser_view()->IsActive())); @@ -936,38 +935,6 @@ bool BrowserNonClientFrameViewChromeOS::GetHideCaptionButtonsForFullscreen() return immersive_controller->ShouldHideTopViews(); } -void BrowserNonClientFrameViewChromeOS::OnUpdateBackgroundColor() { - if (!browser_view()) - return; - - // If `browser` is not associated with a system web app, background color will - // be resolved from web contents and does not need to be overridden. - auto* browser = browser_view()->browser(); - if (!browser || !web_app::IsSystemWebApp(browser)) - return; - - // If the system web app associated with the `browser` does not prefer the - // manifest background color, it will most likely be resolved from web - // contents and does not need to be overridden. Background color will fall - // back to the manifest background color in the event that web contents cannot - // resolve a background color. - auto* app_controller = browser->app_controller(); - if (!app_controller->system_app()->PreferManifestBackgroundColor()) - return; - - auto* contents_web_view = browser_view()->contents_web_view(); - if (!contents_web_view) - return; - - // When a system web app prefers the manifest background color over web - // contents background color, it should be immediately overridden on theme - // changes. This circumvents a lack of synchronization with the `browser` - // frame header transition that would otherwise occur if we waited for web - // contents' background changed event to propagate back to native UI. - contents_web_view->SetBackgroundColorOverride( - app_controller->GetBackgroundColor()); -} - void BrowserNonClientFrameViewChromeOS::OnUpdateFrameColor() { aura::Window* window = frame()->GetNativeWindow(); window->SetProperty(chromeos::kFrameActiveColorKey, diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.h b/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.h index 1e7d08d74f5804..8774edd0c76b81 100644 --- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.h +++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_chromeos.h @@ -225,9 +225,6 @@ class BrowserNonClientFrameViewChromeOS // fullscreen. bool GetHideCaptionButtonsForFullscreen() const; - // Called any time the background color may have changed. - void OnUpdateBackgroundColor(); - // Called any time the frame color may have changed. void OnUpdateFrameColor(); diff --git a/chrome/browser/ui/views/frame/contents_web_view.cc b/chrome/browser/ui/views/frame/contents_web_view.cc index 5f985efa03120d..bc047256f110af 100644 --- a/chrome/browser/ui/views/frame/contents_web_view.cc +++ b/chrome/browser/ui/views/frame/contents_web_view.cc @@ -5,6 +5,7 @@ #include "chrome/browser/ui/views/frame/contents_web_view.h" #include "chrome/browser/themes/theme_properties.h" +#include "chrome/browser/ui/color/chrome_color_id.h" #include "chrome/browser/ui/views/status_bubble_views.h" #include "content/public/browser/render_widget_host_view.h" #include "content/public/browser/web_contents.h" @@ -12,6 +13,7 @@ #include "third_party/skia/include/core/SkColor.h" #include "ui/base/metadata/metadata_impl_macros.h" #include "ui/base/theme_provider.h" +#include "ui/color/color_provider.h" #include "ui/compositor/layer.h" #include "ui/compositor/layer_tree_owner.h" #include "ui/views/background.h" @@ -41,9 +43,8 @@ StatusBubbleViews* ContentsWebView::GetStatusBubble() const { return status_bubble_; } -void ContentsWebView::SetBackgroundColorOverride( - absl::optional background_color) { - background_color_override_ = background_color; +void ContentsWebView::SetBackgroundVisible(bool background_visible) { + background_visible_ = background_visible; if (GetWidget()) UpdateBackgroundColor(); } @@ -68,32 +69,19 @@ void ContentsWebView::OnLetterboxingChanged() { } void ContentsWebView::UpdateBackgroundColor() { - // TODO(pkasting): In a Color Pipeline world, COLOR_NTP_BACKGROUND should get - // overridden by PWA windows in their mixer chain as necessary. Then the - // override here can go away, and the custom calculations for the letterboxing - // case can become a separate color (recipe) in the main chrome mixer. - SkColor ntp_background = background_color_override_.value_or( - GetThemeProvider()->GetColor(ThemeProperties::COLOR_NTP_BACKGROUND)); - if (is_letterboxing()) { - // Set the background color to a dark tint of the new tab page's background - // color. This is the color filled within the WebView's bounds when its - // child view is sized specially for fullscreen tab capture. See WebView - // header file comments for more details. - constexpr SkAlpha kBackgroundBrightness = 0x33; // 20% - // Make sure the background is opaque. - ntp_background = SkColorSetARGB( - SkColorGetA(ntp_background), - SkColorGetR(ntp_background) * kBackgroundBrightness / SK_AlphaOPAQUE, - SkColorGetG(ntp_background) * kBackgroundBrightness / SK_AlphaOPAQUE, - SkColorGetB(ntp_background) * kBackgroundBrightness / SK_AlphaOPAQUE); - } - SetBackground(views::CreateSolidBackground(ntp_background)); + SkColor color = GetColorProvider()->GetColor( + is_letterboxing() ? kColorWebContentsBackgroundLetterboxing + : kColorWebContentsBackground); + SetBackground(background_visible_ ? views::CreateSolidBackground(color) + : nullptr); if (web_contents()) { content::RenderWidgetHostView* rwhv = web_contents()->GetRenderWidgetHostView(); - if (rwhv) - rwhv->SetBackgroundColor(ntp_background); + if (rwhv) { + rwhv->SetBackgroundColor(background_visible_ ? color + : SK_ColorTRANSPARENT); + } } } diff --git a/chrome/browser/ui/views/frame/contents_web_view.h b/chrome/browser/ui/views/frame/contents_web_view.h index 4bc88b9ae97ec2..fcf20ea1511ed1 100644 --- a/chrome/browser/ui/views/frame/contents_web_view.h +++ b/chrome/browser/ui/views/frame/contents_web_view.h @@ -35,9 +35,8 @@ class ContentsWebView void SetStatusBubble(StatusBubbleViews* status_bubble); StatusBubbleViews* GetStatusBubble() const; - // Allow overriding the view background color. This is used to make a - // transparent background for SWAs. - void SetBackgroundColorOverride(absl::optional background_color); + // Toggles whether the background is visible. + void SetBackgroundVisible(bool background_visible); // WebView overrides: bool GetNeedsNotificationWhenVisibleBoundsChange() const override; @@ -57,7 +56,7 @@ class ContentsWebView void UpdateBackgroundColor(); StatusBubbleViews* status_bubble_; - absl::optional background_color_override_; + bool background_visible_ = true; std::unique_ptr cloned_layer_tree_; }; diff --git a/chrome/browser/ui/web_applications/app_browser_controller.cc b/chrome/browser/ui/web_applications/app_browser_controller.cc index bb13ab1d2e7349..97d1ed125f2b09 100644 --- a/chrome/browser/ui/web_applications/app_browser_controller.cc +++ b/chrome/browser/ui/web_applications/app_browser_controller.cc @@ -431,6 +431,8 @@ void AppBrowserController::AddColorMixers( #endif mixer[kColorPwaToolbarBackground] = {ui::kColorEndpointBackground}; mixer[kColorPwaToolbarForeground] = {ui::kColorEndpointForeground}; + if (bg_color) + mixer[kColorWebContentsBackground] = {kColorPwaBackground}; } void AppBrowserController::OnReceivedInitialURL() {