67898 2011-09-10 17:07:18 -0700 REGRESSION(r94900): fast/images/support-broken-image-delegate.html fails on Mac 2011-09-12 14:51:22 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 67819 1 rniwa bdakin bdakin darin dglazkov eric hyatt mitz simon.fraser webkit.review.bot oldest_to_newest 465348 0 rniwa 2011-09-10 17:07:18 -0700 After http://trac.webkit.org/changeset/94900, fast/images/support-broken-image-delegate.html started failing on Leopard, Snow Leopard, & Lion: http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r94911%20(2096)/results.html http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r94909%20(33500)/results.html http://build.webkit.org/results/Lion%20Intel%20Debug%20(Tests)/r94910%20(535)/results.html 465353 1 rniwa 2011-09-10 17:16:58 -0700 Temporarily skipped the test in http://trac.webkit.org/changeset/94915. 465357 2 bdakin 2011-09-10 17:36:09 -0700 Thanks so much Ryosuke. I looked at this a bit, and I think I understand the problem. I changed RenderImage::imageSizeForError() so that instead of accessing CachedImage::image() and then measuring the size, it goes directly to CachedImage::brokenImage(). I think this test shows that that is wrong because in this case m_shouldPaintBrokenImage would be false and rather than returning the brokenImage, CachedImage::image() would have returned the null image. I think I have time to post a patch for this before I have to go. 465362 3 106993 bdakin 2011-09-10 18:01:43 -0700 Created attachment 106993 Patch Here's a patch to fix the test failure. Maybe check check for willPaintBrokenImage() should be inside brokenImage() itself…not sure…for the time being, I made it an independent function. 465368 4 rniwa 2011-09-10 18:20:32 -0700 cc more people in the hope they can review this patch. 465406 5 106993 webkit.review.bot 2011-09-11 14:07:20 -0700 Comment on attachment 106993 Patch Attachment 106993 did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9641169 New failing tests: svg/css/getComputedStyle-basic.xhtml svg/custom/svg-fonts-word-spacing.html media/video-zoom-controls.html 465489 6 106993 simon.fraser 2011-09-11 22:35:56 -0700 Comment on attachment 106993 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=106993&action=review > Source/WebCore/ChangeLog:6 > + REGRESSION(r94900): fast/images/support-broken-image-delegate.html fails on Mac > + > + No new tests. (OOPS!) Some verbage here describing the change would be good. 465801 7 bdakin 2011-09-12 14:38:17 -0700 (In reply to comment #6) > (From update of attachment 106993 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=106993&action=review > > > Source/WebCore/ChangeLog:6 > > + REGRESSION(r94900): fast/images/support-broken-image-delegate.html fails on Mac > > + > > + No new tests. (OOPS!) > > Some verbage here describing the change would be good. Thanks, Simon! I will add some verbiage. I cannot reproduce those test failures that the Bot claims are failures. It would also be very surprising if this change caused failures in those tests anyway. I plan to commit the change and monitor the build bots. 465808 8 bdakin 2011-09-12 14:51:22 -0700 Committed change with revision 94980. 106993 2011-09-10 18:01:43 -0700 2011-09-11 22:35:55 -0700 Patch for-review.txt text/plain 3702 bdakin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk0OTE2KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDktMTAgIEJldGggRGFr aW4gIDxiZGFraW5AYXBwbGUuY29tPgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD02Nzg5OAorICAgICAgICBSRUdSRVNTSU9OKHI5NDkwMCk6IGZhc3Qv aW1hZ2VzL3N1cHBvcnQtYnJva2VuLWltYWdlLWRlbGVnYXRlLmh0bWwgZmFpbHMgb24gTWFjCisK KyAgICAgICAgTm8gbmV3IHRlc3RzLiAoT09QUyEpCisKKyAgICAgICAgKiBsb2FkZXIvY2FjaGUv Q2FjaGVkSW1hZ2UuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q2FjaGVkSW1hZ2U6OndpbGxQYWlu dEJyb2tlbkltYWdlKToKKyAgICAgICAgKiBsb2FkZXIvY2FjaGUvQ2FjaGVkSW1hZ2UuaDoKKyAg ICAgICAgKiByZW5kZXJpbmcvUmVuZGVySW1hZ2UuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVu ZGVySW1hZ2U6OmltYWdlU2l6ZUZvckVycm9yKToKKwogMjAxMS0wOS0xMCAgS2V2aW4gT2xsaXZp ZXIgIDxrZXZpbm9AdGhlb2xsaXZpZXJzLmNvbT4KIAogICAgICAgICBbd3hdIFVucmV2aWV3ZWQg YnVpbGQgZml4LiBNU1cgYnVpbGQgZml4ZXMuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9sb2FkZXIv Y2FjaGUvQ2FjaGVkSW1hZ2UuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2xvYWRl ci9jYWNoZS9DYWNoZWRJbWFnZS5jcHAJKHJldmlzaW9uIDk0OTAwKQorKysgU291cmNlL1dlYkNv cmUvbG9hZGVyL2NhY2hlL0NhY2hlZEltYWdlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTI0LDYg KzEyNCwxMSBAQCBJbWFnZSogQ2FjaGVkSW1hZ2U6OmJyb2tlbkltYWdlKGZsb2F0IGRlCiAgICAg cmV0dXJuIGJyb2tlbkltYWdlTG9SZXM7CiB9CiAKK2Jvb2wgQ2FjaGVkSW1hZ2U6OndpbGxQYWlu dEJyb2tlbkltYWdlKCkgY29uc3QKK3sKKyAgICByZXR1cm4gZXJyb3JPY2N1cnJlZCgpICYmIG1f c2hvdWxkUGFpbnRCcm9rZW5JbWFnZTsKK30KKwogSW1hZ2UqIENhY2hlZEltYWdlOjppbWFnZSgp IGNvbnN0CiB7CiAgICAgQVNTRVJUKCFpc1B1cmdlYWJsZSgpKTsKSW5kZXg6IFNvdXJjZS9XZWJD b3JlL2xvYWRlci9jYWNoZS9DYWNoZWRJbWFnZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJD b3JlL2xvYWRlci9jYWNoZS9DYWNoZWRJbWFnZS5oCShyZXZpc2lvbiA5NDkwMCkKKysrIFNvdXJj ZS9XZWJDb3JlL2xvYWRlci9jYWNoZS9DYWNoZWRJbWFnZS5oCSh3b3JraW5nIGNvcHkpCkBAIC00 OCw2ICs0OCw3IEBAIHB1YmxpYzoKICAgICBib29sIGhhc0ltYWdlKCkgY29uc3QgeyByZXR1cm4g bV9pbWFnZS5nZXQoKTsgfQogCiAgICAgSW1hZ2UqIGJyb2tlbkltYWdlKGZsb2F0IGRldmljZVNj YWxlRmFjdG9yKSBjb25zdDsKKyAgICBib29sIHdpbGxQYWludEJyb2tlbkltYWdlKCkgY29uc3Q7 IAogCiAgICAgYm9vbCBjYW5SZW5kZXIoZmxvYXQgbXVsdGlwbGllcikgY29uc3QgeyByZXR1cm4g IWVycm9yT2NjdXJyZWQoKSAmJiAhaW1hZ2VTaXplKG11bHRpcGxpZXIpLmlzRW1wdHkoKTsgfQog CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVySW1hZ2UuY3BwCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJJbWFnZS5jcHAJKHJldmlzaW9u IDk0OTAxKQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckltYWdlLmNwcAkod29y a2luZyBjb3B5KQpAQCAtODQsNyArODQsMTEgQEAgSW50U2l6ZSBSZW5kZXJJbWFnZTo6aW1hZ2VT aXplRm9yRXJyb3IoQwogICAgIEFTU0VSVF9BUkcobmV3SW1hZ2UsIG5ld0ltYWdlKTsKICAgICBB U1NFUlRfQVJHKG5ld0ltYWdlLCBuZXdJbWFnZS0+aW1hZ2UoKSk7CiAKLSAgICBJbWFnZSogYnJv a2VuSW1hZ2UgPSBuZXdJbWFnZS0+YnJva2VuSW1hZ2UoUGFnZTo6ZGV2aWNlU2NhbGVGYWN0b3Io ZnJhbWUoKSkpOworICAgIEltYWdlKiBicm9rZW5JbWFnZTsKKyAgICBpZiAobmV3SW1hZ2UtPndp bGxQYWludEJyb2tlbkltYWdlKCkpCisgICAgICAgIGJyb2tlbkltYWdlID0gbmV3SW1hZ2UtPmJy b2tlbkltYWdlKFBhZ2U6OmRldmljZVNjYWxlRmFjdG9yKGZyYW1lKCkpKTsKKyAgICBlbHNlCisg ICAgICAgIGJyb2tlbkltYWdlID0gbmV3SW1hZ2UtPmltYWdlKCk7CiAKICAgICAvLyBpbWFnZVNp emUoKSByZXR1cm5zIDAgZm9yIHRoZSBlcnJvciBpbWFnZS4gV2UgbmVlZCB0aGUgdHJ1ZSBzaXpl IG9mIHRoZQogICAgIC8vIGVycm9yIGltYWdlLCBzbyB3ZSBoYXZlIHRvIGdldCBpdCBieSBncmFi YmluZyBpbWFnZSgpIGRpcmVjdGx5LgpJbmRleDogTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIExheW91dFRlc3RzL0NoYW5nZUxvZwkocmV2aXNpb24gOTQ5MTYpCisrKyBMYXlv dXRUZXN0cy9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxMiBAQAorMjAxMS0w OS0xMCAgQmV0aCBEYWtpbiAgPGJkYWtpbkBhcHBsZS5jb20+CisKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTY3ODk4CisgICAgICAgIFJFR1JFU1NJT04o cjk0OTAwKTogZmFzdC9pbWFnZXMvc3VwcG9ydC1icm9rZW4taW1hZ2UtZGVsZWdhdGUuaHRtbCBm YWlscyBvbiBNYWMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIHBsYXRmb3JtL21hYy9Ta2lwcGVkOgorCiAyMDExLTA5LTEwICBSeW9zdWtlIE5pd2Eg IDxybml3YUB3ZWJraXQub3JnPgogCiAgICAgICAgIFNraXAgZmFzdC9pbWFnZXMvc3VwcG9ydC1i cm9rZW4taW1hZ2UtZGVsZWdhdGUuaHRtbCB0ZW1wb3JhcmlseSBvbiBNYWMuCkluZGV4OiBMYXlv dXRUZXN0cy9wbGF0Zm9ybS9tYWMvU2tpcHBlZAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9w bGF0Zm9ybS9tYWMvU2tpcHBlZAkocmV2aXNpb24gOTQ5MTUpCisrKyBMYXlvdXRUZXN0cy9wbGF0 Zm9ybS9tYWMvU2tpcHBlZAkod29ya2luZyBjb3B5KQpAQCAtNDIwLDYgKzQyMCwzIEBAIGh0dHAv dGVzdHMvbG9hZGluZy9wcm9ncmVzcy1maW5pc2hlZC1jYWwKIAogIyBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Njc3MTYKIG1lZGlhL21lZGlhLWNvbnRyb2xzLWludmFs aWQtdXJsLmh0bWwKLQotIyBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 Njc4OTgKLWZhc3QvaW1hZ2VzL3N1cHBvcnQtYnJva2VuLWltYWdlLWRlbGVnYXRlLmh0bWwK