Make theme background tab text readable

While importing a theme, browser_theme_pack now runs through a pass to ensure that tab title text for background tabs maintains a minimum contrast ratio with the tab background.



Bug: 871026
Change-Id: Ic1d76c9d50ac0255de671916138933dd96af49ef
Reviewed-on: https://chromium-review.googlesource.com/1173491
Commit-Queue: Ryan Meier <rameier@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584806}

chrome/browser/themes/browser_theme_pack.h

@@ -175,6 +175,16 @@ class BrowserThemePack : public CustomThemeSupplier {
   // color has been specified.  Must be called after GenerateFrameImages().
   void CreateTabBackgroundImagesAndColors(ImageCache* images);
 
+  // Generates any text colors which have not already been set.
+  void GenerateMissingTextColors();
+
+  // Generates text color for the specified id |text_color_id|, based on the
+  // background color of the tab |tab_color_id|, and using the color already
+  // defined for |source_color_id| as a starting point (if it exists).
+  void GenerateMissingTextColorForID(int text_color_id,
+                                     int tab_color_id,
+                                     int source_color_id);
+
   // Takes all the SkBitmaps in |images|, encodes them as PNGs and places
   // them in |reencoded_images|.
   void RepackImages(const ImageCache& images,

chrome/browser/themes/browser_theme_pack_unittest.cc

@@ -64,6 +64,8 @@ class BrowserThemePackTest : public ::testing::Test {
   static void BuildFromUnpackedExtension(const base::FilePath& extension_path,
                                          scoped_refptr<BrowserThemePack>* pack);
 
+  static base::FilePath GetTestExtensionThemePath(
+      base::StringPiece theme_folder);
   static base::FilePath GetStarGazingPath();
   static base::FilePath GetHiDpiThemePath();
 
@@ -74,6 +76,12 @@ class BrowserThemePackTest : public ::testing::Test {
 
   static void VerifyHiDpiTheme(BrowserThemePack* pack);
 
+  // Verify that the colors in the theme for |color_id_a| and |color_id_b| are
+  // the same.
+  static void VerifyColorsMatch(BrowserThemePack* pack,
+                                TP::OverwritableByUserThemeProperty color_id_a,
+                                TP::OverwritableByUserThemeProperty color_id_b);
+
   const BrowserThemePack& theme_pack() const { return *theme_pack_; }
 
  private:
@@ -214,6 +222,18 @@ void BrowserThemePackTest::BuildFromUnpackedExtension(
   ASSERT_TRUE((*pack)->is_valid());
 }
 
+// static
+base::FilePath BrowserThemePackTest::GetTestExtensionThemePath(
+    base::StringPiece theme_folder) {
+  base::FilePath test_path;
+  const bool result = base::PathService::Get(chrome::DIR_TEST_DATA, &test_path);
+  DCHECK(result);
+
+  test_path = test_path.AppendASCII("extensions");
+  test_path = test_path.AppendASCII(theme_folder);
+  return base::FilePath(test_path);
+}
+
 // static
 base::FilePath BrowserThemePackTest::GetStarGazingPath() {
   base::FilePath test_path;
@@ -412,6 +432,26 @@ void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
   }
 }
 
+// static
+void BrowserThemePackTest::VerifyColorsMatch(
+    BrowserThemePack* pack,
+    TP::OverwritableByUserThemeProperty color_id_a,
+    TP::OverwritableByUserThemeProperty color_id_b) {
+  SkColor color_a;
+  SkColor color_b;
+
+  bool color_a_set = pack->GetColor(color_id_a, &color_a);
+  bool color_b_set = pack->GetColor(color_id_b, &color_b);
+
+  SCOPED_TRACE(testing::Message()
+               << "Color A: " << std::hex << color_a << " (ID: " << std::dec
+               << color_id_a << "), Color B: " << std::hex << color_b
+               << " (ID: " << std::dec << color_id_b << ")");
+  EXPECT_TRUE(color_a_set);
+  EXPECT_TRUE(color_b_set);
+  EXPECT_EQ(color_a, color_b);
+}
+
 // static
 SkColor BrowserThemePackTest::BuildThirdOpacity(SkColor color_link) {
   return SkColorSetA(color_link, SkColorGetA(color_link) / 3);
@@ -490,12 +530,6 @@ TEST_F(BrowserThemePackTest, SupportsAlpha) {
   colors[TP::COLOR_TOOLBAR] = SkColorSetARGB(255, 0, 20, 40);
   colors[TP::COLOR_TAB_TEXT] = SkColorSetARGB(255, 60, 80, 100);
   colors[TP::COLOR_BACKGROUND_TAB_TEXT] = SkColorSetARGB(0, 120, 140, 160);
-  colors[TP::COLOR_BACKGROUND_TAB_TEXT_INACTIVE] =
-      colors[TP::COLOR_BACKGROUND_TAB_TEXT];
-  colors[TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO] =
-      colors[TP::COLOR_BACKGROUND_TAB_TEXT];
-  colors[TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO_INACTIVE] =
-      colors[TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO];
   colors[TP::COLOR_BOOKMARK_TEXT] = SkColorSetARGB(255, 180, 200, 220);
   colors[TP::COLOR_NTP_TEXT] = SkColorSetARGB(128, 240, 255, 0);
   VerifyColorMap(colors);
@@ -703,3 +737,88 @@ TEST_F(BrowserThemePackTest, HiDpiThemeTest) {
     VerifyHiDpiTheme(pack.get());
   }
 }
+
+// Ensure that, given a theme that specifies background tab/text colors which
+// are too similar, the importing process modifies the text color so that it
+// maintains a minimum readable contrast ratio with the background.
+TEST_F(BrowserThemePackTest, TestBackgroundTabTextMinimumContrast) {
+  // Build a theme from test file (theme_tabcontrast).
+  base::FilePath contrast_theme_path =
+      GetTestExtensionThemePath("theme_tabcontrast");
+  scoped_refptr<BrowserThemePack> pack;
+  BuildFromUnpackedExtension(contrast_theme_path, &pack);
+
+  // Check the contrast ratio of text/tab color pairs to make sure that they
+  // meet the minimum criteria for readable contrast ratio.
+  struct TabColorPair {
+    TP::OverwritableByUserThemeProperty tab_color_id;
+    TP::OverwritableByUserThemeProperty text_color_id;
+  };
+  const TabColorPair color_pairs_to_check[] = {
+      {TP::COLOR_BACKGROUND_TAB, TP::COLOR_BACKGROUND_TAB_TEXT},
+      {TP::COLOR_BACKGROUND_TAB_INACTIVE,
+       TP::COLOR_BACKGROUND_TAB_TEXT_INACTIVE},
+      {TP::COLOR_BACKGROUND_TAB_INCOGNITO,
+       TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO},
+      {TP::COLOR_BACKGROUND_TAB_INCOGNITO_INACTIVE,
+       TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO_INACTIVE},
+  };
+
+  for (const TabColorPair& current_pair : color_pairs_to_check) {
+    SkColor cur_tab_color;
+    SkColor cur_text_color;
+
+    pack->GetColor(current_pair.tab_color_id, &cur_tab_color);
+    pack->GetColor(current_pair.text_color_id, &cur_text_color);
+
+    float contrast_ratio =
+        color_utils::GetContrastRatio(cur_tab_color, cur_text_color);
+
+    EXPECT_GE(contrast_ratio, color_utils::kMinimumReadableContrastRatio);
+  }
+}
+
+// Ensure that, given a theme which only specifies a color for
+// COLOR_BACKGROUND_TAB_TEXT, that color is used for the other variants of
+// background tab text (inactive, incognito, and incognito+inactive).
+TEST_F(BrowserThemePackTest, TestBGTabTextColorAutoAssign) {
+  // Build a theme from the test file (theme_testinherittextcolor)
+  // This theme specifies a color for background_tab_text, but none of its
+  // variants.
+  base::FilePath contrast_theme_path =
+      GetTestExtensionThemePath("theme_testinherittextcolor");
+  scoped_refptr<BrowserThemePack> pack;
+  BuildFromUnpackedExtension(contrast_theme_path, &pack);
+
+  // Verify that all background tab text colors match the color for background
+  // tab text.
+  BrowserThemePack* pack_ptr = pack.get();
+  VerifyColorsMatch(pack_ptr, TP::COLOR_BACKGROUND_TAB_TEXT_INACTIVE,
+                    TP::COLOR_BACKGROUND_TAB_TEXT);
+  VerifyColorsMatch(pack_ptr, TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO,
+                    TP::COLOR_BACKGROUND_TAB_TEXT);
+  VerifyColorsMatch(pack_ptr, TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO_INACTIVE,
+                    TP::COLOR_BACKGROUND_TAB_TEXT);
+}
+
+// Ensure that, given a theme which only specifies colors for
+// COLOR_BACKGROUND_TAB_TEXT and COLOR_BACKGROUND_TAB_TEXT_INCOGNITO, those
+// colors are also used for their respective inactive variants.
+TEST_F(BrowserThemePackTest, TestBGTabTextColorAutoAssign_WithIncognito) {
+  // Build a theme from the test file (theme_testinherittextcolor_withincog)
+  // This theme specifies a color for background_tab_text and
+  // background_tab_text_incognito, but neither of their inactive variants.
+  base::FilePath contrast_theme_path =
+      GetTestExtensionThemePath("theme_testinherittextcolor_withincog");
+  scoped_refptr<BrowserThemePack> pack;
+  BuildFromUnpackedExtension(contrast_theme_path, &pack);
+
+  // Verify that background_inactive is getting its color from background, and
+  // background_incognito_inactive is getting its color from
+  // background_incognito.
+  BrowserThemePack* pack_ptr = pack.get();
+  VerifyColorsMatch(pack_ptr, TP::COLOR_BACKGROUND_TAB_TEXT_INACTIVE,
+                    TP::COLOR_BACKGROUND_TAB_TEXT);
+  VerifyColorsMatch(pack_ptr, TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO_INACTIVE,
+                    TP::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO);
+}

chrome/browser/themes/theme_properties.cc

@@ -194,7 +194,6 @@ base::Optional<SkColor> MaybeGetDefaultColorForNewerMaterialUi(int id,
       return base::nullopt;
   }
 }
-
 }  // namespace
 
 // static
@@ -399,3 +398,35 @@ SkColor ThemeProperties::GetDefaultColor(int id, bool incognito) {
 
   return gfx::kPlaceholderColor;
 }
+
+// static
+SkColor ThemeProperties::GetDefaultColor(PropertyLookupPair lookup_pair) {
+  return GetDefaultColor(lookup_pair.property_id, lookup_pair.is_incognito);
+}
+
+// static
+ThemeProperties::PropertyLookupPair ThemeProperties::GetLookupID(int input_id) {
+  // Mapping of incognito property ids to their corresponding non-incognito
+  // property ids.
+  base::flat_map<int, int> incognito_property_map({
+      {ThemeProperties::COLOR_FRAME_INCOGNITO, ThemeProperties::COLOR_FRAME},
+      {ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE,
+       ThemeProperties::COLOR_FRAME_INACTIVE},
+      {ThemeProperties::COLOR_BACKGROUND_TAB_INCOGNITO,
+       ThemeProperties::COLOR_BACKGROUND_TAB},
+      {ThemeProperties::COLOR_BACKGROUND_TAB_INCOGNITO_INACTIVE,
+       ThemeProperties::COLOR_BACKGROUND_TAB_INACTIVE},
+      {ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO,
+       ThemeProperties::COLOR_BACKGROUND_TAB_TEXT},
+      {ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO_INACTIVE,
+       ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INACTIVE},
+      {ThemeProperties::TINT_FRAME_INCOGNITO, ThemeProperties::TINT_FRAME},
+      {ThemeProperties::TINT_FRAME_INCOGNITO_INACTIVE,
+       ThemeProperties::TINT_FRAME_INACTIVE},
+  });
+
+  auto found_entry = incognito_property_map.find(input_id);
+  if (found_entry != incognito_property_map.end())
+    return {found_entry->second, true};
+  return {input_id, false};
+}

chrome/browser/themes/theme_properties.h

@@ -171,6 +171,14 @@ class ThemeProperties {
 #endif  // OS_WIN
   };
 
+  // Represents the lookup values for a theme property.
+  struct PropertyLookupPair {
+    int property_id;    // ID of the property to lookup (should never be an
+                        // incognito variant)
+    bool is_incognito;  // Whether the lookup should use the incognito value
+                        // of this property or not
+  };
+
   // Themes are hardcoded to draw frame images as if they start this many DIPs
   // above the top of the tabstrip, no matter how much space actually exists.
   // This aids with backwards compatibility (for some themes; Chrome's behavior
@@ -204,6 +212,16 @@ class ThemeProperties {
   // Returns gfx::kPlaceholderColor if |id| is invalid.
   static SkColor GetDefaultColor(int id, bool incognito);
 
+  // Returns the default color for the color represented by |lookup_pair|
+  // Returns gfx::kPlaceholderColor if |id| is invalid.
+  static SkColor GetDefaultColor(PropertyLookupPair lookup_pair);
+
+  // Get the PropertyLookupPair  necessary to look up a property for |input_id|
+  // in an incognito-aware context.  Returns a pair with the id to lookup
+  // (always a non-incognito variant), and a boolean representing whether
+  // |input_id| was an incognito variant of the id to lookup
+  static PropertyLookupPair GetLookupID(int input_id);
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(ThemeProperties);
 };

chrome/test/data/extensions/theme_tabcontrast/manifest.json

+{
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1P1I1EQSqVS1eIEFWGNSvEuKHSb1iJZMhIubqZF7cAEnZkF9BzTfvW46p3Nbr/2U4I7z7ndFK2nuEJPsA6p/GNJmIyqjTN7Pv9tjGUW0i5sDteB6ohCpuLrmBU9SjSdR9TI1lm6Q3N6+GcrYmL/Jm6he6CB7h0Fs1rSNO53z/9rB9J7tm9YmmsBi91J4PIrW+njhOIe+rtM2+xVTZBsvIreLFCsDTo3jlqdx5ZmaDBIlBCmp7JA7YSaBVgPVlYAKuV9g18ye25Y7bE62OyOHjhnBQAee2MXm+6T3cmCWQqOe62K2sZmtBwnlq08nxtb8pJSHaah16q6JJEqNPlimcwIDAQAB",
+   "manifest_version": 2,
+   "name": "tabcontrast",
+   "theme": {
+      "colors": {
+        "background_tab": [0, 0, 0],
+        "tab_background_text": [0,0,0],
+        "background_tab_inactive": [50,50,50],
+        "tab_background_text_inactive": [50,50,50],
+        "background_tab_incognito": [100,100,100],
+        "tab_background_text_incognito": [150,150,150],
+        "background_tab_incognito_inactive": [250,250,250],
+        "tab_background_text_incognito_inactive": [250,250,250]
+      }
+   },
+   "version": "0.0.1"
+}

chrome/test/data/extensions/theme_testinherittextcolor/manifest.json

+{
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAoZ6azhu+B5FfbcdldseZLMe73TPeTplm+fQ/gYWjsn3Ms40PtUDNHbpI+YPa8fmjml9cKa13BgDdGUXDBw9wx+QZ7nMOxlcmNYg8+GYp76Ee4Y/KTQss8kJ+rDJZMwZnEUL2ZxW0ejEjgbvtwpTOe4kqQokA1vggODMYD25KxqOp/pNUIqf1kuvFpQyEBpVifF4zRH7VV/OKyZKM3zp52JCr9rqKw+25LcrZTUj54XnnFwVdsXYwWTef9rYlefuB0vC62NqY2DstvhmJBdTXEievsMQO37zsbYaRksqwtsM7LbG7jJrCnAZgV14qRfKvlNciNU7vzelkvh0OdGPpqwIDAQAB",
+   "manifest_version": 2,
+   "name": "test_inherit_text_color",
+   "theme": {
+      "colors": {
+        "background_tab": [0, 0, 0],
+        "tab_background_text": [0,255,0],
+        "background_tab_inactive": [0,0,0],
+        "background_tab_incognito": [0,0,0],
+        "background_tab_incognito_inactive": [0,0,0]
+      }
+   },
+   "version": "0.0.1"
+}
\ No newline at end of file

chrome/test/data/extensions/theme_testinherittextcolor_withincog/manifest.json

+{
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2vnzjgsJi9KCBCQoY2mS8dBTk8qvEfsUGqjeJjXjoXwW01rJvuuYckCOaUJK/HIXl03iom1D8WZ4BlDRTKRmAlsRpllUmHsC7tDmcFB4+3rv1k5VGCmSH3FO1FoCSD1ZulE7pupq7Ifkwx9/NxfzYqsfkRjrr6P062Qtzaj5pVW3Ijv/tSEwTh8cXskFMERtjx+Bb86iqDddytuT/14ABSjMb3BuEaDj1bKhF+RrdNubMM9eliA7HAi0b1WXPVskXUeR1zGIOZYCqjpVfedaRbLtQ5Jk+zoTgkF9zH8LQpDwC2Ew5oaP/rnb7NGUowkT+l0uI+7rAet5tBKTn8HFJwIDAQAB",
+   "manifest_version": 2,
+   "name": "test_inherit_text_color_with_incog",
+   "theme": {
+      "colors": {
+        "background_tab": [0, 0, 0],
+        "tab_background_text": [0,255,0],
+        "background_tab_inactive": [0,0,0],
+        "background_tab_incognito": [0,0,0],
+		"tab_background_text_incognito": [0,0,255],
+        "background_tab_incognito_inactive": [0,0,0]
+      }
+   },
+   "version": "0.0.1"
+}
\ No newline at end of file