111641 2013-03-06 16:14:02 -0800 Conformance Test 1.0.3 (Beta) function: bufferData undefined value failed. 2013-03-10 15:39:49 -0700 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 0 janderssen webkit-unassigned dino esprehn+autocc ojan.autocc webkit-bug-importer webkit.review.bot oldest_to_newest 849418 0 janderssen 2013-03-06 16:14:02 -0800 Base on the conformance test from kronos group : https://www.khronos.org/registry/webgl/sdk/tests/conformance/more/functions/bufferDataBadArgs.html, bufferData was not protecting against an undefined value being provided. 849683 1 191905 janderssen 2013-03-06 20:12:10 -0800 Created attachment 191905 Patch 851015 2 webkit-bug-importer 2013-03-08 02:21:22 -0800 <rdar://problem/13376856> 851023 3 191905 dino 2013-03-08 02:27:28 -0800 Comment on attachment 191905 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191905&action=review These may seem like nitpicking comments (and they are) but WebKit is especially strict about coding style. Sorry. You get used to it :) Make these easy fixes and the r+ is yours. > Source/WebCore/ChangeLog:7 > + Reviewed by NOBODY (OOPS!). > + In this space you should try to describe the bug and change in a little more detail. e.g. "The WebGL specification requires that buffer data cannot be of zero length. This is a new test in the Khronos 1.0.3 WebGL test suite." Also, you should reference the Khronos test here. Typically we require a test for every WebKit commit, but in this case it's ok because we are in the process of incorporating the Khronos tests into WebKit, so it will be there eventually. > Source/WebCore/ChangeLog:9 > + (WebCore): Remove this line. > Source/WebCore/ChangeLog:10 > + (WebCore::WebGLRenderingContext::bufferData): You should always try to explain what you did in the changelog. In this case it is really easy - just something like "Synthesize error and return if size was 0." > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:1115 > + // undefined in WEBGL will parse in 0. No need for this comment. Also, for future reference, WebKit coding style requires comments to start with a capital letter. 851838 4 192356 janderssen 2013-03-09 16:13:43 -0800 Created attachment 192356 Patch 851984 5 192356 dino 2013-03-10 15:31:42 -0700 Comment on attachment 192356 Patch Looks good. I missed this in the first pass, but we could have simply changed the test above to if (size <= 0), but this is explicit for the undefined case so I guess it is ok. 851988 6 192356 webkit.review.bot 2013-03-10 15:39:46 -0700 Comment on attachment 192356 Patch Clearing flags on attachment: 192356 Committed r145334: <http://trac.webkit.org/changeset/145334> 851989 7 webkit.review.bot 2013-03-10 15:39:49 -0700 All reviewed patches have been landed. Closing bug. 191905 2013-03-06 20:12:10 -0800 2013-03-09 16:13:33 -0800 Patch bug-111641-20130307140814.patch text/plain 1740 janderssen U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1MDA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYzg3NTIwZThmMDc2Mzdh NWUwZWZkNDRlZjUxN2VkY2I2OWNkZmM0Ni4uYmI1YTIwOTk1Zjc1YWQwZjAzMmUwNTNhZTIyOWU0 MTUzOGExZjVmMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEzLTAzLTA2ICBKYXNv biBBbmRlcnNzZW4gIDxqYW5kZXJzc2VuQGdtYWlsLmNvbT4KKworICAgICAgICBDb25mb3JtYW5j ZSBUZXN0IDEuMC4zIChCZXRhKSBmdW5jdGlvbjogYnVmZmVyRGF0YSB1bmRlZmluZWQgdmFsdWUg ZmFpbGVkLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTExNjQxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiBodG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwOgorICAgICAgICAoV2ViQ29y ZSk6CisgICAgICAgIChXZWJDb3JlOjpXZWJHTFJlbmRlcmluZ0NvbnRleHQ6OmJ1ZmZlckRhdGEp OgorCiAyMDEzLTAzLTA2ICBBbGV4ZXkgUHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgogCiAg ICAgICAgIFtNYWNdIFN5bnRoZXRpYyBSZXNvdXJjZVJlc3BvbnNlcyBjYW5ub3QgYmUgc2VudCBv dmVyIElQQyB3aXRob3V0IGxvc2luZyBtb3N0IGluZm9ybWF0aW9uCmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwIGIvU291cmNl L1dlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250ZXh0LmNwcAppbmRleCBjYmZi ZDM3MjZlZWY1ZjViODM0ODVmNjBjZmIwYzFiYTI5NDg0MzYyLi4wZTI0MmNiNTM1MzExOTlmNGMz OTk1ODc0YzE4M2U2YTE3YmZjYzI1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1sL2Nh bnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwv Y2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAKQEAgLTExMTEsNiArMTExMSwxMSBAQCB2 b2lkIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6YnVmZmVyRGF0YShHQzNEZW51bSB0YXJnZXQsIGxv bmcgbG9uZyBzaXplLCBHQzNEZW51bQogICAgICAgICBzeW50aGVzaXplR0xFcnJvcihHcmFwaGlj c0NvbnRleHQzRDo6SU5WQUxJRF9WQUxVRSwgImJ1ZmZlckRhdGEiLCAic2l6ZSA8IDAiKTsKICAg ICAgICAgcmV0dXJuOwogICAgIH0KKyAgICBpZiAoIXNpemUpIHsKKyAgICAgICAgLy8gdW5kZWZp bmVkIGluIFdFQkdMIHdpbGwgcGFyc2UgaW4gMC4KKyAgICAgICAgc3ludGhlc2l6ZUdMRXJyb3Io R3JhcGhpY3NDb250ZXh0M0Q6OklOVkFMSURfVkFMVUUsICJidWZmZXJEYXRhIiwgInNpemUgPT0g MCIpOworICAgICAgICByZXR1cm47CisgICAgfQogICAgIGlmICghaXNFcnJvckdlbmVyYXRlZE9u T3V0T2ZCb3VuZHNBY2Nlc3NlcygpKSB7CiAgICAgICAgIGlmICghYnVmZmVyLT5hc3NvY2lhdGVC dWZmZXJEYXRhKHN0YXRpY19jYXN0PEdDM0RzaXplaXB0cj4oc2l6ZSkpKSB7CiAgICAgICAgICAg ICBzeW50aGVzaXplR0xFcnJvcihHcmFwaGljc0NvbnRleHQzRDo6SU5WQUxJRF9WQUxVRSwgImJ1 ZmZlckRhdGEiLCAiaW52YWxpZCBidWZmZXIiKTsK 192356 2013-03-09 16:13:43 -0800 2013-03-10 15:39:46 -0700 Patch bug-111641-20130310100950.patch text/plain 2166 janderssen U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1MDA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYzg3NTIwZThmMDc2Mzdh NWUwZWZkNDRlZjUxN2VkY2I2OWNkZmM0Ni4uNDhhYjFlMjZkZGE4OGM3YzBkNWUyNzg3NWY1OTNh MzY4M2M5NTVmNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEzLTAzLTA2ICBKYXNv biBBbmRlcnNzZW4gIDxqYW5kZXJzc2VuQGdtYWlsLmNvbT4KKworICAgICAgICBDb25mb3JtYW5j ZSBUZXN0IDEuMC4zIChCZXRhKSBmdW5jdGlvbjogYnVmZmVyRGF0YSB1bmRlZmluZWQgdmFsdWUg ZmFpbGVkLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTExNjQxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg VGhlIFdlYkdMIHNwZWNpZmljYXRpb24gcmVxdWlyZXMgdGhhdCBhIHNpemUgb2YgMCBpcyBub3Qg dmFsaWQuIEluIGphdmFzY3JpcHQsIHBhc3NpbmcgaW4gdW5kZWZpbmVkIAorICAgICAgICBhcyBh IHBhcmFtZXRlciB0byBhIGxvbmcgbG9uZyBpcyB0aGUgc2FtZSBhcyBwYXNzaW5nIGluIDAsIHNv IHdlIG11c3QgY2hlY2sgZm9yIHRoaXMgaW5jb3JyZWN0CisgICAgICAgIHZhbHVlIGFuZCBmYWls LiAKKyAgICAgICAgVGhlIHRlc3Qgc3VpdGUgaW4gS3Jvbm9zIDEuMC4zIGZhaWxlZCwgdGVzdCB0 byB2ZXJpZnkgY29uZm9ybWFuY2UgaXMgYXMgZm9sbG93czoKKyAgICAgICAgaHR0cHM6Ly93d3cu a2hyb25vcy5vcmcvcmVnaXN0cnkvd2ViZ2wvc2RrL3Rlc3RzL2NvbmZvcm1hbmNlL21vcmUvZnVu Y3Rpb25zL2J1ZmZlckRhdGFCYWRBcmdzLmh0bWwuCisKKyAgICAgICAgKiBodG1sL2NhbnZhcy9X ZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6V2ViR0xSZW5kZXJp bmdDb250ZXh0OjpidWZmZXJEYXRhKToKKyAgICAgICAgU3ludGhlc2l6ZSBlcnJvciBhbmQgcmV0 dXJuZWQgaWYgc2l6ZSBpcyAwLgorCiAyMDEzLTAzLTA2ICBBbGV4ZXkgUHJvc2t1cnlha292ICA8 YXBAYXBwbGUuY29tPgogCiAgICAgICAgIFtNYWNdIFN5bnRoZXRpYyBSZXNvdXJjZVJlc3BvbnNl cyBjYW5ub3QgYmUgc2VudCBvdmVyIElQQyB3aXRob3V0IGxvc2luZyBtb3N0IGluZm9ybWF0aW9u CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0Nv bnRleHQuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250 ZXh0LmNwcAppbmRleCBjYmZiZDM3MjZlZWY1ZjViODM0ODVmNjBjZmIwYzFiYTI5NDg0MzYyLi40 OTNlOGEwZTkwNTcxMDRkOTFhNzI4YTI3ZWU3OWU2ZTQxMTc1Y2Q5IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9odG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwCisrKyBiL1Nv dXJjZS9XZWJDb3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAKQEAgLTEx MTEsNiArMTExMSwxMCBAQCB2b2lkIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6YnVmZmVyRGF0YShH QzNEZW51bSB0YXJnZXQsIGxvbmcgbG9uZyBzaXplLCBHQzNEZW51bQogICAgICAgICBzeW50aGVz aXplR0xFcnJvcihHcmFwaGljc0NvbnRleHQzRDo6SU5WQUxJRF9WQUxVRSwgImJ1ZmZlckRhdGEi LCAic2l6ZSA8IDAiKTsKICAgICAgICAgcmV0dXJuOwogICAgIH0KKyAgICBpZiAoIXNpemUpIHsK KyAgICAgICAgc3ludGhlc2l6ZUdMRXJyb3IoR3JhcGhpY3NDb250ZXh0M0Q6OklOVkFMSURfVkFM VUUsICJidWZmZXJEYXRhIiwgInNpemUgPT0gMCIpOworICAgICAgICByZXR1cm47CisgICAgfQog ICAgIGlmICghaXNFcnJvckdlbmVyYXRlZE9uT3V0T2ZCb3VuZHNBY2Nlc3NlcygpKSB7CiAgICAg ICAgIGlmICghYnVmZmVyLT5hc3NvY2lhdGVCdWZmZXJEYXRhKHN0YXRpY19jYXN0PEdDM0RzaXpl aXB0cj4oc2l6ZSkpKSB7CiAgICAgICAgICAgICBzeW50aGVzaXplR0xFcnJvcihHcmFwaGljc0Nv bnRleHQzRDo6SU5WQUxJRF9WQUxVRSwgImJ1ZmZlckRhdGEiLCAiaW52YWxpZCBidWZmZXIiKTsK