Crop all frame theme icons to avoid potentially large memory useage

Bug: 854675
Change-Id: Ic1366d927ef3918ff21c8656a4f90baf30768557
Reviewed-on: https://chromium-review.googlesource.com/1164384
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581348}

chrome/browser/themes/browser_theme_pack.cc

@@ -49,11 +49,14 @@ namespace {
 // The tallest tab height in any mode.
 constexpr int kTallestTabHeight = 41;
 
+// The tallest height above the tabs in any mode is 19 DIP.
+constexpr int kTallestFrameHeight = kTallestTabHeight + 19;
+
 // Version number of the current theme pack. We just throw out and rebuild
 // theme packs that aren't int-equal to this. Increment this number if you
 // change default theme assets or if you need themes to recreate their generated
 // images (which are cached).
-const int kThemePackVersion = 56;
+const int kThemePackVersion = 57;
 
 // IDs that are in the DataPack won't clash with the positive integer
 // uint16_t. kHeaderID should always have the maximum value because we want the
@@ -282,12 +285,6 @@ struct CropEntry {
 
   // The maximum useful height of the image at |prs_id|.
   int max_height;
-
-  // Whether cropping the image at |prs_id| should be skipped on OSes which
-  // have a frame border to the left and right of the web contents.
-  // This should be true for images which can be used to decorate the border to
-  // the left and the right of the web contents.
-  bool skip_if_frame_border;
 };
 
 // The images which should be cropped before being saved to the data pack. The
@@ -296,15 +293,15 @@ struct CropEntry {
 // |kThemePackVersion| must be incremented if any of the maximum heights below
 // are modified.
 const struct CropEntry kImagesToCrop[] = {
-  { PRS_THEME_FRAME, 120, true },
-  { PRS_THEME_FRAME_INACTIVE, 120, true },
-  { PRS_THEME_FRAME_INCOGNITO, 120, true },
-  { PRS_THEME_FRAME_INCOGNITO_INACTIVE, 120, true },
-  { PRS_THEME_FRAME_OVERLAY, 120, true },
-  { PRS_THEME_FRAME_OVERLAY_INACTIVE, 120, true },
-  { PRS_THEME_TOOLBAR, 200, false },
-  { PRS_THEME_BUTTON_BACKGROUND, 60, false },
-  { PRS_THEME_WINDOW_CONTROL_BACKGROUND, 50, false },
+    {PRS_THEME_FRAME, kTallestFrameHeight},
+    {PRS_THEME_FRAME_INACTIVE, kTallestFrameHeight},
+    {PRS_THEME_FRAME_INCOGNITO, kTallestFrameHeight},
+    {PRS_THEME_FRAME_INCOGNITO_INACTIVE, kTallestFrameHeight},
+    {PRS_THEME_FRAME_OVERLAY, kTallestFrameHeight},
+    {PRS_THEME_FRAME_OVERLAY_INACTIVE, kTallestFrameHeight},
+    {PRS_THEME_TOOLBAR, 200},
+    {PRS_THEME_BUTTON_BACKGROUND, 60},
+    {PRS_THEME_WINDOW_CONTROL_BACKGROUND, 50},
 };
 
 // A list of images that don't need tinting or any other modification and can
@@ -316,16 +313,6 @@ const int kPreloadIDs[] = {
   PRS_THEME_NTP_ATTRIBUTION,
 };
 
-// Returns true if this OS uses a browser frame which has a non zero width to
-// the left and the right of the web contents.
-bool HasFrameBorder() {
-#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
-  return false;
-#else
-  return true;
-#endif
-}
-
 // Returns a piece of memory with the contents of the file |path|.
 scoped_refptr<base::RefCountedMemory> ReadFileData(const base::FilePath& path) {
   if (!path.empty()) {
@@ -1229,11 +1216,7 @@ bool BrowserThemePack::LoadRawBitmapsTo(
 }
 
 void BrowserThemePack::CropImages(ImageCache* images) const {
-  bool has_frame_border = HasFrameBorder();
   for (size_t i = 0; i < arraysize(kImagesToCrop); ++i) {
-    if (has_frame_border && kImagesToCrop[i].skip_if_frame_border)
-      continue;
-
     int prs_id = kImagesToCrop[i].prs_id;
     ImageCache::iterator it = images->find(prs_id);
     if (it == images->end())
@@ -1301,11 +1284,8 @@ void BrowserThemePack::CreateFrameImagesAndColors(ImageCache* images) {
       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);
+                              kTallestFrameHeight, dest_image);
       }
     }
   }

chrome/browser/themes/browser_theme_pack_unittest.cc

@@ -342,7 +342,8 @@ void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
   const gfx::ImageSkiaRep& rep1 = image_skia->GetRepresentation(1.0f);
   ASSERT_FALSE(rep1.is_null());
   EXPECT_EQ(80, rep1.sk_bitmap().width());
-  EXPECT_EQ(80, rep1.sk_bitmap().height());
+  // Bitmap height will be cropped at 60 - kTallestTabHeight + 19.
+  EXPECT_EQ(60, rep1.sk_bitmap().height());
   EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep1.sk_bitmap().getColor(4, 4));
   EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep1.sk_bitmap().getColor(8, 8));
   EXPECT_EQ(SkColorSetRGB(0, 241, 237), rep1.sk_bitmap().getColor(16, 16));
@@ -352,7 +353,8 @@ void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
   const gfx::ImageSkiaRep& rep2 = image_skia->GetRepresentation(2.0f);
   ASSERT_FALSE(rep2.is_null());
   EXPECT_EQ(160, rep2.sk_bitmap().width());
-  EXPECT_EQ(160, rep2.sk_bitmap().height());
+  // Cropped height will be 2 * 60.
+  EXPECT_EQ(120, rep2.sk_bitmap().height());
   EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep2.sk_bitmap().getColor(4, 4));
   EXPECT_EQ(SkColorSetRGB(223, 42, 0), rep2.sk_bitmap().getColor(8, 8));
   EXPECT_EQ(SkColorSetRGB(223, 42, 0), rep2.sk_bitmap().getColor(16, 16));
@@ -378,7 +380,8 @@ void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
   const gfx::ImageSkiaRep& rep3 = image_skia->GetRepresentation(1.0f);
   ASSERT_FALSE(rep3.is_null());
   EXPECT_EQ(80, rep3.sk_bitmap().width());
-  EXPECT_EQ(80, rep3.sk_bitmap().height());
+  // Bitmap height will be cropped at 60 - kTallestTabHeight + 19.
+  EXPECT_EQ(60, rep3.sk_bitmap().height());
   // We take samples of colors and locations along the diagonal whenever
   // the color changes. Note these colors are slightly different from
   // the input PNG file due to input processing.
@@ -398,7 +401,8 @@ void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
   const gfx::ImageSkiaRep& rep4 = image_skia->GetRepresentation(2.0f);
   ASSERT_FALSE(rep4.is_null());
   EXPECT_EQ(160, rep4.sk_bitmap().width());
-  EXPECT_EQ(160, rep4.sk_bitmap().height());
+  // Cropped height will be 2 * 60.
+  EXPECT_EQ(120, rep4.sk_bitmap().height());
   // We expect the same colors and at locations scaled by 2
   // since this bitmap was scaled by 2.
   for (size_t i = 0; i < normal.size(); ++i) {

chrome/browser/ui/views/frame/opaque_browser_frame_view.cc

@@ -511,11 +511,9 @@ void OpaqueBrowserFrameView::OnPaint(gfx::Canvas* canvas) {
           : 0;
   frame_background_->set_theme_image_y_inset(y_inset);
   frame_background_->set_theme_overlay_image(GetFrameOverlayImage());
-  const int visible_image_height = GetFrameImage().height() -
-                                   ThemeProperties::kFrameHeightAboveTabs +
-                                   GetTopInset(false);
   frame_background_->set_top_area_height(
-      std::max(GetTopAreaHeight(), visible_image_height));
+      GetTopAreaHeight() - ThemeProperties::kFrameHeightAboveTabs +
+      GetTopInset(false));
 
   if (layout_->IsTitleBarCondensed())
     PaintMaximizedFrameBorder(canvas);