Refactor setIcon to allow for more general use of imageData

extensions/renderer/set_icon_natives.cc

 // Copyright 2014 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 "extensions/renderer/set_icon_natives.h"
 
 #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"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/ipc/gfx_param_traits.h"
 
 namespace {
 
 const char* kImageSizeKeys[] = {"19", "38"};
 const char kInvalidDimensions[] = "ImageData has invalid dimensions.";
@@ skipping 64 matching lines @@
          << 16) |
         ((data->Get(v8::Integer::New(isolate, 4 * t + 1))->Int32Value() & 0xFF)
          << 8) |
         ((data->Get(v8::Integer::New(isolate, 4 * t + 2))->Int32Value() & 0xFF)
          << 0));
   }
 
   // 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;
 }
 
 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++) {
     if (!image_data_set->Has(
             v8::String::NewFromUtf8(args.GetIsolate(), kImageSizeKeys[i])))
       continue;
     v8::Local<v8::Object> image_data =
         image_data_set->Get(v8::String::NewFromUtf8(args.GetIsolate(),
                                                     kImageSizeKeys[i]))
             ->ToObject();
     base::Value* image_data_bitmap = NULL;
     if (!ConvertImageDataToBitmapValue(image_data, &image_data_bitmap))
       return false;
     bitmap_set_value->Set(kImageSizeKeys[i], image_data_bitmap);
   }
   return true;
 }
 
 void SetIconNatives::SetIconCommon(
     const v8::FunctionCallbackInfo<v8::Value>& args) {

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

CHECK_EQ(1, args.Length());
CHECK(args[0]->IsObject());

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

Done.

   scoped_ptr<base::DictionaryValue> bitmap_set_value(
       new base::DictionaryValue());
   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",
         details->Get(v8::String::NewFromUtf8(args.GetIsolate(), "tabId"))
             ->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