Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(262)

Unified Diff: Source/bindings/v8/V8Binding.h

Issue 19969004: Update toNativeArray() / toRefPtrNativeArray() do not match Web IDL specification (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/bindings/v8/V8Binding.h
diff --git a/Source/bindings/v8/V8Binding.h b/Source/bindings/v8/V8Binding.h
index 0acf81a5c7b7896557f8a4e76ba588b673d2ea9a..77033aefef7e18f7bcf7a17a4825b0c76077c382 100644
--- a/Source/bindings/v8/V8Binding.h
+++ b/Source/bindings/v8/V8Binding.h
@@ -75,6 +75,8 @@ namespace WebCore {
v8::ArrayBuffer::Allocator* v8ArrayBufferAllocator();
+ v8::Handle<v8::Value> toV8Sequence(v8::Handle<v8::Value>, uint32_t& length, v8::Isolate*);
+
inline v8::Handle<v8::Value> argumentOrNull(const v8::FunctionCallbackInfo<v8::Value>& args, int index)
{
return index >= args.Length() ? v8::Local<v8::Value>() : args[index];
@@ -402,25 +404,31 @@ namespace WebCore {
}
};
+ // Converts a JavaScript value to an array as per the Web IDL specification:
+ // http://www.w3.org/TR/2012/CR-WebIDL-20120419/#es-array
template <class T, class V8T>
Vector<RefPtr<T> > toRefPtrNativeArray(v8::Handle<v8::Value> value, v8::Isolate* isolate, bool* success = 0)
{
if (success)
*success = true;
- if (!value->IsArray())
- return Vector<RefPtr<T> >();
+ v8::Local<v8::Value> v8Value(v8::Local<v8::Value>::New(value));
haraken 2013/07/24 15:59:30 Please pass an Isolate to Local::New. It's sad th
do-not-use 2013/07/25 08:01:26 Ok, I forgot about this is the preferred way now.
+ uint32_t length = 0;
+ if (value->IsArray()) {
+ length = v8::Local<v8::Array>::Cast(v8Value)->Length();
+ } else {
arv (Not doing code reviews) 2013/07/24 16:38:13 else if
+ if (toV8Sequence(value, length, isolate).IsEmpty())
arv (Not doing code reviews) 2013/07/24 16:38:13 Are we converting this twice. Once to see if it is
+ return Vector<RefPtr<T> >();
+ }
Vector<RefPtr<T> > result;
- v8::Local<v8::Value> v8Value(v8::Local<v8::Value>::New(value));
- v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(v8Value);
- size_t length = array->Length();
- for (size_t i = 0; i < length; ++i) {
- v8::Handle<v8::Value> element = array->Get(i);
+ v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(v8Value);
+ for (uint32_t i = 0; i < length; ++i) {
haraken 2013/07/24 15:59:30 I'm just curious about why you choose uint32_t ins
arv (Not doing code reviews) 2013/07/24 16:38:13 I feel like I'm missing something here. Where is t
do-not-use 2013/07/25 08:01:26 It is done in the pre-existing toV8Sequence() func
do-not-use 2013/07/25 08:01:26 The reason is that: - v8::Array::Length() returns
+ v8::Handle<v8::Value> element = object->Get(i);
if (V8T::HasInstance(element, isolate, worldType(isolate))) {
- v8::Handle<v8::Object> object = v8::Handle<v8::Object>::Cast(element);
- result.append(V8T::toNative(object));
+ v8::Handle<v8::Object> elementObject = v8::Handle<v8::Object>::Cast(element);
+ result.append(V8T::toNative(elementObject));
} else {
if (success)
*success = false;
@@ -431,19 +439,25 @@ namespace WebCore {
return result;
}
+ // Converts a JavaScript value to an array as per the Web IDL specification:
+ // http://www.w3.org/TR/2012/CR-WebIDL-20120419/#es-array
template <class T>
- Vector<T> toNativeArray(v8::Handle<v8::Value> value)
+ Vector<T> toNativeArray(v8::Handle<v8::Value> value, v8::Isolate* isolate)
{
- if (!value->IsArray())
- return Vector<T>();
+ v8::Local<v8::Value> v8Value(v8::Local<v8::Value>::New(value));
haraken 2013/07/24 15:59:30 Please pass an Isolate to Local::New.
do-not-use 2013/07/25 08:01:26 Ok.
+ uint32_t length = 0;
+ if (value->IsArray()) {
+ length = v8::Local<v8::Array>::Cast(v8Value)->Length();
+ } else {
+ if (toV8Sequence(value, length, isolate).IsEmpty())
+ return Vector<T>();
+ }
Vector<T> result;
typedef NativeValueTraits<T> TraitsType;
- v8::Local<v8::Value> v8Value(v8::Local<v8::Value>::New(value));
- v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(v8Value);
- size_t length = array->Length();
- for (size_t i = 0; i < length; ++i)
- result.append(TraitsType::nativeValue(array->Get(i)));
+ v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(v8Value);
+ for (uint32_t i = 0; i < length; ++i)
+ result.append(TraitsType::nativeValue(object->Get(i)));
return result;
}
@@ -462,10 +476,13 @@ namespace WebCore {
Vector<v8::Handle<v8::Value> > toVectorOfArguments(const v8::FunctionCallbackInfo<v8::Value>& args);
// Validates that the passed object is a sequence type per WebIDL spec
- // http://www.w3.org/TR/2012/WD-WebIDL-20120207/#es-sequence
+ // http://www.w3.org/TR/2012/CR-WebIDL-20120419/#es-sequence
inline v8::Handle<v8::Value> toV8Sequence(v8::Handle<v8::Value> value, uint32_t& length, v8::Isolate* isolate)
{
- if (!value->IsObject()) {
+ // Attempt converting to a sequence if the value is not already an array but is
+ // any kind of object except for a native Date object or a native RegExp object.
+ ASSERT(!value->IsArray());
+ if (!value->IsObject() || value->IsDate() || value->IsRegExp()) {
haraken 2013/07/24 15:59:30 Would you add a test case about this? We might wan
arv (Not doing code reviews) 2013/07/24 16:38:13 Why are you special casing Date and RegExp? What a
do-not-use 2013/07/25 08:01:26 Yes, I will work on a test case for these.
do-not-use 2013/07/25 08:01:26 Because the Web IDL specification is special casin
arv (Not doing code reviews) 2013/07/25 18:24:41 Please link to WebIDL bug that might clarify why/i
throwTypeError(isolate);
return v8Undefined();
}
@@ -473,6 +490,9 @@ namespace WebCore {
v8::Local<v8::Value> v8Value(v8::Local<v8::Value>::New(value));
v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(v8Value);
+ // FIXME: The specification states that the length property should be used as fallback, if value
+ // is not a platform object that supports indexed properties. If it supports indexed properties,
+ // length should actually be one greater than value’s maximum indexed property index.
V8TRYCATCH(v8::Local<v8::Value>, lengthValue, object->Get(v8::String::NewSymbol("length")));
if (lengthValue->IsUndefined() || lengthValue->IsNull()) {

Powered by Google App Engine
This is Rietveld 408576698