Bug 76040 - [v8] Int16Array.set(array, offset) fails on first execution
Summary: [v8] Int16Array.set(array, offset) fails on first execution
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ulan Degenbaev
URL:
Keywords:
Depends on: 68890
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-11 02:19 PST by Ulan Degenbaev
Modified: 2012-01-11 13:07 PST (History)
4 users (show)

See Also:


Attachments
Pass the offset argument to copyElements (6.77 KB, patch)
2012-01-11 02:36 PST, Ulan Degenbaev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ulan Degenbaev 2012-01-11 02:19:31 PST
The set() method of typed arrays ignores the offset argument on the first execution. For details see http://code.google.com/p/chromium/issues/detail?id=109785
Comment 1 Ulan Degenbaev 2012-01-11 02:36:44 PST
Created attachment 121997 [details]
Pass the offset argument to copyElements

Ken, could you please take a look?

The problem was that I missed the offset argument when calling copyElements. 
It passed the unit-tests because the fast set() method happened to be installed when calling set() with offset.

I added a separate test which calls the set with a valid offset at the beginning. There is not need for testing invalid offset because in such case copyElements is not called at all.
Comment 2 Kenneth Russell 2012-01-11 11:51:56 PST
Comment on attachment 121997 [details]
Pass the offset argument to copyElements

View in context: https://bugs.webkit.org/attachment.cgi?id=121997&action=review

Fix looks good. Is it possible to also incorporate tests for this into the array-unit-tests.html test? If not (because it only happens the first time set() is called for each view type) then the separate test case is fine. Marking cq- until this question is answered.

> ChangeLog:6
> +        Reviewed by Kenneth Russell.

Leave the "Reviewed by NOBODY" line alone when uploading patches. The commit process patches it up automatically.
Comment 3 Ulan Degenbaev 2012-01-11 12:46:06 PST
>  Is it possible to also incorporate tests for this into the array-unit-tests.html test? If not (because it only happens the first time set() is called for each view type) then the separate test case is fine. 

Yes, it happens only once on the first call, so I would prefer a separate test, otherwise this test could be accidentally disabled when changing array-unit-tests in future.

> Leave the "Reviewed by NOBODY" line alone when uploading patches. The commit process patches it up automatically.
I see, thanks.
Comment 4 Kenneth Russell 2012-01-11 12:47:01 PST
Comment on attachment 121997 [details]
Pass the offset argument to copyElements

OK. Code and tests look good. Setting cq+.
Comment 5 WebKit Review Bot 2012-01-11 13:06:48 PST
Comment on attachment 121997 [details]
Pass the offset argument to copyElements

Clearing flags on attachment: 121997

Committed r104736: <http://trac.webkit.org/changeset/104736>
Comment 6 WebKit Review Bot 2012-01-11 13:07:00 PST
All reviewed patches have been landed.  Closing bug.