More informative error messages for non-Transferables.

Source/bindings/v8/V8Binding.h

Index: Source/bindings/v8/V8Binding.h
diff --git a/Source/bindings/v8/V8Binding.h b/Source/bindings/v8/V8Binding.h
index d00ee43e8b3c5cb55fd7930df88ecd1f23e4280c..9f4a76ff59a5d9002a55e13c41ff338f1f51caba 100644
--- a/Source/bindings/v8/V8Binding.h
+++ b/Source/bindings/v8/V8Binding.h
@@ -75,7 +75,7 @@ namespace WebCore {
 
     v8::ArrayBuffer::Allocator* v8ArrayBufferAllocator();
 
-    v8::Handle<v8::Value> toV8Sequence(v8::Handle<v8::Value>, uint32_t& length, v8::Isolate*);
+    v8::Handle<v8::Value> toV8Sequence(v8::Handle<v8::Value>, uint32_t& length, v8::Isolate*, bool* notASequence = 0);

[Mike West, 2013/10/22 06:50:40]

I think I'd prefer to keep the Isolate as the last argument by creating another
version of this function. I don't know of anywhere else in the bindings code
where the Isolate isn't at the end, and I'd like to maintain that unless there's
a good reason not to.

That is, I'd prefer to see:

 v8::Handle<v8::Value> toV8Sequence(v8::Handle<v8::Value>, uint32_t& length,
v8::Isolate*);
 v8::Handle<v8::Value> toV8Sequence(v8::Handle<v8::Value>, uint32_t& length,
bool* notASequence, v8::Isolate*);

where the first just wraps the second with a temporary bool. If that overhead
causes perf regressions, that'd be a good reason to break convention, but I
don't think it will.

[sof, 2013/10/22 16:06:56]

Done, and dropped the "nicety" of having the extra argument be optional.
However, the argument ordering convention of having 'out' arguments appear last
isn't followed, but I assume that doesn't trump Isolate consistency in
this particular case.

(The previous version was patterned on the ordering that toRefPtrNativeArray()
uses.)

 
     inline v8::Handle<v8::Value> argumentOrNull(const v8::FunctionCallbackInfo<v8::Value>& args, int index)
     {
@@ -510,7 +510,7 @@ namespace WebCore {
 
     // Validates that the passed object is a sequence type per WebIDL spec
     // 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)
+    inline v8::Handle<v8::Value> toV8Sequence(v8::Handle<v8::Value> value, uint32_t& length, v8::Isolate* isolate, bool* notASequence)
     {
         // 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.
@@ -518,7 +518,11 @@ namespace WebCore {
         // FIXME: Do we really need to special case Date and RegExp object?
         // https://www.w3.org/Bugs/Public/show_bug.cgi?id=22806
         if (!value->IsObject() || value->IsDate() || value->IsRegExp()) {
-            throwTypeError(isolate);
+            // Signal that the caller must handle the type error.
+            if (notASequence)
+                *notASequence = true;
+            else
+                throwTypeError(isolate);

[Mike West, 2013/10/22 06:50:40]

I'd be happy to review a subsequent patch that added a nicer error message here,
and on line 542. :)

[sof, 2013/10/22 16:06:56]

Definitely, I can have a look at improving that.

             return v8Undefined();
         }
 
@@ -531,7 +535,11 @@ namespace WebCore {
         V8TRYCATCH(v8::Local<v8::Value>, lengthValue, object->Get(v8::String::NewSymbol("length")));
 
         if (lengthValue->IsUndefined() || lengthValue->IsNull()) {
-            throwTypeError(isolate);
+            // Signal that the caller must handle the type error.
+            if (notASequence)
+                *notASequence = true;
+            else
+                throwTypeError(isolate);
             return v8Undefined();
         }