Bug 40755 - Clean up error conditions for Typed Arrays
Summary: Clean up error conditions for Typed Arrays
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-16 19:13 PDT by Kenneth Russell
Modified: 2010-06-17 11:15 PDT (History)
6 users (show)

See Also:


Attachments
Patch (38.17 KB, patch)
2010-06-16 19:30 PDT, Kenneth Russell
kbr: commit-queue-
Details | Formatted Diff | Diff
Revised patch (38.17 KB, patch)
2010-06-16 19:49 PDT, Kenneth Russell
dglazkov: review+
kbr: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-06-16 19:13:32 PDT
Based on feedback from Cedric Vivier and others on the public WebGL mailing list, there are several error condition corner cases that need to be clarified or fixed in the Typed Array implementation.

1. For parity with the behavior of "new Array(length)", RangeError should be thrown if the length passed to the ArrayBuffer or ArrayBufferView constructors is negative.
2. INDEX_SIZE_ERR should be thrown when the length passed to the constructor was too large to allocate.
3. INDEX_SIZE_ERR must be thrown when the offset passed to the ArrayBufferView(ArrayBuffer, offset, length) is unaligned, or the offset and length are out of range.
4. Additional null checks are needed in some allocation code paths to prevent crashes.

The unit tests need to be updated to cover more of these cases.
Comment 1 Kenneth Russell 2010-06-16 19:30:12 PDT
Created attachment 58954 [details]
Patch

From the ChangeLog:

Fixed handling of NaN/+inf/-inf lengths. Throw RangeError for too-large or negative lengths. Changed all error code paths to throw exceptions. Clarified exception types. Added necessary null checks during allocation. Added tests for overloaded ArrayBufferView constructors taking null, and improperly aligned offsets. Improved precision of tests causing INDEX_SIZE_ERR to be thrown.
Comment 2 Eric Seidel (no email) 2010-06-16 19:36:11 PDT
Attachment 58954 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/3315260
Comment 3 Kenneth Russell 2010-06-16 19:49:10 PDT
Created attachment 58955 [details]
Revised patch

Fixed build problem on Leopard with last patch.
Comment 4 Dimitri Glazkov (Google) 2010-06-17 10:47:29 PDT
Comment on attachment 58955 [details]
Revised patch

ok.

I am a bit concerned that throwError family is multiplying like rabbits. Should we take away some when add new ones? Like throwError("Foo") already throws a type error by default.
Comment 5 Kenneth Russell 2010-06-17 11:15:44 PDT
Committed r61339: <http://trac.webkit.org/changeset/61339>