Remove some theme images that are not used any more.

chrome/browser/themes/browser_theme_pack.cc

Index: chrome/browser/themes/browser_theme_pack.cc
diff --git a/chrome/browser/themes/browser_theme_pack.cc b/chrome/browser/themes/browser_theme_pack.cc
index 7da11d3af2e8584edb82f89992d943339626056b..89ab1e80568b26cf4b06d95d6b0c826dd16b93be 100644
--- a/chrome/browser/themes/browser_theme_pack.cc
+++ b/chrome/browser/themes/browser_theme_pack.cc
@@ -28,7 +28,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "ui/base/resource/data_pack.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/safe_integer_conversions.h"
@@ -1239,8 +1238,6 @@ void BrowserThemePack::CropImages(ImageCache* images) const {
 }
 
 void BrowserThemePack::CreateFrameImages(ImageCache* images) const {
-  ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-
   // 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;
@@ -1248,12 +1245,13 @@ void BrowserThemePack::CreateFrameImages(ImageCache* images) const {
   for (size_t i = 0; i < arraysize(kFrameTintMap); ++i) {
     int prs_id = kFrameTintMap[i].key;
     gfx::Image frame;
-    // If there's no frame image provided for the specified id, then load
-    // the default provided frame. If that's not provided, skip this whole
-    // thing and just use the default images.
-    int prs_base_id = 0;

[Evan Stade, 2016/12/13 17:40:11]

explanation of changes to this function.

1) This was a rather pointless assignment and then check that's always true.
2) we only need/use prs_base_id if there's no image for prs_id

-
-    if (!prs_base_id) {
+    if (images->count(prs_id)) {
+      frame = (*images)[prs_id];
+    } else {
+      // If there's no frame image provided for the specified id, then load
+      // the default provided frame. If that's not provided, skip this whole
+      // thing and just use theme colors.

[Peter Kasting, 2016/12/14 04:27:54]

Nit: The last sentence isn't really about what this code does, but what some
caller does.  If it says anything, it should maybe be "If that's not provided,
the caller will fall back to using the frame colors instead."

[Evan Stade, 2016/12/15 03:31:14]

Done.

+      int prs_base_id = prs_id;
       if (prs_id == PRS_THEME_FRAME_INCOGNITO_INACTIVE) {
         prs_base_id = images->count(PRS_THEME_FRAME_INCOGNITO) ?
                       PRS_THEME_FRAME_INCOGNITO : PRS_THEME_FRAME;
@@ -1264,26 +1262,11 @@ void BrowserThemePack::CreateFrameImages(ImageCache* images) const {
       } else if (prs_id == PRS_THEME_FRAME_INCOGNITO &&
                  !images->count(PRS_THEME_FRAME_INCOGNITO)) {
         prs_base_id = PRS_THEME_FRAME;
-      } else {
-        prs_base_id = prs_id;
       }

[Peter Kasting, 2016/12/14 04:27:54]

Nit: This isn't much better (it's just as long), but it does avoid duplicating
the logic for "if there is no incognito image, fall back to the normal image",
which seems nice.

      if (prs_base_id == PRS_THEME_FRAME_OVERLAY_INACTIVE) {
        prs_base_id = PRS_THEME_FRAME_OVERLAY;
      } else if (prs_base_id == PRS_THEME_FRAME_INACTIVE) {
        prs_base_id = PRS_THEME_FRAME;
      } else {
        if (prs_base_id == PRS_THEME_FRAME_INCOGNITO_INACTIVE)
          prs_base_id = PRS_THEME_FRAME_INCOGNITO;
        if (prs_base_id == PRS_THEME_FRAME_INCOGNITO &&
            !images->count(PRS_THEME_FRAME_INCOGNITO))
          prs_base_id = PRS_THEME_FRAME;
      }

[Evan Stade, 2016/12/15 03:31:14]

I further refined this for simplicity.

+      if (prs_base_id != prs_id && images->count(prs_base_id))
+        frame = (*images)[prs_base_id];
     }
-    if (images->count(prs_id)) {
-      frame = (*images)[prs_id];
-    } else if (prs_base_id != prs_id && images->count(prs_base_id)) {
-      frame = (*images)[prs_base_id];
-    } else if (prs_base_id == PRS_THEME_FRAME_OVERLAY) {
-      if (images->count(PRS_THEME_FRAME)) {
-        // If there is no theme overlay, don't tint the default frame,
-        // because it will overwrite the custom frame image when we cache and
-        // reload from disk.
-        frame = gfx::Image();

[Evan Stade, 2016/12/13 17:40:11]

3) this was a pointless assignment. This entire branch could be rewritten

else if (prs_base_id != PRS_THEME_FRAME_OVERLAY) {
  frame = rb.GetImageNamed(IDR_THEME_FRAME);
}

4) however, even that I found pointless. I tried dozens of seemingly
frame-image-less themes and couldn't find one that didn't specify either a frame
image or a frame color (themes with solid color frames usually provide an
image). The only one I could find that was even hitting this code path was [1]
and it does fine without it, falling back to frame color.

bonus justification: for the use_ash condition, this was already using
notused.png, i.e. not doing anything, and to my knowledge we don't have any
outstanding bug reports.

[1]
https://chrome.google.com/webstore/detail/kalis-lake-meade-simple-t/lajcphnmk...

-      }
-    } else {
-      // If the theme doesn't specify an image, then apply the tint to
-      // the default frame.
-      frame = rb.GetImageNamed(IDR_THEME_FRAME);
-    }
+
     if (!frame.IsEmpty()) {
       temp_output[prs_id] = CreateHSLShiftedImage(
           frame, GetTintInternal(kFrameTintMap[i].value));