[webkit-reviews] review denied: [Bug 91056] [V8Bindings] Implement generalised method to validates that the passed object is a sequence type. : [Attachment 151882] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Vineet Chaudhary (vineetc)
<rgf748 at motorola.com>'s request for review:
Bug 91056: [V8Bindings] Implement generalised method to validates that the
passed object is a sequence type.
https://bugs.webkit.org/show_bug.cgi?id=91056

Attachment 151882: Patch
https://bugs.webkit.org/attachment.cgi?id=151882&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=151882&action=review


> Source/WebCore/bindings/v8/V8Binding.h:444
> +    inline v8::Handle<v8::Object> toV8Sequence(v8::Handle<v8::Value> value,
uint32_t& length)

I think the current toJSSequence() is conformed to the spec. Would you
implement toV8Sequence() so that it exactly corresponds to toJSSequence()?
(toV8Sequence() in this patch is not conformed to the spec and has a couple of
bugs.)

> Source/WebCore/bindings/v8/V8Binding.h:447
> +	       V8Proxy::throwTypeError("TransferArray argument must be an
object");

We want to make this message the same as the toJSSequence()'s one.

> Source/WebCore/bindings/v8/V8Binding.h:455
> +	       v8::Local<v8::Array> array =
v8::Local<v8::Array>::Cast(v8Value);
> +	       length = array->Length();

You are doing something like:

  array = ...;
  length = ...;
  return object;

which would be wrong.