Refactor setIcon to allow for more general use of imageData

chrome/browser/extensions/api/extension_action/extension_action_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_action_api.h"
 
 #include "base/base64.h"
 #include "base/lazy_instance.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ skipping 653 matching lines @@
 bool ExtensionActionHideFunction::RunExtensionAction() {
   return SetVisible(false);
 }
 
 bool ExtensionActionSetIconFunction::RunExtensionAction() {
   EXTENSION_FUNCTION_VALIDATE(details_);
 
   // setIcon can take a variant argument: either a dictionary of canvas
   // ImageData, or an icon index.
   base::DictionaryValue* canvas_set = NULL;
-  int icon_index;
   if (details_->GetDictionary("imageData", &canvas_set)) {
     gfx::ImageSkia icon;
     // Extract icon representations from the ImageDataSet dictionary.
     for (size_t i = 0; i < arraysize(kIconSizes); i++) {
-      base::BinaryValue* binary;
-      if (canvas_set->GetBinary(kIconSizes[i].size_string, &binary)) {
-        IPC::Message pickle(binary->GetBuffer(), binary->GetSize());
+      std::string binary_string64;
+      if (canvas_set->GetString(kIconSizes[i].size_string, &binary_string64)) {
+        std::string binary_data;
+        if (!base::Base64Decode(binary_string64, &binary_data))
+          return false;
+
+        IPC::Message pickle(binary_data.c_str(), binary_data.length());

[not at google - send to devlin, 2014/08/19 00:04:43]

You should only need to do this Binary -> std::string conversion when you're
reading/writing from the database. The API can still use Binary, I hope?

[gpdavis, 2014/08/19 00:54:15]

You don't think we should keep it this way so it will be serializable once it's
expanded for use in declarativeContent, and perhaps other places?  It should
work here without conversion, if you'd prefer it that way.

[not at google - send to devlin, 2014/08/19 17:32:29]

There are 2 things here:
(1) The serialization format over IPC.
(2) The serialization format when reading/writing to disk.

They don't need to be the same.

Of course, as you know the limitation of (2) is that it must be a string, so
we're forced to base64 it. (1) doesn't have the same limitation. It's more
efficient to not do conversion and just pickle/unpickle directly into the
base::Binary format, *after which* we convert to base64.

         PickleIterator iter(pickle);
         SkBitmap bitmap;
         EXTENSION_FUNCTION_VALIDATE(IPC::ReadParam(&pickle, &iter, &bitmap));
         CHECK(!bitmap.isNull());
         float scale = ui::GetScaleForScaleFactor(kIconSizes[i].scale);
         icon.AddRepresentation(gfx::ImageSkiaRep(bitmap, scale));
       }
     }
 
     extension_action_->SetIcon(tab_id_, gfx::Image(icon));
-  } else if (details_->GetInteger("iconIndex", &icon_index)) {
+  } else if (details_->HasKey("iconIndex")) {
     // Obsolete argument: ignore it.
     return true;
   } else {
     EXTENSION_FUNCTION_VALIDATE(false);
   }
   NotifyChange();
   return true;
 }
 
 bool ExtensionActionSetTitleFunction::RunExtensionAction() {
@@ skipping 212 matching lines @@
   return true;
 }
 
 bool EnablePageActionsFunction::RunSync() {
   return SetPageActionEnabled(true);
 }
 
 bool DisablePageActionsFunction::RunSync() {
   return SetPageActionEnabled(false);
 }