Themes: Remove unused IDs from ThemeProperties

chrome/browser/themes/browser_theme_pack.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/themes/browser_theme_pack.h"
 
 #include <limits.h>
 #include <stddef.h>
 
 #include <limits>
@@ skipping 30 matching lines @@
 
 using content::BrowserThread;
 using extensions::Extension;
 
 namespace {
 
 // 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 = 45;

[Elliot Glaysher, 2017/01/31 21:35:34]

Does this change need the theme pack version bumped?

[Tom (Use chromium acct), 2017/02/01 00:21:17]

idk, but I did it anyway

 
 // 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
 // "header" to be written last. That way we can detect whether the pack was
 // successfully written and ignore and regenerate if it was only partially
 // written (i.e. chrome crashed on a different thread while writing the pack).
 const int kMaxID = 0x0000FFFF;  // Max unsigned 16-bit int.
 const int kHeaderID = kMaxID - 1;
 const int kTintsID = kMaxID - 2;
 const int kColorsID = kMaxID - 3;
@@ skipping 166 matching lines @@
   { "frame_incognito", ThemeProperties::COLOR_FRAME_INCOGNITO },
   { "frame_incognito_inactive",
     ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE },
   { "toolbar", ThemeProperties::COLOR_TOOLBAR },
   { "tab_text", ThemeProperties::COLOR_TAB_TEXT },
   { "tab_background_text", ThemeProperties::COLOR_BACKGROUND_TAB_TEXT },
   { "bookmark_text", ThemeProperties::COLOR_BOOKMARK_TEXT },
   { "ntp_background", ThemeProperties::COLOR_NTP_BACKGROUND },
   { "ntp_text", ThemeProperties::COLOR_NTP_TEXT },
   { "ntp_link", ThemeProperties::COLOR_NTP_LINK },
-  { "ntp_link_underline", ThemeProperties::COLOR_NTP_LINK_UNDERLINE },
   { "ntp_header", ThemeProperties::COLOR_NTP_HEADER },
   { "ntp_section", ThemeProperties::COLOR_NTP_SECTION },
-  { "ntp_section_text", ThemeProperties::COLOR_NTP_SECTION_TEXT },
-  { "ntp_section_link", ThemeProperties::COLOR_NTP_SECTION_LINK },
-  { "ntp_section_link_underline",
-    ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE },
   { "button_background", ThemeProperties::COLOR_BUTTON_BACKGROUND },
 };
 const size_t kColorTableLength = arraysize(kColorTable);
 
 // Strings used by themes to identify display properties keys in JSON.
 const StringToIntTable kDisplayProperties[] = {
   { "ntp_background_alignment",
     ThemeProperties::NTP_BACKGROUND_ALIGNMENT },
   { "ntp_background_repeat", ThemeProperties::NTP_BACKGROUND_TILING },
   { "ntp_logo_alternate", ThemeProperties::NTP_LOGO_ALTERNATE },
@@ skipping 709 matching lines @@
           } else {
             // Invalid entry for part 4.
             continue;
           }
         } else {
           color = SkColorSetRGB(r, g, b);
         }
 
         int id = GetIntForString(iter.key(), kColorTable, kColorTableLength);
         if (id != -1) {
           (*temp_colors)[id] = color;

[Tom (Use chromium acct), 2017/01/31 21:29:27]

We could remove COLOR_NTP_SECTION too if we add a special case here.

The only thing it's used for is to provide a fallback for COLOR_NTP_HEADER when
the theme doesn't provide one

[Evan Stade, 2017/01/31 23:51:54]

I guess I would be in favor of that. We have to special case below anyway.

[Tom (Use chromium acct), 2017/02/01 00:21:17]

Done.

         }
       }
     }
   }
 }
 
 void BrowserThemePack::GenerateMissingColors(
     std::map<int, SkColor>* colors) {
-  // Generate link colors, if missing. (See GetColor()).
   if (!colors->count(ThemeProperties::COLOR_NTP_HEADER) &&
       colors->count(ThemeProperties::COLOR_NTP_SECTION)) {
     (*colors)[ThemeProperties::COLOR_NTP_HEADER] =
         (*colors)[ThemeProperties::COLOR_NTP_SECTION];
   }
 
-  if (!colors->count(ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE) &&
-      colors->count(ThemeProperties::COLOR_NTP_SECTION_LINK)) {
-    SkColor color_section_link =
-        (*colors)[ThemeProperties::COLOR_NTP_SECTION_LINK];
-    (*colors)[ThemeProperties::COLOR_NTP_SECTION_LINK_UNDERLINE] =
-        SkColorSetA(color_section_link, SkColorGetA(color_section_link) / 3);
-  }
-
-  if (!colors->count(ThemeProperties::COLOR_NTP_LINK_UNDERLINE) &&
-      colors->count(ThemeProperties::COLOR_NTP_LINK)) {
-    SkColor color_link = (*colors)[ThemeProperties::COLOR_NTP_LINK];
-    (*colors)[ThemeProperties::COLOR_NTP_LINK_UNDERLINE] =
-        SkColorSetA(color_link, SkColorGetA(color_link) / 3);
-  }
-
   // 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);
   }
@@ skipping 437 matching lines @@
                                            false,
                                            &bitmap_data)) {
       NOTREACHED() << "Unable to encode theme image for prs_id="
                    << prs_id << " for scale_factor=" << scale_factors_[i];
       break;
     }
     image_memory_[scaled_raw_id] =
         base::RefCountedBytes::TakeVector(&bitmap_data);
   }
 }