Allow serialization of ArrayBuffer params in extension/apps API methods

content/renderer/v8_value_converter_impl.cc

-// 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.
 
 #include "content/renderer/v8_value_converter_impl.h"
 
 #include <string>
 
 #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() {
   return new V8ValueConverterImpl();
 }
 
 }
 
 V8ValueConverterImpl::V8ValueConverterImpl()
     : allow_undefined_(false),
@@ skipping 46 matching lines @@
       CHECK(value->GetAsString(&val));
       return v8::String::New(val.c_str(), val.length());
     }
 
     case Value::TYPE_LIST:
       return ToV8Array(static_cast<const ListValue*>(value));
 
     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();
   }
 }
 
 v8::Handle<v8::Value> V8ValueConverterImpl::ToV8Array(
     const ListValue* val) const {
   v8::Handle<v8::Array> result(v8::Array::New(val->GetSize()));
 
@@ skipping 30 matching lines @@
     result->Set(v8::String::New(key.c_str(), key.length()), child_v8);
     if (try_catch.HasCaught()) {
       LOG(ERROR) << "Setter for property " << key.c_str() << " threw an "
                  << "exception.";
     }
   }
 
   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());
 
   if (val->IsNull())
     return Value::CreateNullValue();
 
   if (val->IsBoolean())
     return Value::CreateBooleanValue(val->ToBoolean()->Value());
 
   if (val->IsInt32())
@@ skipping 17 matching lines @@
 
   if (allow_regexp_ && val->IsRegExp()) {
     return Value::CreateStringValue(
         *v8::String::Utf8Value(val->ToString()));
   }
 
   // v8::Value doesn't have a ToArray() method for some reason.
   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();
 }
 
 ListValue* V8ValueConverterImpl::FromV8Array(v8::Handle<v8::Array> val) const {
   ListValue* result = new ListValue();
   for (uint32 i = 0; i < val->Length(); ++i) {
     v8::TryCatch try_catch;
     v8::Handle<v8::Value> child_v8 = val->Get(i);
     if (try_catch.HasCaught()) {
       LOG(ERROR) << "Getter for index " << i << " threw an exception.";
       child_v8 = v8::Null();
     }
 
     // TODO(aa): It would be nice to support getters, but we need
     // http://code.google.com/p/v8/issues/detail?id=1342 to do it properly.
     if (!val->HasRealIndexedProperty(i))
       continue;
 
     Value* child = FromV8ValueImpl(child_v8);
     CHECK(child);
 
     result->Append(child);
   }
   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();
   v8::Handle<v8::Array> property_names(val->GetPropertyNames());
   for (uint32 i = 0; i < property_names->Length(); ++i) {
     v8::Handle<v8::String> name(property_names->Get(i).As<v8::String>());
 
     // TODO(aa): It would be nice to support getters, but we need
     // http://code.google.com/p/v8/issues/detail?id=1342 to do it properly.
     if (!val->HasRealNamedProperty(name))
@@ skipping 10 matching lines @@
     }
 
     Value* child = FromV8ValueImpl(child_v8);
     CHECK(child);
 
     result->SetWithoutPathExpansion(std::string(*name_utf8, name_utf8.length()),
                                     child);
   }
   return result;
 }