[Webkit-unassigned] [Bug 66646] ArrayBuffer should have slice method.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=66646


Kenneth Russell <kbr at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #104951|review?                     |review-
               Flag|                            |




--- Comment #6 from Kenneth Russell <kbr at google.com>  2011-10-18 12:02:42 PST ---
(From update of attachment 104951)
View in context: https://bugs.webkit.org/attachment.cgi?id=104951&action=review

Thanks for picking up this work. The code looks fine and the test looks great. A couple of minor nits to clean up before committing. Also, please mark cq? on the bug if you would like it committed after review.

> LayoutTests/ChangeLog:3
> +        Added unittest for ArrayBuffer#slice

Please use the bug description verbatim along with the URL. If necessary, provide detail in additional paragraphs.

> Source/WebCore/ChangeLog:3
> +        Implemented ArrayBuffer#slice.

Same comment here.

> Source/WebCore/html/canvas/ArrayBuffer.cpp:33
> +static long clampValue(long x, long left, long right)

This should take and return ints instead of longs.

> Source/WebCore/html/canvas/ArrayBuffer.h:54
> +    unsigned index(int idx) const;

This method name is a little confusing; what about "clampIndex"? Also, avoid abbreviations (see http://www.webkit.org/coding/coding-style.html ), so the argument should be named "index".

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.