Allow serialization of ArrayBuffer params in extension/apps API methods

content/renderer/v8_value_converter_impl.cc

Index: content/renderer/v8_value_converter_impl.cc
diff --git a/content/renderer/v8_value_converter_impl.cc b/content/renderer/v8_value_converter_impl.cc
index b0762ef11f1db00778796f391aa0f4163fbf5a22..ec5217f9166d339771024dd1bdbf4b97ed8d5e18 100644
--- a/content/renderer/v8_value_converter_impl.cc
+++ b/content/renderer/v8_value_converter_impl.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -9,8 +9,17 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/values.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebArrayBuffer.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebArrayBufferView.h"
 #include "v8/include/v8.h"
 
+using base::BinaryValue;
+using base::DictionaryValue;
+using base::ListValue;
+using base::StringValue;
+using base::Value;
+
+
 namespace content {
 
 V8ValueConverter* V8ValueConverter::create() {
@@ -77,6 +86,9 @@ v8::Handle<v8::Value> V8ValueConverterImpl::ToV8ValueImpl(
     case Value::TYPE_DICTIONARY:
       return ToV8Object(static_cast<const DictionaryValue*>(value));
 
+    case Value::TYPE_BINARY:
+      return ToArrayBuffer(static_cast<const BinaryValue*>(value));
+
     default:
       LOG(ERROR) << "Unexpected value type: " << value->GetType();
       return v8::Null();
@@ -127,6 +139,14 @@ v8::Handle<v8::Value> V8ValueConverterImpl::ToV8Object(
   return result;
 }
 
+v8::Handle<v8::Value> V8ValueConverterImpl::ToArrayBuffer(
+    const BinaryValue* value) const {
+  WebKit::WebArrayBuffer buffer =
+    WebKit::WebArrayBuffer::create(value->GetSize(), 1);

[Matt Perry, 2012/05/04 19:22:21]

indent += 2

[asargent_no_longer_on_chrome, 2012/05/04 22:42:22]

Done.

+  memcpy(buffer.data(), value->GetBuffer(), value->GetSize());

[asargent_no_longer_on_chrome, 2012/05/04 18:10:08]

Note that I'm looking into ways to avoid the need to copy here, but that will be
follow-up performance optimization work and might require a bit of refactoring
(actually, I'm not even sure if it's possible to transfer ownership of memory
from data in incoming IPC's, since it might be backed by OS shared memory
regions?).

[Matt Perry, 2012/05/04 19:22:21]

BinaryValues are serialized in ipc_message_utils.cc. There, they use
BinaryValue::CreateWithCopiedBuffer, so the buffer is already a copy of the IPC
stream. If there were a way to transfer ownership of the BinaryValue's buffer,
you could use that. (There isn't currently, but you could add a swap mtehod.) Of
course that assumes WebArrayBuffer supports it as well.

[asargent_no_longer_on_chrome, 2012/05/04 22:42:22]

Yep, but I'd also probably have to do something about the "const"-ness of the
incoming parameter. Maybe add the concept of BinaryValues with read-only
ref-counted shared buffers? (I think WebKit has something like this in WTF that
they use to avoid copies of WebArrayBuffer objects). 

+  return buffer.toV8Value();
+}
+
 Value* V8ValueConverterImpl::FromV8ValueImpl(v8::Handle<v8::Value> val) const {
   CHECK(!val.IsEmpty());
 
@@ -164,9 +184,13 @@ Value* V8ValueConverterImpl::FromV8ValueImpl(v8::Handle<v8::Value> val) const {
   if (val->IsArray())
     return FromV8Array(val.As<v8::Array>());
 
-  if (val->IsObject())
-    return FromV8Object(val->ToObject());
-
+  if (val->IsObject()) {
+    BinaryValue* binary_value = FromV8Buffer(val);
+    if (binary_value)

[Matt Perry, 2012/05/04 19:22:21]

nit: use braces in compound if/else statements.

[asargent_no_longer_on_chrome, 2012/05/04 22:42:22]

Done.

+      return binary_value;
+    else
+      return FromV8Object(val->ToObject());
+  }
   LOG(ERROR) << "Unexpected v8 value type encountered.";
   return Value::CreateNullValue();
 }
@@ -194,6 +218,31 @@ ListValue* V8ValueConverterImpl::FromV8Array(v8::Handle<v8::Array> val) const {
   return result;
 }
 
+base::BinaryValue* V8ValueConverterImpl::FromV8Buffer(
+    v8::Handle<v8::Value> val) const {
+  char* data = NULL;
+  size_t length = 0;
+
+  WebKit::WebArrayBuffer* array_buffer =
+      WebKit::WebArrayBuffer::createFromV8Value(val);
+  if (array_buffer) {
+    data = reinterpret_cast<char*>(array_buffer->data());
+    length = array_buffer->byteLength();
+  } else {
+    WebKit::WebArrayBufferView* view =
+        WebKit::WebArrayBufferView::createFromV8Value(val);
+    if (view) {
+      data = reinterpret_cast<char*>(view->baseAddress()) + view->byteOffset();
+      length = view->byteLength();
+    }
+  }
+
+  if (data)
+    return base::BinaryValue::CreateWithCopiedBuffer(data, length);

[asargent_no_longer_on_chrome, 2012/05/04 18:10:08]

Similarly here, it would be nice to avoid needing to make a copy. One thing we
might do is add the notion of a BinaryValue with an "unowned" buffer that is
read-only. The only danger is that we would need to somehow make sure we don't
hold on to that pointer past the lifetime of this method dispatching (ie keep
from holding on to it past where we get back into V8/Webkit code which could
change or deallocate the buffer)

[Matt Perry, 2012/05/04 19:22:21]

Hmm... maybe introduce a WeakBinaryValue that has a weak pointer to the buffer?
But that might be overkill.

+  else
+    return NULL;
+}
+
 DictionaryValue* V8ValueConverterImpl::FromV8Object(
     v8::Handle<v8::Object> val) const {
   DictionaryValue* result = new DictionaryValue();