Revert "Compute colors for the frame and tabs from provided images."

This reverts commit df84748a.

Reason for revert: To fix unit_tests (I have to revert this before the earlier patch). Likely suspect for BrowserThemePackTest.HiDpiThemeTest failing on bots. For example, https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel/11230 :

[ RUN      ] BrowserThemePackTest.HiDpiThemeTest
../../chrome/browser/themes/browser_theme_pack_unittest.cc:204: Failure
Expected equality of these values:
  ""
  error
    Which is: "File doesn't exist."
Stack trace:
#0 0x0000026103ac testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop()
#1 0x00000260fd89 testing::internal::AssertHelper::operator=()
#2 0x000000fc9b98 BrowserThemePackTest::BuildFromUnpackedExtension()
#3 0x000000fd22cc BrowserThemePackTest_HiDpiThemeTest_Test::TestBody()
../../chrome/browser/themes/browser_theme_pack_unittest.cc:205: Failure
Value of: valid_value.get()
  Actual: false
Expected: true
Stack trace:
#0 0x0000026103ac testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop()
#1 0x00000260fd89 testing::internal::AssertHelper::operator=()
#2 0x000000fc9e0b BrowserThemePackTest::BuildFromUnpackedExtension()
#3 0x000000fd22cc BrowserThemePackTest_HiDpiThemeTest_Test::TestBody()
Received signal 11 SEGV_MAPERR 000000000028
#0 0x000004d00e1c base::debug::StackTrace::StackTrace()
#1 0x000004d00981 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7f26bba79330 <unknown>
#3 0x0000050f3e24 BrowserThemePack::WriteToDisk()
#4 0x000000fd22db BrowserThemePackTest_HiDpiThemeTest_Test::TestBody()
#5 0x0000026161b2 testing::Test::Run()
#6 0x000002616d30 testing::TestInfo::Run()
#7 0x000002617247 testing::TestCase::Run()
#8 0x000002622747 testing::internal::UnitTestImpl::RunAllTests()
#9 0x0000026222bd testing::UnitTest::Run()
#10 0x0000045565d1 base::TestSuite::Run()
#11 0x000004557fca base::(anonymous namespace)::LaunchUnitTestsInternal()
#12 0x000004557e7a base::LaunchUnitTests()
#13 0x00000454e155 main
#14 0x7f26b86c9f45 __libc_start_main
#15 0x0000006fa82a _start
  r8: 0000000000000000  r9: 54656d6568546970 r10: 747365545f747365 r11: 0000000000000000
 r12: 000024a8af0c0d20 r13: 00007fff447924e8 r14: 00007fff44792560 r15: 0000000000000000
  di: 0000000000000000  si: 00007fff44792560  bp: 00007fff44792520  bx: 00007fff44792578
  dx: 00000000000005e7  ax: 0000000000001fdd  cx: 0000000000000023  sp: 00007fff44792490
  ip: 00000000050f3e24 efl: 0000000000010206 cgf: 0000000000000033 erf: 0000000000000004
 trp: 000000000000000e msk: 0000000000000000 cr2: 0000000000000028
[end of stack trace]

Original change's description:
> Compute colors for the frame and tabs from provided images.
> 
> If theme authors explicitly provide values here, we'll use them; but if they
> don't, but do provide images, set the colors to be the dominant colors of the
> images, using an existing K-means algorithm (similar to what we use for
> computing representative favicon colors etc.).
> 
> This allows us to rely on those colors later when deciding what color to make
> tab text, the new tab button, and similar tabstrip/window frame items.
> 
> This also does a bit of cleanup to the theme pack code, e.g. moving a map used
> in only one function into that function to make its provenance clearer.  I can
> try to split this apart into more CLs if desired.
> 
> Bug: 862664
> Change-Id: I8c3f15893ad491a6fee8aa5f76ae162cecb2c6fd
> Reviewed-on: https://chromium-review.googlesource.com/1152517
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578823}

TBR=pkasting@chromium.org,asvitkine@chromium.org,estade@chromium.org

Change-Id: I6a5a7f2916136ccf93101f36c34a17a4e5ef5d4f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 862664
Reviewed-on: https://chromium-review.googlesource.com/1153988Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578856}

chrome/browser/themes/browser_theme_pack.cc

@@ -30,7 +30,6 @@
 #include "ui/base/resource/data_pack.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/codec/png_codec.h"
-#include "ui/gfx/color_analysis.h"
 #include "ui/gfx/geometry/safe_integer_conversions.h"
 #include "ui/gfx/geometry/size_conversions.h"
 #include "ui/gfx/image/canvas_image_source.h"
@@ -275,6 +274,24 @@ int GetIntForString(const std::string& key,
   return -1;
 }
 
+struct IntToIntTable {
+  int key;
+  int value;
+};
+
+// Mapping used in CreateFrameImages() to associate frame images with the
+// tint ID that should maybe be applied to it.
+const IntToIntTable kFrameTintMap[] = {
+  { PRS_THEME_FRAME, ThemeProperties::TINT_FRAME },
+  { PRS_THEME_FRAME_INACTIVE, ThemeProperties::TINT_FRAME_INACTIVE },
+  { PRS_THEME_FRAME_OVERLAY, ThemeProperties::TINT_FRAME },
+  { PRS_THEME_FRAME_OVERLAY_INACTIVE,
+    ThemeProperties::TINT_FRAME_INACTIVE },
+  { PRS_THEME_FRAME_INCOGNITO, ThemeProperties::TINT_FRAME_INCOGNITO },
+  { PRS_THEME_FRAME_INCOGNITO_INACTIVE,
+    ThemeProperties::TINT_FRAME_INCOGNITO_INACTIVE },
+};
+
 struct CropEntry {
   int prs_id;
 
@@ -307,7 +324,7 @@ const struct CropEntry kImagesToCrop[] = {
 
 // A list of images that don't need tinting or any other modification and can
 // be byte-copied directly into the finished DataPack. This should contain the
-// persistent IDs for all themeable image IDs that aren't in kFrameValues,
+// persistent IDs for all themeable image IDs that aren't in kFrameTintMap,
 // kTabBackgroundMap or kImagesToCrop.
 const int kPreloadIDs[] = {
   PRS_THEME_NTP_BACKGROUND,
@@ -344,6 +361,15 @@ scoped_refptr<base::RefCountedMemory> ReadFileData(const base::FilePath& path) {
   return nullptr;
 }
 
+// Shifts an image's HSL values. The caller is responsible for deleting
+// the returned image.
+gfx::Image CreateHSLShiftedImage(const gfx::Image& image,
+                                 const color_utils::HSL& hsl_shift) {
+  const gfx::ImageSkia* src_image = image.ToImageSkia();
+  return gfx::Image(gfx::ImageSkiaOperations::CreateHSLShiftedImage(
+      *src_image, hsl_shift));
+}
+
 // Computes a bitmap at one scale from a bitmap at a different scale.
 SkBitmap CreateLowQualityResizedBitmap(const SkBitmap& source_bitmap,
                                        ui::ScaleFactor source_scale_factor,
@@ -535,37 +561,6 @@ BrowserThemePack::~BrowserThemePack() {
   }
 }
 
-void BrowserThemePack::SetColor(int id, SkColor color) {
-  DCHECK(colors_);
-
-  int first_available_color = -1;
-  for (size_t i = 0; i < kColorTableLength; ++i) {
-    if (colors_[i].id == id) {
-      colors_[i].color = color;
-      return;
-    }
-    if (colors_[i].id == -1 && first_available_color == -1)
-      first_available_color = i;
-  }
-
-  DCHECK_NE(-1, first_available_color);
-  colors_[first_available_color].id = id;
-  colors_[first_available_color].color = color;
-}
-
-void BrowserThemePack::ComputeColorFromImage(int color_id,
-                                             int height,
-                                             const gfx::Image& image) {
-  SkColor temp_color;
-  if (!GetColor(color_id, &temp_color)) {
-    // Include all colors in the analysis.
-    constexpr color_utils::HSL kNoBounds = {-1, -1, -1};
-    const SkColor color = color_utils::CalculateKMeanColorOfBitmap(
-        *image.ToSkBitmap(), height, kNoBounds, kNoBounds, false);
-    SetColor(color_id, color);
-  }
-}
-
 // static
 void BrowserThemePack::BuildFromExtension(
     const extensions::Extension* extension,
@@ -591,22 +586,7 @@ void BrowserThemePack::BuildFromExtension(
   if (!pack->LoadRawBitmapsTo(file_paths, &pack->images_))
     return;
 
-  pack->CropImages(&pack->images_);
-
-  // Create frame images, and generate frame colors from images where relevant.
-  // This must be done after reading colors from JSON (so they aren't
-  // overwritten).
-  pack->CreateFrameImagesAndColors(&pack->images_);
-
-  // Generate any missing frame colors.  This must be done after generating
-  // colors from the frame images, so only colors with no matching images are
-  // generated.
-  pack->GenerateFrameColors();
-
-  // Create the tab background images, and generate colors where relevant.  This
-  // must be done after all frame colors are set, since they are used when
-  // creating these.
-  pack->CreateTabBackgroundImagesAndColors(&pack->images_);
+  pack->CreateImages(&pack->images_);
 
   // Make sure the |images_on_file_thread_| has bitmaps for supported
   // scale factors before passing to FILE thread.
@@ -951,21 +931,7 @@ void BrowserThemePack::BuildColorsFromJSON(
   std::map<int, SkColor> temp_colors;
   if (colors_value)
     ReadColorsFromJSON(colors_value, &temp_colors);
-
-  // Generate inactive background tab text colors if active colors were
-  // specified.
-  static constexpr int kColorsToCopy[][2] = {
-    {ThemeProperties::COLOR_BACKGROUND_TAB_TEXT,
-     ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INACTIVE},
-    {ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO,
-     ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO_INACTIVE},
-  };
-  for (const int* text_colors : kColorsToCopy) {
-    const auto src_it = temp_colors.find(text_colors[0]);
-    if (src_it != temp_colors.end() &&
-        !base::ContainsKey(temp_colors, text_colors[1]))
-      temp_colors[text_colors[1]] = src_it->second;
-  }
+  GenerateMissingColors(&temp_colors);
 
   // Copy data from the intermediary data structure to the array.
   size_t count = 0;
@@ -1022,6 +988,54 @@ void BrowserThemePack::ReadColorsFromJSON(
   }
 }
 
+void BrowserThemePack::GenerateMissingColors(
+    std::map<int, SkColor>* colors) {
+  // Generate frame colors, if missing. (See GenerateFrameColors()).
+  SkColor frame;
+  std::map<int, SkColor>::const_iterator it =
+      colors->find(ThemeProperties::COLOR_FRAME);
+  if (it != colors->end()) {
+    frame = it->second;
+  } else {
+    frame =
+        ThemeProperties::GetDefaultColor(ThemeProperties::COLOR_FRAME, false);
+  }
+
+  if (!colors->count(ThemeProperties::COLOR_FRAME)) {
+    (*colors)[ThemeProperties::COLOR_FRAME] =
+        HSLShift(frame, GetTintInternal(ThemeProperties::TINT_FRAME));
+  }
+  if (!colors->count(ThemeProperties::COLOR_FRAME_INACTIVE)) {
+    (*colors)[ThemeProperties::COLOR_FRAME_INACTIVE] =
+        HSLShift(frame, GetTintInternal(
+            ThemeProperties::TINT_FRAME_INACTIVE));
+  }
+  if (!colors->count(ThemeProperties::COLOR_FRAME_INCOGNITO)) {
+    (*colors)[ThemeProperties::COLOR_FRAME_INCOGNITO] =
+        HSLShift(frame, GetTintInternal(
+            ThemeProperties::TINT_FRAME_INCOGNITO));
+  }
+  if (!colors->count(ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE)) {
+    (*colors)[ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE] =
+        HSLShift(frame, GetTintInternal(
+            ThemeProperties::TINT_FRAME_INCOGNITO_INACTIVE));
+  }
+
+  // Generate inactive background tab text colors if active colors were
+  // specified.
+  static constexpr int kColorsToCopy[][2] = {
+    {ThemeProperties::COLOR_BACKGROUND_TAB_TEXT,
+     ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INACTIVE},
+    {ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO,
+     ThemeProperties::COLOR_BACKGROUND_TAB_TEXT_INCOGNITO_INACTIVE},
+  };
+  for (const int* text_colors : kColorsToCopy) {
+    const auto src_it = colors->find(text_colors[0]);
+    if (src_it != colors->end() && !base::ContainsKey(*colors, text_colors[1]))
+      (*colors)[text_colors[1]] = src_it->second;
+  }
+}
+
 void BrowserThemePack::BuildDisplayPropertiesFromJSON(
     const base::DictionaryValue* display_properties_value) {
   display_properties_ = new DisplayPropertyPair[kDisplayPropertiesSize];
@@ -1201,6 +1215,12 @@ bool BrowserThemePack::LoadRawBitmapsTo(
   return true;
 }
 
+void BrowserThemePack::CreateImages(ImageCache* images) const {
+  CropImages(images);
+  CreateFrameImages(images);
+  CreateTabBackgroundImages(images);
+}
+
 void BrowserThemePack::CropImages(ImageCache* images) const {
   bool has_frame_border = HasFrameBorder();
   for (size_t i = 0; i < arraysize(kImagesToCrop); ++i) {
@@ -1219,104 +1239,41 @@ void BrowserThemePack::CropImages(ImageCache* images) const {
   }
 }
 
-void BrowserThemePack::CreateFrameImagesAndColors(ImageCache* images) {
-  static constexpr struct FrameValues {
-    int prs_id;
-    int tint_id;
-    base::Optional<int> color_id;
-  } kFrameValues[] = {
-      {PRS_THEME_FRAME, ThemeProperties::TINT_FRAME,
-       ThemeProperties::COLOR_FRAME},
-      {PRS_THEME_FRAME_INACTIVE, ThemeProperties::TINT_FRAME_INACTIVE,
-       ThemeProperties::COLOR_FRAME_INACTIVE},
-      {PRS_THEME_FRAME_OVERLAY, ThemeProperties::TINT_FRAME, base::nullopt},
-      {PRS_THEME_FRAME_OVERLAY_INACTIVE, ThemeProperties::TINT_FRAME_INACTIVE,
-       base::nullopt},
-      {PRS_THEME_FRAME_INCOGNITO, ThemeProperties::TINT_FRAME_INCOGNITO,
-       ThemeProperties::COLOR_FRAME_INCOGNITO},
-      {PRS_THEME_FRAME_INCOGNITO_INACTIVE,
-       ThemeProperties::TINT_FRAME_INCOGNITO_INACTIVE,
-       ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE},
-  };
-
+void BrowserThemePack::CreateFrameImages(ImageCache* images) const {
   // Create all the output images in a separate cache and move them back into
   // the input images because there can be name collisions.
   ImageCache temp_output;
 
-  for (const auto frame_values : kFrameValues) {
-    int src_id = frame_values.prs_id;
+  for (size_t i = 0; i < arraysize(kFrameTintMap); ++i) {
+    int prs_id = kFrameTintMap[i].key;
     // If the theme doesn't provide an image, attempt to fall back to one it
     // does.
-    if (!images->count(src_id)) {
+    if (!images->count(prs_id)) {
       // Fall back from inactive overlay to active overlay.
-      if (src_id == PRS_THEME_FRAME_OVERLAY_INACTIVE)
-        src_id = PRS_THEME_FRAME_OVERLAY;
+      if (prs_id == PRS_THEME_FRAME_OVERLAY_INACTIVE)
+        prs_id = PRS_THEME_FRAME_OVERLAY;
 
       // Fall back from inactive incognito to active incognito.
-      if (src_id == PRS_THEME_FRAME_INCOGNITO_INACTIVE)
-        src_id = PRS_THEME_FRAME_INCOGNITO;
+      if (prs_id == PRS_THEME_FRAME_INCOGNITO_INACTIVE)
+        prs_id = PRS_THEME_FRAME_INCOGNITO;
 
       // For all non-overlay images, fall back to PRS_THEME_FRAME as a last
       // resort.
-      if (!images->count(src_id) && src_id != PRS_THEME_FRAME_OVERLAY)
-        src_id = PRS_THEME_FRAME;
+      if (!images->count(prs_id) && prs_id != PRS_THEME_FRAME_OVERLAY)
+        prs_id = PRS_THEME_FRAME;
     }
 
     // Note that if the original ID and all the fallbacks are absent, the caller
     // will rely on the frame colors instead.
-    const auto image = images->find(src_id);
-    if (image != images->end()) {
-      const gfx::Image dest_image(
-          gfx::ImageSkiaOperations::CreateHSLShiftedImage(
-              *image->second.ToImageSkia(),
-              GetTintInternal(frame_values.tint_id)));
-
-      temp_output[frame_values.prs_id] = dest_image;
-
-      if (frame_values.color_id) {
-        // The tallest frame height above the top of tabs in any mode.
-        constexpr int kTallestFrameHeight = 19;
-        ComputeColorFromImage(frame_values.color_id.value(),
-                              kTallestTabHeight + kTallestFrameHeight,
-                              dest_image);
-      }
+    if (images->count(prs_id)) {
+      temp_output[kFrameTintMap[i].key] = CreateHSLShiftedImage(
+          (*images)[prs_id], GetTintInternal(kFrameTintMap[i].value));
     }
   }
   MergeImageCaches(temp_output, images);
 }
 
-void BrowserThemePack::GenerateFrameColors() {
-  using TP = ThemeProperties;
-
-  SkColor frame;
-  if (!GetColor(TP::COLOR_FRAME, &frame)) {
-    frame = ThemeProperties::GetDefaultColor(TP::COLOR_FRAME, false);
-    SetColor(TP::COLOR_FRAME,
-             HSLShift(frame, GetTintInternal(ThemeProperties::TINT_FRAME)));
-  }
-
-  SkColor temp;
-  if (!GetColor(TP::COLOR_FRAME_INACTIVE, &temp)) {
-    SetColor(
-        TP::COLOR_FRAME_INACTIVE,
-        HSLShift(frame, GetTintInternal(ThemeProperties::TINT_FRAME_INACTIVE)));
-  }
-
-  if (!GetColor(TP::COLOR_FRAME_INCOGNITO, &temp)) {
-    SetColor(TP::COLOR_FRAME_INCOGNITO,
-             HSLShift(frame,
-                      GetTintInternal(ThemeProperties::TINT_FRAME_INCOGNITO)));
-  }
-
-  if (!GetColor(TP::COLOR_FRAME_INCOGNITO_INACTIVE, &temp)) {
-    SetColor(
-        TP::COLOR_FRAME_INCOGNITO_INACTIVE,
-        HSLShift(frame, GetTintInternal(
-                            ThemeProperties::TINT_FRAME_INCOGNITO_INACTIVE)));
-  }
-}
-
-void BrowserThemePack::CreateTabBackgroundImagesAndColors(ImageCache* images) {
+void BrowserThemePack::CreateTabBackgroundImages(ImageCache* images) const {
   static constexpr struct TabValues {
     // The background image to create/update.
     int tab_id;
@@ -1331,22 +1288,16 @@ void BrowserThemePack::CreateTabBackgroundImagesAndColors(ImageCache* images) {
 
     // The frame color to use as the base of this tab background image.
     int frame_color_id;
-
-    // The color to compute and store for this image, if not present.
-    int color_id;
   } kTabBackgroundMap[] = {
       {PRS_THEME_TAB_BACKGROUND, base::nullopt, PRS_THEME_FRAME,
-       ThemeProperties::COLOR_FRAME, ThemeProperties::COLOR_BACKGROUND_TAB},
+       ThemeProperties::COLOR_FRAME},
       {PRS_THEME_TAB_BACKGROUND_INACTIVE, PRS_THEME_TAB_BACKGROUND,
-       PRS_THEME_FRAME_INACTIVE, ThemeProperties::COLOR_FRAME_INACTIVE,
-       ThemeProperties::COLOR_BACKGROUND_TAB_INACTIVE},
+       PRS_THEME_FRAME_INACTIVE, ThemeProperties::COLOR_FRAME_INACTIVE},
       {PRS_THEME_TAB_BACKGROUND_INCOGNITO, base::nullopt,
-       PRS_THEME_FRAME_INCOGNITO, ThemeProperties::COLOR_FRAME_INCOGNITO,
-       ThemeProperties::COLOR_BACKGROUND_TAB_INCOGNITO},
+       PRS_THEME_FRAME_INCOGNITO, ThemeProperties::COLOR_FRAME_INCOGNITO},
       {PRS_THEME_TAB_BACKGROUND_INCOGNITO_INACTIVE,
        PRS_THEME_TAB_BACKGROUND_INCOGNITO, PRS_THEME_FRAME_INCOGNITO_INACTIVE,
-       ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE,
-       ThemeProperties::COLOR_BACKGROUND_TAB_INCOGNITO_INACTIVE},
+       ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE},
   };
 
   ImageCache temp_output;
@@ -1395,11 +1346,8 @@ void BrowserThemePack::CreateTabBackgroundImagesAndColors(ImageCache* images) {
           kRestoredTabVerticalOffset);
       gfx::Size dest_size = image_to_tint.size();
       dest_size.SetToMax(gfx::Size(0, kTallestTabHeight));
-      const gfx::Image dest_image(gfx::ImageSkia(std::move(source), dest_size));
-      temp_output[tab_id] = dest_image;
-
-      ComputeColorFromImage(kTabBackgroundMap[i].color_id, kTallestTabHeight,
-                            dest_image);
+      temp_output[tab_id] =
+          gfx::Image(gfx::ImageSkia(std::move(source), dest_size));
     }
   }
   MergeImageCaches(temp_output, images);

chrome/browser/themes/browser_theme_pack.h

@@ -111,14 +111,6 @@ class BrowserThemePack : public CustomThemeSupplier {
 
   ~BrowserThemePack() override;
 
-  // Modifies |colors_| to set the entry with identifier |id| to |color|.  Only
-  // valid to call after BuildColorsFromJSON(), which creates |colors_|.
-  void SetColor(int id, SkColor color);
-
-  // If |colors_| does not already contain an entry with identifier |id|, sets
-  // it to the dominant color of the top |height| rows of |image|.
-  void ComputeColorFromImage(int id, int height, const gfx::Image& image);
-
   // Builds a header ready to write to disk.
   void BuildHeader(const extensions::Extension* extension);
 
@@ -133,6 +125,7 @@ class BrowserThemePack : public CustomThemeSupplier {
   // Implementation details of BuildColorsFromJSON().
   void ReadColorsFromJSON(const base::DictionaryValue* colors_value,
                           std::map<int, SkColor>* temp_colors);
+  void GenerateMissingColors(std::map<int, SkColor>* temp_colors);
 
   // Transforms the JSON display properties into |display_properties_|.
   void BuildDisplayPropertiesFromJSON(
@@ -157,23 +150,23 @@ class BrowserThemePack : public CustomThemeSupplier {
   bool LoadRawBitmapsTo(const FilePathMap& file_paths,
                         ImageCache* image_cache);
 
+  // Populate |images| cache with empty gfx::Images. Image reps are lazily
+  // generated when an image rep is requested via ImageSkia::GetRepresentation.
+  // Source and destination is |images|.
+  void CreateImages(ImageCache* images) const;
+
   // Crops images down to a size such that most of the cropped image will be
   // displayed in the UI. Cropping is useful because images from custom themes
   // can be of any size. Source and destination is |images|.
   void CropImages(ImageCache* images) const;
 
   // Creates tinted and composited frame images. Source and destination is
-  // |images|.  Also sets frame colors corresponding to these images if no
-  // explicit color has been specified for these colors.
-  void CreateFrameImagesAndColors(ImageCache* images);
-
-  // Generates any frame colors which have not already been set.
-  void GenerateFrameColors();
+  // |images|.
+  void CreateFrameImages(ImageCache* images) const;
 
   // Creates the semi-transparent tab background images, putting the results
-  // in |images|.  Also sets colors corresponding to these images if no explicit
-  // color has been specified.  Must be called after GenerateFrameImages().
-  void CreateTabBackgroundImagesAndColors(ImageCache* images);
+  // in |images|. Must be called after GenerateFrameImages().
+  void CreateTabBackgroundImages(ImageCache* images) const;
 
   // Takes all the SkBitmaps in |images|, encodes them as PNGs and places
   // them in |reencoded_images|.

chrome/browser/themes/browser_theme_pack_unittest.cc

@@ -145,7 +145,6 @@ void BrowserThemePackTest::LoadColorJSON(const std::string& json) {
 
 void BrowserThemePackTest::LoadColorDictionary(base::DictionaryValue* value) {
   theme_pack_->BuildColorsFromJSON(value);
-  theme_pack_->GenerateFrameColors();
 }
 
 void BrowserThemePackTest::LoadTintJSON(const std::string& json) {

ui/gfx/color_analysis.cc

@@ -543,8 +543,7 @@ SkColor CalculateKMeanColorOfBuffer(uint8_t* decoded_data,
                                     int img_height,
                                     const HSL& lower_bound,
                                     const HSL& upper_bound,
-                                    KMeanImageSampler* sampler,
-                                    bool find_closest) {
+                                    KMeanImageSampler* sampler) {
   SkColor color = kDefaultBgColor;
   if (img_width > 0 && img_height > 0) {
     std::vector<KMeanCluster> clusters;
@@ -676,11 +675,9 @@ SkColor CalculateKMeanColorOfBuffer(uint8_t* decoded_data,
     }
   }
 
-  // The K-mean cluster center will not usually be a color that appears in the
-  // image.  If desired, find a color that actually appears.
-  return find_closest
-             ? FindClosestColor(decoded_data, img_width, img_height, color)
-             : color;
+  // Find a color that actually appears in the image (the K-mean cluster center
+  // will not usually be a color that appears in the image).
+  return FindClosestColor(decoded_data, img_width, img_height, color);
 }
 
 SkColor CalculateKMeanColorOfPNG(scoped_refptr<base::RefCountedMemory> png,
@@ -693,11 +690,18 @@ SkColor CalculateKMeanColorOfPNG(scoped_refptr<base::RefCountedMemory> png,
   SkColor color = kDefaultBgColor;
 
   if (png.get() && png->size() &&
-      gfx::PNGCodec::Decode(png->front(), png->size(),
-                            gfx::PNGCodec::FORMAT_BGRA, &decoded_data,
-                            &img_width, &img_height)) {
-    return CalculateKMeanColorOfBuffer(&decoded_data[0], img_width, img_height,
-                                       lower_bound, upper_bound, sampler, true);
+      gfx::PNGCodec::Decode(png->front(),
+                            png->size(),
+                            gfx::PNGCodec::FORMAT_BGRA,
+                            &decoded_data,
+                            &img_width,
+                            &img_height)) {
+    return CalculateKMeanColorOfBuffer(&decoded_data[0],
+                                       img_width,
+                                       img_height,
+                                       lower_bound,
+                                       upper_bound,
+                                       sampler);
   }
   return color;
 }
@@ -709,27 +713,25 @@ SkColor CalculateKMeanColorOfPNG(scoped_refptr<base::RefCountedMemory> png) {
 }
 
 SkColor CalculateKMeanColorOfBitmap(const SkBitmap& bitmap,
-                                    int height,
                                     const HSL& lower_bound,
                                     const HSL& upper_bound,
-                                    bool find_closest) {
+                                    KMeanImageSampler* sampler) {
   // SkBitmap uses pre-multiplied alpha but the KMean clustering function
   // above uses non-pre-multiplied alpha. Transform the bitmap before we
   // analyze it because the function reads each pixel multiple times.
-  int pixel_count = bitmap.width() * height;
+  int pixel_count = bitmap.width() * bitmap.height();
   std::unique_ptr<uint32_t[]> image(new uint32_t[pixel_count]);
   UnPreMultiply(bitmap, image.get(), pixel_count);
 
-  GridSampler sampler;
   return CalculateKMeanColorOfBuffer(reinterpret_cast<uint8_t*>(image.get()),
-                                     bitmap.width(), height, lower_bound,
-                                     upper_bound, &sampler, find_closest);
+                                     bitmap.width(), bitmap.height(),
+                                     lower_bound, upper_bound, sampler);
 }
 
 SkColor CalculateKMeanColorOfBitmap(const SkBitmap& bitmap) {
+  GridSampler sampler;
   return CalculateKMeanColorOfBitmap(
-      bitmap, bitmap.height(), kDefaultLowerHSLBound, kDefaultUpperHSLBound,
-      true);
+      bitmap, kDefaultLowerHSLBound, kDefaultUpperHSLBound, &sampler);
 }
 
 std::vector<SkColor> CalculateProminentColorsOfBitmap(

ui/gfx/color_analysis.h

@@ -98,17 +98,12 @@ GFX_EXPORT SkColor
 GFX_EXPORT SkColor CalculateKMeanColorOfPNG(
     scoped_refptr<base::RefCountedMemory> png);
 
-// Computes a dominant color for the first |height| rows of |bitmap| using the
-// above algorithm and a reasonable default sampler. If |find_closest| is true,
-// the returned color will be the closest color to the true K-mean color that
-// actually appears in the image; if false, the true color is returned
-// regardless of whether it actually appears.
+// Returns an SkColor that represents the calculated dominant color in the
+// image. See CalculateKMeanColorOfPNG() for details.
 GFX_EXPORT SkColor CalculateKMeanColorOfBitmap(const SkBitmap& bitmap,
-                                               int height,
                                                const HSL& lower_bound,
                                                const HSL& upper_bound,
-                                               bool find_closest);
-
+                                               KMeanImageSampler* sampler);
 // Computes a dominant color using the above algorithm and reasonable defaults
 // for |lower_bound|, |upper_bound| and |sampler|.
 GFX_EXPORT SkColor CalculateKMeanColorOfBitmap(const SkBitmap& bitmap);