Bug 42125

Summary: bufferData and bufferSubData generate wrong error when null buffer is bound
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Zhenyao Mo <zmo>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cjerdonek, cmarrin, dbates, dglazkov, eric, fishd, japhet, levin, oliver, zmo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 41884    
Bug Blocks:    
Attachments:
Description Flags
patch
zmo: commit-queue-
revised patch japhet: review+, zmo: commit-queue-

Description Kenneth Russell 2010-07-12 18:19:15 PDT 
When no buffer is bound to the ARRAY_BUFFER or ELEMENT_ARRAY_BUFFER binding point and bufferData or bufferSubData are called, the WebGLRenderingContext will synthesize INVALID_ENUM. In this case it should synthesize INVALID_OPERATION.
Comment 1 Zhenyao Mo 2010-07-13 16:39:22 PDT 
Created attachment 61432 [details]
patch
Comment 2 Zhenyao Mo 2010-07-15 14:34:53 PDT 
Created attachment 61723 [details]
revised patch

Sync the test with khronos conformance tests using kbr's script.  No other changes.
Comment 3 Kenneth Russell 2010-07-15 15:11:50 PDT 
Comment on attachment 61723 [details]
revised patch

Looks nice.
Comment 4 Nate Chapin 2010-07-15 16:03:25 PDT 
Comment on attachment 61723 [details]
revised patch

Ok.
Comment 5 David Levin 2010-07-15 16:10:30 PDT 
It looks like you change the behavior so that when usage is not a valid enum, it will set INVALID_ENUM but it did not do this before from what I can tell.

Would you add a test for that as well?
Comment 6 Zhenyao Mo 2010-07-15 18:32:31 PDT 
(In reply to comment #5)
> It looks like you change the behavior so that when usage is not a valid enum, it will set INVALID_ENUM but it did not do this before from what I can tell.
> 
> Would you add a test for that as well?

The test case was added by an earlier patch to invalid-passed-params.html.

See https://bugs.webkit.org/show_bug.cgi?id=41574
Comment 7 Zhenyao Mo 2010-07-15 19:20:30 PDT 
Seems like there is a bug in webkit-patch land.  Changeset message got mixed with a previous patch, and it didn't automatically close this bug.

Close it manually: http://trac.webkit.org/changeset/63505
Comment 8 Zhenyao Mo 2010-07-15 19:36:41 PDT 
(In reply to comment #7)
> Seems like there is a bug in webkit-patch land.  Changeset message got mixed with a previous patch, and it didn't automatically close this bug.
> 
> Close it manually: http://trac.webkit.org/changeset/63505

Seems like the cause is the automatic rebasing of WebCore/Changelog went wrong: a latest patch's log wasn't pushed on top; it was inserted second place.  Manually fix this issue in the ChangeLog: http://trac.webkit.org/changeset/63506
Comment 9 Eric Seidel (no email) 2010-07-16 07:53:42 PDT 
@abarth:  Could this be the new retry code failing?  Or did svn-apply somehow apply this (seeminly correctly created diff) wrong?
Comment 10 Adam Barth 2010-07-16 15:23:28 PDT 
> Could this be the new retry code failing? 

I doubt it.  That's for commit-queue only.

> Or did svn-apply somehow apply this (seeminly correctly created diff) wrong?

I be the committer ran update-webkit can got a clean merge so resolve-ChangeLogs didn't move the ChangeLog entry to the top.  We just need to error out of webkit-patch land if the diff to the ChangeLog isn't at the top.
Comment 11 Eric Seidel (no email) 2010-07-16 16:40:00 PDT 
Oh!  I was confused.  I thought this was landed by the cq.  Yes, if you landed it manually, git will often mangle your ChangeLogs like this.

We need to put some detection code in webkit-patch land to abort hte commit if your ChangeLog got git-mangled.