Update terminal to use background color rather than theme

Terminal updated in crrev.com/c/2384669.

Bug: 1114542
Change-Id: I467dec67e76d77f956352e8ce2299bfde599f5cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2384689Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803773}

chrome/browser/ui/web_applications/app_browser_controller.cc

@@ -543,38 +543,24 @@ void AppBrowserController::UpdateThemePack() {
   base::Optional<SkColor> theme_color = GetThemeColor();
 
   AutogeneratedThemeColors colors;
-  // TODO(crbug.com/1114542): Make terminal use page background color to set
-  // active tab color and remove this branch.
-  if (system_app_type_ == SystemAppType::TERMINAL) {
-    if (!theme_color) {
-      theme_pack_ = nullptr;
-      return;
-    }
-
-    // For terminal app, active tab gets theme color.
-    colors.frame_color = GetAltColor(*theme_color);
-    colors.active_tab_color = *theme_color;
-    colors.ntp_color = *theme_color;
-  } else {
-    // TODO(crbug.com/1053823): Add tests for theme properties being set in this
-    // branch.
-    base::Optional<SkColor> background_color = GetBackgroundColor();
-    if (!theme_color && !background_color) {
-      theme_pack_ = nullptr;
-      return;
-    }
+  // TODO(crbug.com/1053823): Add tests for theme properties being set in this
+  // branch.
+  base::Optional<SkColor> background_color = GetBackgroundColor();
+  if (!theme_color && !background_color) {
+    theme_pack_ = nullptr;
+    return;
+  }
 
-    if (!theme_color)
-      theme_color = GetAltColor(*background_color);
-    else if (!background_color)
-      background_color = GetAltColor(*theme_color);
+  if (!theme_color)
+    theme_color = GetAltColor(*background_color);
+  else if (!background_color)
+    background_color = GetAltColor(*theme_color);
 
-    // For regular web apps, frame gets theme color and active tab gets
-    // background color.
-    colors.frame_color = *theme_color;
-    colors.active_tab_color = *background_color;
-    colors.ntp_color = *background_color;
-  }
+  // For regular web apps, frame gets theme color and active tab gets
+  // background color.
+  colors.frame_color = *theme_color;
+  colors.active_tab_color = *background_color;
+  colors.ntp_color = *background_color;
 
   colors.frame_text_color =
       color_utils::GetColorWithMaxContrast(colors.frame_color);

chrome/browser/ui/web_applications/app_browser_controller_browsertest.cc

@@ -41,10 +41,10 @@
 #include "ui/display/types/display_constants.h"
 
 namespace {
-SkColor GetTabColor(Browser* browser) {
+SkColor GetFrameColor(Browser* browser) {
   CustomThemeSupplier* theme = browser->app_controller()->GetThemeSupplier();
   SkColor result;
-  EXPECT_TRUE(theme->GetColor(ThemeProperties::COLOR_TOOLBAR, &result));
+  EXPECT_TRUE(theme->GetColor(ThemeProperties::COLOR_FRAME_ACTIVE, &result));
   return result;
 }
 }  // namespace
@@ -77,7 +77,7 @@ class LoadFinishedWaiter : public TabStripModelObserver,
 
   // content::WebContentsObserver:
   void DidFinishNavigation(content::NavigationHandle* handle) override {
-    color_at_navigation_ = GetTabColor(browser_);
+    color_at_navigation_ = GetFrameColor(browser_);
   }
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override {
@@ -211,8 +211,8 @@ IN_PROC_BROWSER_TEST_F(AppBrowserControllerBrowserTest, NonAppUrl) {
 IN_PROC_BROWSER_TEST_F(AppBrowserControllerBrowserTest, TabLoadNoThemeChange) {
   InstallAndLaunchMockApp();
   EXPECT_EQ(app_browser_->tab_strip_model()->count(), 1);
-  // First tab gets manifest theme immediately.
-  EXPECT_EQ(GetTabColor(app_browser_), SK_ColorGREEN);
+  // Frame gets manifest theme immediately.
+  EXPECT_EQ(GetFrameColor(app_browser_), SK_ColorGREEN);
 
   // Dynamically change color.
   content::WebContents* web_contents =
@@ -225,7 +225,7 @@ IN_PROC_BROWSER_TEST_F(AppBrowserControllerBrowserTest, TabLoadNoThemeChange) {
                               content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
                               /*world_id=*/1));
   theme_waiter.Wait();
-  EXPECT_EQ(GetTabColor(app_browser_), SK_ColorYELLOW);
+  EXPECT_EQ(GetFrameColor(app_browser_), SK_ColorYELLOW);
 
   // Second tab keeps dynamic theme until loaded.
   LoadFinishedWaiter load_waiter(app_browser_);
@@ -233,13 +233,13 @@ IN_PROC_BROWSER_TEST_F(AppBrowserControllerBrowserTest, TabLoadNoThemeChange) {
   load_waiter.Wait();
   EXPECT_EQ(app_browser_->tab_strip_model()->count(), 2);
   EXPECT_EQ(load_waiter.GetColorAtNavigation(), SK_ColorYELLOW);
-  EXPECT_EQ(GetTabColor(app_browser_), SK_ColorGREEN);
+  EXPECT_EQ(GetFrameColor(app_browser_), SK_ColorGREEN);
 
   // Switching tabs updates themes immediately.
   chrome::SelectNextTab(app_browser_);
-  EXPECT_EQ(GetTabColor(app_browser_), SK_ColorYELLOW);
+  EXPECT_EQ(GetFrameColor(app_browser_), SK_ColorYELLOW);
   chrome::SelectNextTab(app_browser_);
-  EXPECT_EQ(GetTabColor(app_browser_), SK_ColorGREEN);
+  EXPECT_EQ(GetFrameColor(app_browser_), SK_ColorGREEN);
 }
 
 // App Popups are only used on Chrome OS. See https://crbug.com/1060917.

chrome/browser/web_applications/web_app_install_finalizer.cc

@@ -433,14 +433,12 @@ void WebAppInstallFinalizer::SetWebAppManifestFieldsAndWriteData(
 
   web_app->SetDescription(base::UTF16ToUTF8(web_app_info.description));
   web_app->SetScope(web_app_info.scope);
-  if (web_app_info.theme_color) {
-    DCHECK_EQ(SkColorGetA(*web_app_info.theme_color), SK_AlphaOPAQUE);
-    web_app->SetThemeColor(web_app_info.theme_color);
-  }
-  if (web_app_info.background_color) {
-    DCHECK_EQ(SkColorGetA(*web_app_info.background_color), SK_AlphaOPAQUE);
-    web_app->SetBackgroundColor(*web_app_info.background_color);
-  }
+  DCHECK(!web_app_info.theme_color.has_value() ||
+         SkColorGetA(*web_app_info.theme_color) == SK_AlphaOPAQUE);
+  web_app->SetThemeColor(web_app_info.theme_color);
+  DCHECK(!web_app_info.background_color.has_value() ||
+         SkColorGetA(*web_app_info.background_color) == SK_AlphaOPAQUE);
+  web_app->SetBackgroundColor(web_app_info.background_color);
 
   WebApp::SyncFallbackData sync_fallback_data;
   sync_fallback_data.name = base::UTF16ToUTF8(web_app_info.title);
@@ -467,8 +465,8 @@ void WebAppInstallFinalizer::SetWebAppManifestFieldsAndWriteData(
 
   if (base::FeatureList::IsEnabled(features::kDesktopPWAsRunOnOsLogin) &&
       web_app_info.run_on_os_login) {
-    // TODO(crbug.com/1091964): Obtain actual mode, currently set to the default
-    // (windowed).
+    // TODO(crbug.com/1091964): Obtain actual mode, currently set to the
+    // default (windowed).
     web_app->SetRunOnOsLoginMode(RunOnOsLoginMode::kWindowed);
   }