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 <[email protected]> Reviewed-by: David Black <[email protected]> Reviewed-by: Jeffrey Young <[email protected]> Commit-Queue: Peter Kasting <[email protected]> Cr-Commit-Position: refs/heads/main@{#983456}
scheib · Mar 21, 2022 · 428150f · 428150f
1 parent eb2f5b4
commit 428150f
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 67 deletions.
diff --git a/...r/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.cc b/...r/ash/web_applications/personalization_app/personalization_app_wallpaper_provider_impl.cc
@@ -132,7 +132,7 @@ void PersonalizationAppWallpaperProviderImpl::MakeTransparent() {
   static_cast<ContentsWebView*>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -5,13 +5,15 @@
 #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"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #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<SkColor> 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
@@ -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<SkColor> 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<SkColor> background_color_override_;
+  bool background_visible_ = true;
 
   std::unique_ptr<ui::LayerTreeOwner> 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
@@ -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() {