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/memory/scoped_ptr.h"
 #include "content/public/common/common_param_traits.h"
 #include "extensions/renderer/request_sender.h"
 #include "extensions/renderer/script_context.h"
 #include "ipc/ipc_message_utils.h"
+#include "third_party/WebKit/public/web/WebArrayBufferConverter.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.";
 const char kInvalidData[] = "ImageData data length does not match dimensions.";
 const char kNoMemory[] = "Chrome was unable to initialize icon.";
 
 }  // namespace
 
 namespace extensions {
 
 SetIconNatives::SetIconNatives(RequestSender* request_sender,
                                ScriptContext* context)
     : ObjectBackedNativeHandler(context), request_sender_(request_sender) {
   RouteFunction(
       "SetIconCommon",
       base::Bind(&SetIconNatives::SetIconCommon, base::Unretained(this)));
 }
 
 bool SetIconNatives::ConvertImageDataToBitmapValue(
     const v8::Local<v8::Object> image_data,
-    base::Value** bitmap_value) {
+    v8::Local<v8::Value>* image_data_bitmap) {
   v8::Isolate* isolate = context()->v8_context()->GetIsolate();
   v8::Local<v8::Object> data =
       image_data->Get(v8::String::NewFromUtf8(isolate, "data"))->ToObject();
   int width =
       image_data->Get(v8::String::NewFromUtf8(isolate, "width"))->Int32Value();
   int height =
       image_data->Get(v8::String::NewFromUtf8(isolate, "height"))->Int32Value();
 
   if (width <= 0 || height <= 0) {
     isolate->ThrowException(v8::Exception::Error(
@@ skipping 36 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());
+  blink::WebArrayBuffer buffer =
+      blink::WebArrayBuffer::create(bitmap_pickle.size(), 1);
+  memcpy(buffer.data(), bitmap_pickle.data(), bitmap_pickle.size());
+  *image_data_bitmap = blink::WebArrayBufferConverter::toV8Value(
+      &buffer, context()->v8_context()->Global(), isolate);
 
   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>* bitmap_set_value) {
+  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;
+    v8::Local<v8::Value> image_data_bitmap;
     if (!ConvertImageDataToBitmapValue(image_data, &image_data_bitmap))
       return false;
-    bitmap_set_value->Set(kImageSizeKeys[i], image_data_bitmap);
+    (*bitmap_set_value)->Set(
+        v8::String::NewFromUtf8(args.GetIsolate(), kImageSizeKeys[i]),
+        image_data_bitmap);
   }
   return true;
 }
 
 void SetIconNatives::SetIconCommon(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
-  scoped_ptr<base::DictionaryValue> bitmap_set_value(
-      new base::DictionaryValue());
-  if (!ConvertImageDataSetToBitmapValueSet(args, bitmap_set_value.get()))
+  CHECK_EQ(1, args.Length());
+  CHECK(args[0]->IsObject());
+  v8::Local<v8::Object> bitmap_set_value(v8::Object::New(args.GetIsolate()));
+  if (!ConvertImageDataSetToBitmapValueSet(args, &bitmap_set_value))

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

All ConvertImageDataSetToBitmapValueSet does to |args| is
1- extract args[0]
2- get the isolate

1 is repeating work, because we grab args[0] here right after (line 140). So you
should get the |details| first, then pass just that to
ConvertImageDataSetToBitmapValueSet (of course you'll need to change the
Arguments value to a const v8::Handle<v8::Object>&).

It's better to do that anyway, it's a bit odd to validate the arguments to the
function then ignore it. You should assign |details| straight after the CHECK.

2, well, we have the isolate here, it's context()->isolate() (yes they're the
same isolate), so you can just call that directly from
ConvertImageDataSetToBitmapValueSet. No need to pass it in.

[gpdavis, 2014/08/20 17:58:18]

Ah, I wasn't sure if these were the same isolate and I figured there was a
reason the args were passed in (now that I look at it again, though, args was
probably passed because more information was extracted in
ConvertImageDataSetToBitmapValueSet before).

Done.

     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();
-  dict->Set("imageData", bitmap_set_value.release());
-
+  v8::Local<v8::Object> dict(v8::Object::New(args.GetIsolate()));
+  dict->Set(v8::String::NewFromUtf8(args.GetIsolate(), "imageData"),
+            bitmap_set_value);
   if (details->Has(v8::String::NewFromUtf8(args.GetIsolate(), "tabId"))) {
-    dict->SetInteger(
-        "tabId",
-        details->Get(v8::String::NewFromUtf8(args.GetIsolate(), "tabId"))
-            ->Int32Value());
+    dict->Set(
+        v8::String::NewFromUtf8(args.GetIsolate(), "tabId"),
+        details->Get(v8::String::NewFromUtf8(args.GetIsolate(), "tabId")));
   }
-
-  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);
+  args.GetReturnValue().Set(dict);
 }
 
 }  // namespace extensions