[Webkit-unassigned] [Bug 29664] [Chromium] Add initial V8 bindings for WebGL

[bugzilla-daemon at webkit.org]

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


Dimitri Glazkov (Google) <dglazkov at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #39968|review?(dglazkov at chromium.o |review-
               Flag|rg)                         |




--- Comment #2 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2009-09-23 13:24:57 PDT ---
(From update of attachment 39968)
Welcome to the club, kbr! :)

Here's your first round of hazing. I only looked at style so far. Let's get
that out of the way first.

> +    if (argLen != 1) {
> +        return throwError("Wrong number of arguments specified to constructor (requires 1)");
> +    }

No braces for one-liner conditions.

> +    int len = 0;
> +    if (!args[0]->IsInt32()) {
> +        return throwError("Argument to CanvasArrayBuffer constructor was not an integer");
> +    }

Ditto.

> +        if (argLen == 0) {
> +            return throwError("No arguments specified to constructor");
> +        }

Ditto.

> +
> +        // See whether the first argument is a CanvasArrayBuffer.
> +        if (V8CanvasArrayBuffer::HasInstance(args[0])) {
> +            if (argLen > 3) {
> +                return throwError("Wrong number of arguments to new Canvas<T>Array(CanvasArrayBuffer, int, int)");
> +            }

Ditto.

> +            CanvasArrayBuffer* buf =
> +                V8DOMWrapper::convertToNativeObject<CanvasArrayBuffer>(V8ClassIndex::CANVASARRAYBUFFER,
> +                                                                       args[0]->ToObject());

BTW, no need to constrain to 80 cols. WebKit style allows you to run free.

> +            if (buf == NULL) {
> +                return throwError("Could not convert argument 0 to a CanvasArrayBuffer");
> +            }

No braces.

> +            int offset = 0;
> +            if (argLen > 1) {
> +                offset = toInt32(args[1], ok);
> +                if (!ok) {
> +                    return throwError("Could not convert argument 1 to an integer");
> +                }

Ditto.

> +            if (argLen > 2) {
> +                length = toInt32(args[2], ok);
> +                if (!ok) {
> +                    return throwError("Could not convert argument 2 to an integer");
> +                }

Ditto.

> +            }
> +            if (length < 0) {
> +                return throwError("Length / offset out of range");

Ditto and elsewhere.

> +                if (!val->IsNumber()) {
> +                    char buf[256];
> +                    sprintf(buf, "Could not convert array element %d to a number", i);

Probably shouldn't be sprintf'ing. LOG(...), perhaps?

> +    if ((index < 0) || (index >= byteBuffer->length()))

too many parens here and elsewhere.

> +    if ((index >= 0) && (index < array->length())) {

Ditto.

> +    float* data;
> +    if (!tryFastMalloc(len * sizeof(float)).getValue(data)) {
> +        return NULL;

return 0; here and elsewhere.

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