Bug 76040

Summary: [v8] Int16Array.set(array, offset) fails on first execution
Product: WebKit Reporter: Ulan Degenbaev <ulan>
Component: WebGLAssignee: Ulan Degenbaev <ulan>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, japhet, kbr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 68890    
Bug Blocks:    
Attachments:
Description Flags
Pass the offset argument to copyElements none

Ulan Degenbaev
Reported 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
Attachments
Pass the offset argument to copyElements (6.77 KB, patch)
2012-01-11 02:36 PST, Ulan Degenbaev 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ulan Degenbaev
Comment 1 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.
Kenneth Russell
Comment 2 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.
Ulan Degenbaev
Comment 3 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.
Kenneth Russell
Comment 4 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+.
WebKit Review Bot
Comment 5 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>
WebKit Review Bot
Comment 6 2012-01-11 13:07:00 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.