Refactor setIcon to allow for more general use of imageData

extensions/renderer/set_icon_natives.cc

Index: extensions/renderer/set_icon_natives.cc
diff --git a/extensions/renderer/set_icon_natives.cc b/extensions/renderer/set_icon_natives.cc
index 192112f1d812215e823bbf4929e716d412b3ebed..353aa2e1ea1ff336c8da55d7b4fe651ccb03de59 100644
--- a/extensions/renderer/set_icon_natives.cc
+++ b/extensions/renderer/set_icon_natives.cc
@@ -6,8 +6,10 @@
 
 #include <limits>
 
+#include "base/base64.h"
 #include "base/memory/scoped_ptr.h"
 #include "content/public/common/common_param_traits.h"
+#include "content/public/renderer/v8_value_converter.h"
 #include "extensions/renderer/request_sender.h"
 #include "extensions/renderer/script_context.h"
 #include "ipc/ipc_message_utils.h"
@@ -92,8 +94,11 @@ bool SetIconNatives::ConvertImageDataToBitmapValue(
   // Construct the Value object.
   IPC::Message bitmap_pickle;
   IPC::WriteParam(&bitmap_pickle, bitmap);
-  *bitmap_value = base::BinaryValue::CreateWithCopiedBuffer(
-      static_cast<const char*>(bitmap_pickle.data()), bitmap_pickle.size());
+  std::string bitmap_data = std::string(reinterpret_cast<const char*>(
+      bitmap_pickle.data()), bitmap_pickle.size());
+  std::string data64;
+  base::Base64Encode(bitmap_data, &data64);

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

Ideally you'd only need to convert to a binary value that's understood by the
V8ValueConverter already, I.e., rather than going:

ImageData -> base::BinaryValue -> V8ValueConverter::ToV8Value -> ArrayBuffer ->
JavaScript -> V8ValueConverter::FromV8Value -> base::BinaryValue

just:

ImageData -> ArrayBuffer -> JavaScript -> V8ValueConverter::FromV8Value ->
base::BinaryValue

i.e. cut out one of the base::BinaryValue conversion steps. Check out how
V8ValueConverterImpl does it:

https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/v...

[gpdavis, 2014/08/19 22:05:07]

Well, that took a while, but I finally figured out how to work with these V8
values.  Done.

[not at google - send to devlin, 2014/08/20 14:42:18]

Nice!

+  *bitmap_value = new base::StringValue(data64);
 
   return true;
 }
@@ -101,13 +106,10 @@ bool SetIconNatives::ConvertImageDataToBitmapValue(
 bool SetIconNatives::ConvertImageDataSetToBitmapValueSet(
     const v8::FunctionCallbackInfo<v8::Value>& args,
     base::DictionaryValue* bitmap_set_value) {
-  v8::Local<v8::Object> extension_args = args[1]->ToObject();
-  v8::Local<v8::Object> details =
-      extension_args->Get(v8::String::NewFromUtf8(args.GetIsolate(), "0"))
-          ->ToObject();
+  v8::Local<v8::Object> details = args[0]->ToObject();
   v8::Local<v8::Object> image_data_set =
-      details->Get(v8::String::NewFromUtf8(args.GetIsolate(), "imageData"))
-          ->ToObject();
+      details->Get(v8::String::NewFromUtf8(
+      args.GetIsolate(), "imageData"))->ToObject();
 
   DCHECK(bitmap_set_value);
   for (size_t i = 0; i < arraysize(kImageSizeKeys); i++) {
@@ -133,14 +135,10 @@ void SetIconNatives::SetIconCommon(
   if (!ConvertImageDataSetToBitmapValueSet(args, bitmap_set_value.get()))
     return;
 
-  v8::Local<v8::Object> extension_args = args[1]->ToObject();
-  v8::Local<v8::Object> details =
-      extension_args->Get(v8::String::NewFromUtf8(args.GetIsolate(), "0"))
-          ->ToObject();
+  v8::Local<v8::Object> details = args[0]->ToObject();
 
   base::DictionaryValue* dict = new base::DictionaryValue();

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

This now leaks |dict| since it's not having its ownership transferred to
|list_value|.

Just declare |dict| on the stack now.

[gpdavis, 2014/08/19 22:05:07]

Ah, gotcha.  Nice catch!

   dict->Set("imageData", bitmap_set_value.release());
-
   if (details->Has(v8::String::NewFromUtf8(args.GetIsolate(), "tabId"))) {
     dict->SetInteger(
         "tabId",
@@ -148,16 +146,12 @@ void SetIconNatives::SetIconCommon(
             ->Int32Value());
   }
 
-  base::ListValue list_value;
-  list_value.Append(dict);
-
-  std::string name = *v8::String::Utf8Value(args[0]);
-  int request_id = args[2]->Int32Value();
-  bool has_callback = args[3]->BooleanValue();
-  bool for_io_thread = args[4]->BooleanValue();
-
-  request_sender_->StartRequest(
-      context(), name, request_id, has_callback, for_io_thread, &list_value);
+  v8::Isolate* isolate = context()->v8_context()->GetIsolate();
+  v8::Handle<v8::Value> result = v8::Object::New(isolate);
+  scoped_ptr<content::V8ValueConverter> converter(
+      content::V8ValueConverter::create());
+  result = converter->ToV8Value(dict, context()->v8_context());
+  args.GetReturnValue().Set(result);
 }
 
 }  // namespace extensions