Bug 33929 – [V8] Need to handle WebGLUnsignedByteArray as method argument for V8 binding

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Rogers]

This only applies to V8 (not JSC).  Apparently, WebGLUnsignedByteArray was accidentally left out of the GL types which may appear as method arguments in IDL files.
Nobody has noticed so far because this case has not been needed up to this point - but I'm working on a patch which will need it.

[Chris Rogers]

Created attachment 47084 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 47084 [details]
patch

This results in a functional change to the V8 JavaScript bindings, no?  If it does, it needs a test.  If it doesn't, it's just a build fix and doesn't need review. :)

[Eric Seidel (no email)]

Comment on attachment 47084 [details]
patch

I meant to mark it r-.  By "test" I mean a layout test.  Some javascript code which creates one of these WebGLUnsignedByteArray and tries to pass it somewhere where behavior has now changed.

[Chris Rogers]

Eric, AFAIK there are currently no IDL files which take WebGLUnsignedByteArray as an argument so there is nothing I can test.  I have a future patch which will need this and currently the JSC generator already handles this case, but V8 does not.  Put in other words, this fixes the V8 generator to be on par with JSC.  But currently there are no APIs which can be tested.  The omission of WebGLUnsignedByteArray (among the other WebGL types) appears to be an oversight.

thanks for having a look

[Eric Seidel (no email)]

Comment on attachment 47084 [details]
patch

Ok.  If there is nothing that uses this yet, then there is no way to test it.

ideally you should say that in the ChangeLog so that folks using annotate later have a better chance of understanding. :)

[Eric Seidel (no email)]

Comment on attachment 47084 [details]
patch

Chris isn't a committer, so marking cq+ too.

[WebKit Commit Bot]

Comment on attachment 47084 [details]
patch

Rejecting patch 47084 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Last 500 characters of output:
e/LayoutTests
Testing 12030 test cases.
http/tests/history/redirect-302.pl -> timed out
Sampling process 88106 for 10 seconds with 10 milliseconds of run time between samples
Sampling completed, processing symbols...
Sample analysis of process 88106 written to file /Users/eseidel/Library/Logs/DumpRenderTree/HangReport.txt

Exiting early after 1 failures. 11241 tests run.
297.88s total testing time

11240 test cases (99%) succeeded
1 test case (<1%) timed out
6 test cases (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/203615

[Jeremy Orlow]

Comment on attachment 47084 [details]
patch

Looks like flake.  Try again.

[Eric Seidel (no email)]

(In reply to comment #8)
> (From update of attachment 47084 [details])
> Looks like flake.  Try again.

Yes.  bug 30391.  Trying again was the proper response. :)

[WebKit Commit Bot]

Comment on attachment 47084 [details]
patch

Clearing flags on attachment: 47084

Committed r53760: <http://trac.webkit.org/changeset/53760>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.