[webkit-reviews] review denied: [Bug 69427] HTML canvas strokes with dash and dashOffset for V8. : [Attachment 109794] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Young Han Lee <joybro201 at gmail.com>'s
request for review:
Bug 69427: HTML canvas strokes with dash and dashOffset for V8.
https://bugs.webkit.org/show_bug.cgi?id=69427

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=109794&action=review


> Source/WebCore/bindings/v8/custom/V8CanvasRenderingContext2DCustom.cpp:116
> +    v8::Local<v8::Array> result = v8::Array::New(dash->size());
> +    DashArray::const_iterator end = dash->end();
> +    int index = 0;
> +    for (DashArray::const_iterator it = dash->begin(); it != end; ++it,
++index)
> +	   result->Set(v8::Integer::New(index), v8::Number::New(*it));
> +
> +    return result;
> +}

I think we already have a function that does this.  If we don't have one, you
should add it.	It's a very general operation.

Also, per the recent webkit-dev thread, we should be using indicies rather than
iterators to walk arrays.  That will also help you avoid the nuttiness of
incrementing the iterator and the index at the same time.