48962 2010-11-03 16:56:24 -0700 Broken WebGL tests due to r71274 2010-11-08 08:57:48 -0800 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 enne enne dumi enne kbr zmo oldest_to_newest 304359 0 enne 2010-11-03 16:56:24 -0700 In LayoutTests/fast/canvas/webgl/, gl-object-get-calls and bad-arguments-test.html were broken because several cases of returning empty string or undefined were changed to null. 304365 1 72885 enne 2010-11-03 16:59:18 -0700 Created attachment 72885 Patch 304370 2 72885 kbr 2010-11-03 17:05:40 -0700 Comment on attachment 72885 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72885&action=review > LayoutTests/fast/canvas/webgl/gl-object-get-calls.html:43 > +shouldBeTrue('gl.getAttachedShaders(null) == undefined'); I don't think this should ever return undefined. See https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#5.14.3 . Why do we not use shouldBeNull here? > WebCore/html/canvas/WebGLRenderingContext.cpp:1633 > + return ""; How are these return values different? Does one translate to null at the JS level and one to an empty string? 304372 3 72885 kbr 2010-11-03 17:07:10 -0700 Comment on attachment 72885 Patch In the interest of un-breaking the tree I'm going to r+ this patch because it looks like the author is out for the rest of the day. However I'm leaving the bug open because I think a revision is necessary. 304373 4 enne 2010-11-03 17:10:16 -0700 (In reply to comment #2) > (From update of attachment 72885 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=72885&action=review > > > LayoutTests/fast/canvas/webgl/gl-object-get-calls.html:43 > > +shouldBeTrue('gl.getAttachedShaders(null) == undefined'); > > I don't think this should ever return undefined. See https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#5.14.3 . Why do we not use shouldBeNull here? I'm not sure what you're linking to there. Isn't that about lost contexts? In my reading of the spec, both null and undefined are valid return values in the case of an invalid program. > > WebCore/html/canvas/WebGLRenderingContext.cpp:1633 > > + return ""; > > How are these return values different? Does one translate to null at the JS level and one to an empty string? That's exactly it. I could add a comment to this effect. 304375 5 kbr 2010-11-03 17:14:42 -0700 (In reply to comment #4) > (In reply to comment #2) > > (From update of attachment 72885 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=72885&action=review > > > > > LayoutTests/fast/canvas/webgl/gl-object-get-calls.html:43 > > > +shouldBeTrue('gl.getAttachedShaders(null) == undefined'); > > > > I don't think this should ever return undefined. See https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/WebGL-spec.html#5.14.3 . Why do we not use shouldBeNull here? > > I'm not sure what you're linking to there. Isn't that about lost contexts? "All methods returning nullable values or any return null." > In my reading of the spec, both null and undefined are valid return values in the case of an invalid program. Implementations should not return undefined for WebGLShader[ ] under any circumstances. Where does the spec indicate that this might happen? > > > WebCore/html/canvas/WebGLRenderingContext.cpp:1633 > > > + return ""; > > > > How are these return values different? Does one translate to null at the JS level and one to an empty string? > > That's exactly it. I could add a comment to this effect. Might be helpful. 304377 6 kbr 2010-11-03 17:26:29 -0700 Committed r71289: <http://trac.webkit.org/changeset/71289> 304379 7 kbr 2010-11-03 17:27:16 -0700 Accidentally closed this bug when necessarily landing the patch above to un-break the tree. Reopening. 304391 8 enne 2010-11-03 18:10:56 -0700 (In reply to comment #7) > Accidentally closed this bug when necessarily landing the patch above to un-break the tree. Reopening. gl-object-get-calls.html is still showing up with an error because of one extra blank line at the end. It ran cleanly, but my git settings stripped that line when I committed locally and uploaded. I will rebaseline this test. 72885 2010-11-03 16:59:18 -0700 2010-11-03 17:07:10 -0700 Patch bug-48962-20101103165917.patch text/plain 5334 enne ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA1OTRiMjY2MjY4NWRkYjM1NmY5OGY2ZDM0Y2FhODA4NTY3Njc1MjY4Li45NjYwY2Y2 NTBjMTkzZDMwNjkxYWViNDNjZWRhNTA1NzZjNDFhODdiIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0 cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAK KzIwMTAtMTEtMDMgIEFkcmllbm5lIFdhbGtlciAgPGVubmVAZ29vZ2xlLmNvbT4KKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZWJHTCBzcGVjIGRvZXNu J3Qgc3BlY2lmeSB3aGV0aGVyIHRoZSByZXR1cm4gZnJvbSBnZXRBdHRhY2hlZFNoYWRlcnMKKyAg ICAgICAgY2FuIGJlIHVuZGVmaW5lZCBvciBudWxsLiAgVG9sZXJhdGUgYm90aC4KKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ4OTYyCisKKyAgICAgICAg KiBmYXN0L2NhbnZhcy93ZWJnbC9nbC1vYmplY3QtZ2V0LWNhbGxzLWV4cGVjdGVkLnR4dDoKKyAg ICAgICAgKiBmYXN0L2NhbnZhcy93ZWJnbC9nbC1vYmplY3QtZ2V0LWNhbGxzLmh0bWw6CisKIDIw MTAtMTEtMDMgIEFuZGVycyBDYXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAg ICBSZXZpZXdlZCBieSBBZGFtIFJvYmVuLgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9j YW52YXMvd2ViZ2wvZ2wtb2JqZWN0LWdldC1jYWxscy1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0 cy9mYXN0L2NhbnZhcy93ZWJnbC9nbC1vYmplY3QtZ2V0LWNhbGxzLWV4cGVjdGVkLnR4dAppbmRl eCAyZTJmY2M1MTk5YzIzN2NmMzViMTYxMjI5OTlmYmFhOGNlNTkwMTAzLi5iYWI1ZWZmY2JmZmQw ZjM0YmQ4OWY1NDI1Y2MzMDg1YTkxZjZjYjNjIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9mYXN0 L2NhbnZhcy93ZWJnbC9nbC1vYmplY3QtZ2V0LWNhbGxzLWV4cGVjdGVkLnR4dAorKysgYi9MYXlv dXRUZXN0cy9mYXN0L2NhbnZhcy93ZWJnbC9nbC1vYmplY3QtZ2V0LWNhbGxzLWV4cGVjdGVkLnR4 dApAQCAtNSw3ICs1LDcgQEAgT24gc3VjY2VzcywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQ QVNTIiBtZXNzYWdlcywgZm9sbG93ZWQgYnkgIlRFU1QgQ09NUExFVEUKIFBBU1Mgc2hhZGVycy5s ZW5ndGggaXMgMgogUEFTUyBzaGFkZXJzWzBdID09IHN0YW5kYXJkVmVydCAmJiBzaGFkZXJzWzFd ID09IHN0YW5kYXJkRnJhZyB8fCBzaGFkZXJzWzFdID09IHN0YW5kYXJkVmVydCAmJiBzaGFkZXJz WzBdID09IHN0YW5kYXJkRnJhZyBpcyB0cnVlCiBQQVNTIGdldEVycm9yIHdhcyBleHBlY3RlZCB2 YWx1ZTogTk9fRVJST1IgOiAKLVBBU1MgZ2wuZ2V0QXR0YWNoZWRTaGFkZXJzKG51bGwpIGlzIHVu ZGVmaW5lZC4KK1BBU1MgZ2wuZ2V0QXR0YWNoZWRTaGFkZXJzKG51bGwpID09IHVuZGVmaW5lZCBp cyB0cnVlCiBQQVNTIGdldEVycm9yIHdhcyBleHBlY3RlZCB2YWx1ZTogSU5WQUxJRF9WQUxVRSA6 IAogUEFTUyBnbC5nZXRBdHRhY2hlZFNoYWRlcnMoc3RhbmRhcmRWZXJ0KSB0aHJldyBleGNlcHRp b24gVHlwZUVycm9yOiBUeXBlIGVycm9yLgogUEFTUyBnZXRFcnJvciB3YXMgZXhwZWN0ZWQgdmFs dWU6IE5PX0VSUk9SIDogCkBAIC05MCw0ICs5MCwzIEBAIFBBU1MgZ2V0RXJyb3Igd2FzIGV4cGVj dGVkIHZhbHVlOiBOT19FUlJPUiA6CiBQQVNTIHN1Y2Nlc3NmdWxseVBhcnNlZCBpcyB0cnVlCiAK IFRFU1QgQ09NUExFVEUKLQpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9jYW52YXMvd2Vi Z2wvZ2wtb2JqZWN0LWdldC1jYWxscy5odG1sIGIvTGF5b3V0VGVzdHMvZmFzdC9jYW52YXMvd2Vi Z2wvZ2wtb2JqZWN0LWdldC1jYWxscy5odG1sCmluZGV4IDhhNmRiMzRkNzc4N2I2OGJkYjYzY2Ix Yzk1OGQwZmM5NjE0MzJlMmEuLjU2NDM5ZGJlNzVjYWY0YjRlYzI1ZmE5NTlmOTRiOTllMWJjZjQ2 OWMgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2Zhc3QvY2FudmFzL3dlYmdsL2dsLW9iamVjdC1n ZXQtY2FsbHMuaHRtbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2NhbnZhcy93ZWJnbC9nbC1vYmpl Y3QtZ2V0LWNhbGxzLmh0bWwKQEAgLTQwLDcgKzQwLDcgQEAgdmFyIHNoYWRlcnMgPSBnbC5nZXRB dHRhY2hlZFNoYWRlcnMoc3RhbmRhcmRQcm9ncmFtKTsKIHNob3VsZEJlKCdzaGFkZXJzLmxlbmd0 aCcsICcyJyk7CiBzaG91bGRCZVRydWUoJ3NoYWRlcnNbMF0gPT0gc3RhbmRhcmRWZXJ0ICYmIHNo YWRlcnNbMV0gPT0gc3RhbmRhcmRGcmFnIHx8IHNoYWRlcnNbMV0gPT0gc3RhbmRhcmRWZXJ0ICYm IHNoYWRlcnNbMF0gPT0gc3RhbmRhcmRGcmFnJyk7CiBnbEVycm9yU2hvdWxkQmUoZ2wsIGdsLk5P X0VSUk9SKTsKLXNob3VsZEJlVW5kZWZpbmVkKCdnbC5nZXRBdHRhY2hlZFNoYWRlcnMobnVsbCkn KTsKK3Nob3VsZEJlVHJ1ZSgnZ2wuZ2V0QXR0YWNoZWRTaGFkZXJzKG51bGwpID09IHVuZGVmaW5l ZCcpOwogZ2xFcnJvclNob3VsZEJlKGdsLCBnbC5JTlZBTElEX1ZBTFVFKTsKIHNob3VsZFRocm93 KCdnbC5nZXRBdHRhY2hlZFNoYWRlcnMoc3RhbmRhcmRWZXJ0KScpOwogZ2xFcnJvclNob3VsZEJl KGdsLCBnbC5OT19FUlJPUik7CmRpZmYgLS1naXQgYS9XZWJDb3JlL0NoYW5nZUxvZyBiL1dlYkNv cmUvQ2hhbmdlTG9nCmluZGV4IDM5ZWJlYTI4ZWVkMzY4ZGZhNDEwOWIyY2M1ZGY5OWFmODcxZjEx NzQuLjE3ODdkYzZhNmY2MmYxZjgyZjJjOTlkMTgwMjlkMDEyZmJmYmU1NGUgMTAwNjQ0Ci0tLSBh L1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMTgg QEAKIDIwMTAtMTEtMDMgIEFkcmllbm5lIFdhbGtlciAgPGVubmVAZ29vZ2xlLmNvbT4KIAorICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBQcm9wZXJseSByZXR1 cm4gZW1wdHkgc3RyaW5ncyBpbnN0ZWFkIG9mIG51bGwgd2hlbiB1c2luZyBpbnZhbGlkCisgICAg ICAgIG9iamVjdHMgZm9yIHNvbWUgV2ViR0wgY2FsbHMuICBUaGlzIGZpeGVzIGlzc3VlcyBjYXVz ZWQgYnkgcjcxMjc0LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9NDg5NjIKKworICAgICAgICAqIGh0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4 dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpXZWJHTFJlbmRlcmluZ0NvbnRleHQ6OmdldFByb2dy YW1JbmZvTG9nKToKKyAgICAgICAgKFdlYkNvcmU6OldlYkdMUmVuZGVyaW5nQ29udGV4dDo6Z2V0 U2hhZGVySW5mb0xvZyk6CisgICAgICAgIChXZWJDb3JlOjpXZWJHTFJlbmRlcmluZ0NvbnRleHQ6 OmdldFNoYWRlclNvdXJjZSk6CisKKzIwMTAtMTEtMDMgIEFkcmllbm5lIFdhbGtlciAgPGVubmVA Z29vZ2xlLmNvbT4KKwogICAgICAgICBSZXZpZXdlZCBieSBLZW5uZXRoIFJ1c3NlbGwuCiAKICAg ICAgICAgSW1wbGVtZW50IGxvc3QsIHJlc3RvcmVkLCBhbmQgY3JlYXRpb24gZmFpbHVyZSBjb250 ZXh0IGV2ZW50cyBmb3IKZGlmZiAtLWdpdCBhL1dlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5k ZXJpbmdDb250ZXh0LmNwcCBiL1dlYkNvcmUvaHRtbC9jYW52YXMvV2ViR0xSZW5kZXJpbmdDb250 ZXh0LmNwcAppbmRleCBlYjkwYWNjYjUwOTBjZDBlNjVjZDJmOTFiYTY1YjczZTgzMTdhMmEyLi45 N2M1MGUxYmZkYTg3OTkzNDQ0MDI0NzhkNzBiYWU2YWMxYjlhM2EwIDEwMDY0NAotLS0gYS9XZWJD b3JlL2h0bWwvY2FudmFzL1dlYkdMUmVuZGVyaW5nQ29udGV4dC5jcHAKKysrIGIvV2ViQ29yZS9o dG1sL2NhbnZhcy9XZWJHTFJlbmRlcmluZ0NvbnRleHQuY3BwCkBAIC0xNjI3LDggKzE2MjcsMTAg QEAgV2ViR0xHZXRJbmZvIFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6Z2V0UHJvZ3JhbVBhcmFtZXRl cihXZWJHTFByb2dyYW0qIHByb2dyYW0sIHUKIFN0cmluZyBXZWJHTFJlbmRlcmluZ0NvbnRleHQ6 OmdldFByb2dyYW1JbmZvTG9nKFdlYkdMUHJvZ3JhbSogcHJvZ3JhbSwgRXhjZXB0aW9uQ29kZSYg ZWMpCiB7CiAgICAgVU5VU0VEX1BBUkFNKGVjKTsKLSAgICBpZiAoaXNDb250ZXh0TG9zdCgpIHx8 ICF2YWxpZGF0ZVdlYkdMT2JqZWN0KHByb2dyYW0pKQorICAgIGlmIChpc0NvbnRleHRMb3N0KCkp CiAgICAgICAgIHJldHVybiBTdHJpbmcoKTsKKyAgICBpZiAoIXZhbGlkYXRlV2ViR0xPYmplY3Qo cHJvZ3JhbSkpCisgICAgICAgIHJldHVybiAiIjsKICAgICBXZWJHTFN0YXRlUmVzdG9yZXIodGhp cywgZmFsc2UpOwogICAgIHJldHVybiBtX2NvbnRleHQtPmdldFByb2dyYW1JbmZvTG9nKG9iamVj dE9yWmVybyhwcm9ncmFtKSk7CiB9CkBAIC0xNjk2LDggKzE2OTgsMTAgQEAgV2ViR0xHZXRJbmZv IFdlYkdMUmVuZGVyaW5nQ29udGV4dDo6Z2V0U2hhZGVyUGFyYW1ldGVyKFdlYkdMU2hhZGVyKiBz aGFkZXIsIHVuc2kKIFN0cmluZyBXZWJHTFJlbmRlcmluZ0NvbnRleHQ6OmdldFNoYWRlckluZm9M b2coV2ViR0xTaGFkZXIqIHNoYWRlciwgRXhjZXB0aW9uQ29kZSYgZWMpCiB7CiAgICAgVU5VU0VE X1BBUkFNKGVjKTsKLSAgICBpZiAoaXNDb250ZXh0TG9zdCgpIHx8ICF2YWxpZGF0ZVdlYkdMT2Jq ZWN0KHNoYWRlcikpCisgICAgaWYgKGlzQ29udGV4dExvc3QoKSkKICAgICAgICAgcmV0dXJuIFN0 cmluZygpOworICAgIGlmICghdmFsaWRhdGVXZWJHTE9iamVjdChzaGFkZXIpKQorICAgICAgICBy ZXR1cm4gIiI7CiAgICAgV2ViR0xTdGF0ZVJlc3RvcmVyKHRoaXMsIGZhbHNlKTsKICAgICByZXR1 cm4gbV9jb250ZXh0LT5nZXRTaGFkZXJJbmZvTG9nKG9iamVjdE9yWmVybyhzaGFkZXIpKTsKIH0K QEAgLTE3MDUsOCArMTcwOSwxMCBAQCBTdHJpbmcgV2ViR0xSZW5kZXJpbmdDb250ZXh0OjpnZXRT aGFkZXJJbmZvTG9nKFdlYkdMU2hhZGVyKiBzaGFkZXIsIEV4Y2VwdGlvbkNvZAogU3RyaW5nIFdl YkdMUmVuZGVyaW5nQ29udGV4dDo6Z2V0U2hhZGVyU291cmNlKFdlYkdMU2hhZGVyKiBzaGFkZXIs IEV4Y2VwdGlvbkNvZGUmIGVjKQogewogICAgIFVOVVNFRF9QQVJBTShlYyk7Ci0gICAgaWYgKGlz Q29udGV4dExvc3QoKSB8fCAhdmFsaWRhdGVXZWJHTE9iamVjdChzaGFkZXIpKQorICAgIGlmIChp c0NvbnRleHRMb3N0KCkpCiAgICAgICAgIHJldHVybiBTdHJpbmcoKTsKKyAgICBpZiAoIXZhbGlk YXRlV2ViR0xPYmplY3Qoc2hhZGVyKSkKKyAgICAgICAgcmV0dXJuICIiOwogICAgIFdlYkdMU3Rh dGVSZXN0b3Jlcih0aGlzLCBmYWxzZSk7CiAgICAgcmV0dXJuIG1fY29udGV4dC0+Z2V0U2hhZGVy U291cmNlKG9iamVjdE9yWmVybyhzaGFkZXIpKTsKIH0K