Update browserAction.setIcon and pageAction.setIcon for hidpi

chrome/browser/extensions/api/extension_action/extension_actions_api.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/api/extension_action/extension_actions_api.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/string_number_conversions.h"
@@ skipping 26 matching lines @@
 const char kBadgeBackgroundColorStorageKey[] = "badge_background_color";
 const char kBadgeTextColorStorageKey[] = "badge_text_color";
 const char kAppearanceStorageKey[] = "appearance";
 
 // Errors.
 const char kNoExtensionActionError[] =
     "This extension has no action specified.";
 const char kNoTabError[] = "No tab with id: *.";
 const char kIconIndexOutOfBounds[] = "Page action icon index out of bounds.";
 
+struct IconRepresentationInfo {
+  // Size as a string that will be used to retrieve represantation value from

[Jeffrey Yasskin, 2012/08/17 23:20:28]

sp: represantation -> representation

[tbarzic, 2012/08/21 00:22:07]

Done.

+  // 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 }
+};
+
 // Conversion function for reading/writing to storage.
 SkColor RawStringToSkColor(const std::string& str) {
   uint64 value = 0;
   base::StringToUint64(str, &value);
   SkColor color = static_cast<SkColor>(value);
   DCHECK(value == color);  // ensure value fits into color's 32 bits
   return color;
 }
 
 // Conversion function for reading/writing to storage.
 std::string SkColorToRawString(SkColor color) {
   return base::Uint64ToString(color);
 }
 
 // Conversion function for reading/writing to storage.
 bool StringToSkBitmap(const std::string& str, SkBitmap* bitmap) {
   // TODO(mpcomplete): Remove the base64 encode/decode step when
   // http://crbug.com/140546 is fixed.
   std::string raw_str;
   if (!base::Base64Decode(str, &raw_str))
     return false;
   IPC::Message bitmap_pickle(raw_str.data(), raw_str.size());
   PickleIterator iter(bitmap_pickle);
   return IPC::ReadParam(&bitmap_pickle, &iter, bitmap);
 }
 
 // Conversion function for reading/writing to storage.
-std::string ImageToString(const gfx::Image& image) {
+std::string RepresentationToString(const gfx::ImageSkia& image,
+                                   ui::ScaleFactor scale) {
+  SkBitmap bitmap = image.GetRepresentation(scale).sk_bitmap();
   IPC::Message bitmap_pickle;
-  IPC::WriteParam(&bitmap_pickle, image.AsBitmap());
+  IPC::WriteParam(&bitmap_pickle, bitmap);
   std::string raw_str(static_cast<const char*>(bitmap_pickle.data()),
                       bitmap_pickle.size());
   std::string base64_str;
   if (!base::Base64Encode(raw_str, &base64_str))
     return std::string();
   return base64_str;
 }
 
 // Set |action|'s default values to those specified in |dict|.
-void SetDefaultsFromValue(const base::DictionaryValue* dict,
+void SetDefaultsFromValue(base::DictionaryValue* dict,

[Jeffrey Yasskin, 2012/08/17 23:20:28]

Ew @ modifying the dict. I think you can avoid this by making icon_value a const
base::DictionaryValue*?

[tbarzic, 2012/08/21 00:22:07]

Done.

                           ExtensionAction* action) {
   const int kTabId = ExtensionAction::kDefaultTabId;
   std::string str_value;
   int int_value;
   SkBitmap bitmap;
+  gfx::ImageSkia icon;
 
   if (dict->GetString(kPopupUrlStorageKey, &str_value))
     action->SetPopupUrl(kTabId, GURL(str_value));
   if (dict->GetString(kTitleStorageKey, &str_value))
     action->SetTitle(kTabId, str_value);
   if (dict->GetString(kBadgeTextStorageKey, &str_value))
     action->SetBadgeText(kTabId, str_value);
   if (dict->GetString(kBadgeBackgroundColorStorageKey, &str_value))
     action->SetBadgeBackgroundColor(kTabId, RawStringToSkColor(str_value));
   if (dict->GetString(kBadgeTextColorStorageKey, &str_value))
     action->SetBadgeTextColor(kTabId, RawStringToSkColor(str_value));
   if (dict->GetInteger(kAppearanceStorageKey, &int_value))
     action->SetAppearance(kTabId,
                           static_cast<ExtensionAction::Appearance>(int_value));
-  if (dict->GetString(kIconStorageKey, &str_value) &&
-      StringToSkBitmap(str_value, &bitmap)) {
-    CHECK(!bitmap.isNull());
-    action->SetIcon(kTabId, gfx::Image(bitmap));
+
+  base::DictionaryValue* icon_value = NULL;
+  if (dict->GetDictionary(kIconStorageKey, &icon_value)) {
+    for (size_t i = 0; i < arraysize(kIconSizes); i++) {
+      if (icon_value->GetString(kIconSizes[i].size_string, &str_value) &&
+          StringToSkBitmap(str_value, &bitmap)) {
+        CHECK(!bitmap.isNull());
+        icon.AddRepresentation(gfx::ImageSkiaRep(bitmap, kIconSizes[i].scale));
+      }
+    }
+    action->SetIcon(kTabId, gfx::Image(icon));
   }
 }
 
 // Store |action|'s default values in a DictionaryValue for use in storing to
 // disk.
 scoped_ptr<base::DictionaryValue> DefaultsToValue(ExtensionAction* action) {
   const int kTabId = ExtensionAction::kDefaultTabId;
   scoped_ptr<base::DictionaryValue> dict(new DictionaryValue());
 
   dict->SetString(kPopupUrlStorageKey, action->GetPopupUrl(kTabId).spec());
   dict->SetString(kTitleStorageKey, action->GetTitle(kTabId));
   dict->SetString(kBadgeTextStorageKey, action->GetBadgeText(kTabId));
   dict->SetString(kBadgeBackgroundColorStorageKey,
                   SkColorToRawString(action->GetBadgeBackgroundColor(kTabId)));
   dict->SetString(kBadgeTextColorStorageKey,
                   SkColorToRawString(action->GetBadgeTextColor(kTabId)));
   dict->SetInteger(kAppearanceStorageKey,
                    action->GetIsVisible(kTabId) ?
                        ExtensionAction::ACTIVE : ExtensionAction::INVISIBLE);
-  dict->SetString(kIconStorageKey, ImageToString(action->GetIcon(kTabId)));
 
+  gfx::ImageSkia icon = action->GetExplicitlySetIcon(kTabId);
+  if (!icon.empty()) {
+    base::DictionaryValue* icon_value = new base::DictionaryValue();
+    for (size_t i = 0; i < arraysize(kIconSizes); i++) {
+      if (icon.HasRepresentation(kIconSizes[i].scale)) {
+        icon_value->SetString(
+            kIconSizes[i].size_string,
+            RepresentationToString(icon, kIconSizes[i].scale));
+      }
+    }
+    dict->Set(kIconStorageKey, icon_value);
+  }
   return dict.Pass();
 }
 
+// Updates |icon| with a new image representation for |scale_factor|. The
+// representation is found in dictionary |representations| under |size_key|. If
+// there is no image representation under desired key, |icon| remains unchanged.
+bool UpdateIconWithRepresentation(const char* size_key,

[Jeffrey Yasskin, 2012/08/17 23:20:28]

This function doesn't seem to be pulling its weight. The behavior of both
pulling a sized image out of a dictionary and adding an ImageSkiaRep don't feel
like they logically belong together, and the function's only called once, so
you'd actually save code lines by inlining it.

[tbarzic, 2012/08/21 00:22:07]

Done.

+                                  base::DictionaryValue* representations,
+                                  ui::ScaleFactor scale_factor,
+                                  gfx::ImageSkia* icon) {
+  base::BinaryValue* binary;
+  if (representations->GetBinary(size_key, &binary)) {
+    IPC::Message bitmap_pickle(binary->GetBuffer(), binary->GetSize());
+    PickleIterator iter(bitmap_pickle);
+    SkBitmap bitmap;
+    if (!IPC::ReadParam(&bitmap_pickle, &iter, &bitmap))
+      return false;
+    CHECK(!bitmap.isNull());
+    icon->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
+  }
+  return true;
+}
+
 }  // namespace
 
 namespace extensions {
 
 //
 // ExtensionActionStorageManager
 //
 
 ExtensionActionStorageManager::ExtensionActionStorageManager(Profile* profile)
     : profile_(profile) {
@@ skipping 251 matching lines @@
 
 bool ExtensionActionShowFunction::RunExtensionAction() {
   return SetVisible(true);
 }
 
 bool ExtensionActionHideFunction::RunExtensionAction() {
   return SetVisible(false);
 }
 
 bool ExtensionActionSetIconFunction::RunExtensionAction() {
-  // setIcon can take a variant argument: either a canvas ImageData, or an
-  // icon index.
-  base::BinaryValue* binary = NULL;
+  // setIcon can take a variant argument: either a dictionary of canvases
+  // ImageDataSet, or an icon index.
   int icon_index;
-  if (details_->GetBinary("imageData", &binary)) {
-    IPC::Message bitmap_pickle(binary->GetBuffer(), binary->GetSize());
-    PickleIterator iter(bitmap_pickle);
-    SkBitmap bitmap;
-    EXTENSION_FUNCTION_VALIDATE(IPC::ReadParam(&bitmap_pickle, &iter, &bitmap));
-    CHECK(!bitmap.isNull());
-    extension_action_->SetIcon(tab_id_, gfx::Image(bitmap));
+  base::DictionaryValue* canvas_set = NULL;
+  if (details_->GetDictionary("imageDataSet", &canvas_set)) {
+    gfx::ImageSkia icon;
+    for (size_t i = 0; i < arraysize(kIconSizes); i++) {
+      EXTENSION_FUNCTION_VALIDATE(UpdateIconWithRepresentation(
+          kIconSizes[i].size_string, canvas_set, kIconSizes[i].scale, &icon));
+    }
+    extension_action_->SetIcon(tab_id_, gfx::Image(icon));
   } else if (details_->GetInteger("iconIndex", &icon_index)) {
     // If --enable-script-badges is on there might legitimately be an iconIndex
     // set. Until we decide what to do with that, ignore.
     if (!GetExtension()->page_action())
       return true;
     if (icon_index < 0 ||
         static_cast<size_t>(icon_index) >=
             extension_action_->icon_paths()->size()) {
       error_ = kIconIndexOutOfBounds;
       return false;
@@ skipping 83 matching lines @@
 bool ExtensionActionGetBadgeBackgroundColorFunction::RunExtensionAction() {
   ListValue* list = new ListValue();
   SkColor color = extension_action_->GetBadgeBackgroundColor(tab_id_);
   list->Append(Value::CreateIntegerValue(SkColorGetR(color)));
   list->Append(Value::CreateIntegerValue(SkColorGetG(color)));
   list->Append(Value::CreateIntegerValue(SkColorGetB(color)));
   list->Append(Value::CreateIntegerValue(SkColorGetA(color)));
   SetResult(list);
   return true;
 }