Handle more scale factors for extension Browser Action icons

chrome/browser/extensions/extension_action.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/extensions/extension_action.h"
 
 #include <algorithm>
 
 #include "base/base64.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 #include "extensions/browser/extension_icon_image.h"
 #include "extensions/browser/extension_icon_placeholder.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_icon_set.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "grit/theme_resources.h"
 #include "grit/ui_resources.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_utils.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
+#include "ui/base/resource/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/animation/animation_delegate.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_source.h"
 #include "ui/gfx/ipc/gfx_param_traits.h"
 #include "ui/gfx/skbitmap_operations.h"
 #include "url/gurl.h"
 
 namespace {
 
 // Returns the default icon image for extensions.
 gfx::Image GetDefaultIcon() {
   return ui::ResourceBundle::GetSharedInstance().GetImageNamed(
       IDR_EXTENSIONS_FAVICON);
 }
 
-// Given the extension action type, returns the size the extension action icon
-// should have. The icon should be square, so only one dimension is
-// returned.
-int GetIconSizeForType(extensions::ActionInfo::Type type) {
-  switch (type) {
-    case extensions::ActionInfo::TYPE_BROWSER:
-    case extensions::ActionInfo::TYPE_PAGE:
-    case extensions::ActionInfo::TYPE_SYSTEM_INDICATOR:
-      // TODO(dewittj) Report the actual icon size of the system
-      // indicator.
-      return extension_misc::EXTENSION_ICON_ACTION;
-    default:
-      NOTREACHED();
-      return 0;
-  }
-}
-
 class GetAttentionImageSource : public gfx::ImageSkiaSource {
  public:
   explicit GetAttentionImageSource(const gfx::ImageSkia& icon)
       : icon_(icon) {}
 
   // gfx::ImageSkiaSource overrides:
   gfx::ImageSkiaRep GetImageForScale(float scale) override {
     gfx::ImageSkiaRep icon_rep = icon_.GetRepresentation(scale);
     color_utils::HSL shift = {-1, 0, 0.5};
     return gfx::ImageSkiaRep(
         SkBitmapOperations::CreateHSLShiftedBitmap(icon_rep.sk_bitmap(), shift),
         icon_rep.scale());
   }
 
  private:
   const gfx::ImageSkia icon_;
 };
 
 struct IconRepresentationInfo {
   // Size as a string that will be used to retrieve a representation value from
   // SetIcon function arguments.
   const char* size_string;
   // Scale factor for which the represantion should be used.
   ui::ScaleFactor scale;
 };
 
-const IconRepresentationInfo kIconSizes[] = {{"19", ui::SCALE_FACTOR_100P},
-                                             {"38", ui::SCALE_FACTOR_200P}};
-
 template <class T>
 bool HasValue(const std::map<int, T>& map, int tab_id) {
   return map.find(tab_id) != map.end();
 }
 
 }  // namespace
 
+extension_misc::ExtensionIcons ExtensionAction::ActionIconSize() {
+  return ui::MaterialDesignController::IsModeMaterial()
+             ? extension_misc::EXTENSION_ICON_BITTY
+             : extension_misc::EXTENSION_ICON_ACTION;
+}
+
 const int ExtensionAction::kDefaultTabId = -1;
-const int ExtensionAction::kPageActionIconMaxSize =
-    extension_misc::EXTENSION_ICON_ACTION;
 
 ExtensionAction::ExtensionAction(const extensions::Extension& extension,
                                  extensions::ActionInfo::Type action_type,
                                  const extensions::ActionInfo& manifest_data)
     : extension_id_(extension.id()),
       extension_name_(extension.name()),
       action_type_(action_type) {
   // Page/script actions are hidden/disabled by default, and browser actions are
   // visible/enabled by default.
   SetIsVisible(kDefaultTabId,
@@ skipping 21 matching lines @@
   return GetValue(&popup_url_, tab_id);
 }
 
 void ExtensionAction::SetIcon(int tab_id, const gfx::Image& image) {
   SetValue(&icon_, tab_id, image);
 }
 
 bool ExtensionAction::ParseIconFromCanvasDictionary(
     const base::DictionaryValue& dict,
     gfx::ImageSkia* icon) {
-  // Try to extract an icon for each known scale.
-  for (size_t i = 0; i < arraysize(kIconSizes); i++) {
+  for (base::DictionaryValue::Iterator iter(dict); !iter.IsAtEnd();
+       iter.Advance()) {
+    int icon_size = 0;
+    if (!base::StringToInt(iter.key(), &icon_size))
+      continue;
+
     const base::BinaryValue* image_data;
     std::string binary_string64;
     IPC::Message pickle;
-    if (dict.GetBinary(kIconSizes[i].size_string, &image_data)) {
+    if (iter.value().GetAsBinary(&image_data)) {
       pickle = IPC::Message(image_data->GetBuffer(), image_data->GetSize());
-    } else if (dict.GetString(kIconSizes[i].size_string, &binary_string64)) {
+    } else if (iter.value().GetAsString(&binary_string64)) {
       std::string binary_string;
       if (!base::Base64Decode(binary_string64, &binary_string))
         return false;
       pickle = IPC::Message(binary_string.c_str(), binary_string.length());
     } else {
       continue;
     }
-    base::PickleIterator iter(pickle);
+    base::PickleIterator pickle_iter(pickle);
     SkBitmap bitmap;
-    if (!IPC::ReadParam(&pickle, &iter, &bitmap))
+    if (!IPC::ReadParam(&pickle, &pickle_iter, &bitmap))
       return false;
     CHECK(!bitmap.isNull());
-    float scale = ui::GetScaleForScaleFactor(kIconSizes[i].scale);
+    float scale =
+        static_cast<float>(ExtensionAction::ActionIconSize()) / icon_size;
     icon->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale));
   }
   return true;
 }
 
 gfx::Image ExtensionAction::GetExplicitlySetIcon(int tab_id) const {
   return GetValue(&icon_, tab_id);
 }
 
 bool ExtensionAction::SetIsVisible(int tab_id, bool new_visibility) {
@@ skipping 59 matching lines @@
   // when the tab's closed.  There's a race between the
   // LocationBarController and the ContentRulesRegistry on navigation,
   // which prevents me from cleaning everything up now.
 }
 
 extensions::IconImage* ExtensionAction::LoadDefaultIconImage(
     const extensions::Extension& extension,
     content::BrowserContext* browser_context) {
   if (default_icon_ && !default_icon_image_) {
     default_icon_image_.reset(new extensions::IconImage(
-        browser_context,
-        &extension,
-        *default_icon(),
-        GetIconSizeForType(action_type_),
-        *GetDefaultIcon().ToImageSkia(),
-        nullptr));
+        browser_context, &extension, *default_icon(), ActionIconSize(),
+        *GetDefaultIcon().ToImageSkia(), nullptr));
   }
   return default_icon_image_.get();
 }
 
 gfx::Image ExtensionAction::GetDefaultIconImage() const {
   // If we have a default icon, it should be loaded before trying to use it.
   DCHECK(!default_icon_image_ == !default_icon_);
   if (default_icon_image_)
     return default_icon_image_->image();
 
   if (placeholder_icon_image_.IsEmpty()) {
     // If the extension action redesign is enabled, we use a special placeholder
     // icon (with the first letter of the extension name) rather than the
     // default (puzzle piece).
     if (extensions::FeatureSwitch::extension_action_redesign()->IsEnabled()) {
       placeholder_icon_image_ =
-          extensions::ExtensionIconPlaceholder::CreateImage(
-              extension_misc::EXTENSION_ICON_ACTION, extension_name_);
+          extensions::ExtensionIconPlaceholder::CreateImage(ActionIconSize(),
+                                                            extension_name_);
     } else {
       placeholder_icon_image_ = GetDefaultIcon();
     }
   }
 
   return placeholder_icon_image_;
 }
 
 bool ExtensionAction::HasPopupUrl(int tab_id) const {
   return HasValue(popup_url_, tab_id);
@@ skipping 32 matching lines @@
                                const extensions::ActionInfo& manifest_data) {
   // If the manifest doesn't specify a title, set it to |extension|'s name.
   const std::string& title =
       !manifest_data.default_title.empty() ? manifest_data.default_title :
       extension.name();
   SetTitle(kDefaultTabId, title);
   SetPopupUrl(kDefaultTabId, manifest_data.default_popup_url);
   set_id(manifest_data.id);
 
   // Initialize the specified icon set.
-  if (!manifest_data.default_icon.empty())
+  if (!manifest_data.default_icon.empty()) {
     default_icon_.reset(new ExtensionIconSet(manifest_data.default_icon));
-
-  const ExtensionIconSet& extension_icons =
-      extensions::IconsInfo::GetIcons(&extension);
-  // Look for any other icons.
-  std::string largest_icon = extension_icons.Get(
-      extension_misc::EXTENSION_ICON_GIGANTOR, ExtensionIconSet::MATCH_SMALLER);
-
-  if (!largest_icon.empty()) {
-    // We found an icon to use, so create an icon set if one doesn't exist.
-    if (!default_icon_)
-      default_icon_.reset(new ExtensionIconSet());
-    int largest_icon_size = extension_icons.GetIconSizeFromPath(largest_icon);
-    // Replace any missing extension action icons with the largest icon
-    // retrieved from |extension|'s manifest so long as the largest icon is
-    // larger than the current key.
-    for (int i = extension_misc::kNumExtensionActionIconSizes - 1; i >= 0;
-         --i) {
-      int size = extension_misc::kExtensionActionIconSizes[i].size;
-      if (default_icon_->Get(size, ExtensionIconSet::MATCH_BIGGER).empty() &&
-          largest_icon_size > size) {
-        default_icon_->Add(size, largest_icon);
-        break;
-      }
-    }
+  } else {
+    // Fall back to the product icons if no action icon exists.

[Evan Stade, 2015/12/02 23:25:32]

this is a behavioral change (and for the better, IMO). Before if you specified
19 but not 38, the 38 would get filled by a product icon. This means that for 2x
scale you have a different browser action icon than 1x scale (instead of a
blown-up version of 1x). Now you'd only get the product icon here if you have no
browser action icons specified at any size whatsoever.

[Devlin, 2015/12/03 00:00:37]

+1

+    const ExtensionIconSet& product_icons =
+        extensions::IconsInfo::GetIcons(&extension);
+    if (!product_icons.empty())
+      default_icon_.reset(new ExtensionIconSet(product_icons));
   }
 }
 
 // Determines which icon would be returned by |GetIcon|, and returns its width.
 int ExtensionAction::GetIconWidth(int tab_id) const {
   // If icon has been set, return its width.
   gfx::Image icon = GetValue(&icon_, tab_id);
   if (!icon.IsEmpty())
     return icon.Width();
   // If there is a default icon, the icon width will be set depending on our
   // action type.
   if (default_icon_)
-    return GetIconSizeForType(action_type());
+    return ActionIconSize();
 
   // If no icon has been set and there is no default icon, we need favicon
   // width.
   return ui::ResourceBundle::GetSharedInstance().GetImageNamed(
           IDR_EXTENSIONS_FAVICON).Width();
 }