121455 2013-09-16 15:02:41 -0700 Bad ASSERT() in RasterShapeIntervals::firstIncludedIntervalY() 2013-09-17 09:45:27 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 giles_joplin giles_joplin commit-queue esprehn+autocc glenn kondapallykalyan oldest_to_newest 929943 0 giles_joplin 2013-09-16 15:02:41 -0700 The changes for https://bugs.webkit.org/show_bug.cgi?id=120211 included an over-constrained ASSERT() in RasterShapeIntervals::firstIncludedIntervalY(): bool RasterShapeIntervals::firstIncludedIntervalY(int minY, const IntSize& minSize, LayoutUnit& result) const { minY = std::max<int>(bounds().y(), minY); ASSERT(minY >= 0 && minY + minSize.height() < size()); ... } It's only necessary for the caller to ensure that minY is effectively < size(). Legitimate calls to firstIncluedIntervalY() may specify values of minY that are greater than size() - minSize.height(). 929964 1 211840 giles_joplin 2013-09-16 16:08:17 -0700 Created attachment 211840 Patch Fixed the bad ASSERT() in RasterShapeIntervals::firstIncludedIntervalY(). 930251 2 211840 darin 2013-09-17 08:58:41 -0700 Comment on attachment 211840 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211840&action=review > Source/WebCore/rendering/shapes/RasterShape.cpp:100 > - ASSERT(minY >= 0 && minY + minSize.height() < size()); > + ASSERT(minY >= 0 && minY < size()); Wouldn’t the right fix be to make the assertion "<=" rather than leaving out the height? 930257 3 giles_joplin 2013-09-17 09:18:28 -0700 (In reply to comment #2) > (From update of attachment 211840 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=211840&action=review > > > Source/WebCore/rendering/shapes/RasterShape.cpp:100 > > - ASSERT(minY >= 0 && minY + minSize.height() < size()); > > + ASSERT(minY >= 0 && minY < size()); > > Wouldn’t the right fix be to make the assertion "<=" rather than leaving out the height? If minY + minSize.height() extends beyond the bounds of the shape, then this function is just supposed to return false. And bounds().height() <= size(). The assertion is only supposed to insure that minY is in range. Thanks for the quick review! 930268 4 211840 commit-queue 2013-09-17 09:45:25 -0700 Comment on attachment 211840 Patch Clearing flags on attachment: 211840 Committed r155965: <http://trac.webkit.org/changeset/155965> 930269 5 commit-queue 2013-09-17 09:45:27 -0700 All reviewed patches have been landed. Closing bug. 211840 2013-09-16 16:08:17 -0700 2013-09-17 09:45:25 -0700 Patch tmp.patch text/plain 3396 giles_joplin ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA2Y2Q5MDk4Li5iODkwYjBjIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTMtMDkt MTYgIEhhbnMgTXVsbGVyICA8aG11bGxlckBhZG9iZS5jb20+CisKKyAgICAgICAgQmFkIEFTU0VS VCgpIGluIFJhc3RlclNoYXBlSW50ZXJ2YWxzOjpmaXJzdEluY2x1ZGVkSW50ZXJ2YWxZKCkKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyMTQ1NQorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFNpbXBsZSByZWdy ZXNzaW9uIHRlc3QsIGl0IGNyYXNoZXMgaW4gdGhlIGJhZCBBU1NFUlQoKS4KKworICAgICAgICAq IGZhc3Qvc2hhcGVzL3NoYXBlLWluc2lkZS9zaGFwZS1pbnNpZGUtZmlyc3QtZml0LWNyYXNoLWV4 cGVjdGVkLnR4dDogQWRkZWQuCisgICAgICAgICogZmFzdC9zaGFwZXMvc2hhcGUtaW5zaWRlL3No YXBlLWluc2lkZS1maXJzdC1maXQtY3Jhc2guaHRtbDogQWRkZWQuCisKIDIwMTMtMDktMTYgIEFs ZXhleSBQcm9za3VyeWFrb3YgIDxhcEBhcHBsZS5jb20+CiAKICAgICAgICAgRmxha3kgVGVzdDog c3ZnL2JhdGlrL21hc2tpbmcvbWFza1JlZ2lvbnMuc3ZnCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0 cy9mYXN0L3NoYXBlcy9zaGFwZS1pbnNpZGUvc2hhcGUtaW5zaWRlLWZpcnN0LWZpdC1jcmFzaC1l eHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9mYXN0L3NoYXBlcy9zaGFwZS1pbnNpZGUvc2hhcGUt aW5zaWRlLWZpcnN0LWZpdC1jcmFzaC1leHBlY3RlZC50eHQKbmV3IGZpbGUgbW9kZSAxMDA2NDQK aW5kZXggMDAwMDAwMC4uYTkzNmMyZgotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRlc3RzL2Zh c3Qvc2hhcGVzL3NoYXBlLWluc2lkZS9zaGFwZS1pbnNpZGUtZmlyc3QtZml0LWNyYXNoLWV4cGVj dGVkLnR4dApAQCAtMCwwICsxLDIgQEAKK1RoaXMgdGVzdCBzaG91bGQgbm90IGNyYXNoCitEb3du IHRoZSBSYWJiaXQtSG9sZQpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9zaGFwZXMvc2hh cGUtaW5zaWRlL3NoYXBlLWluc2lkZS1maXJzdC1maXQtY3Jhc2guaHRtbCBiL0xheW91dFRlc3Rz L2Zhc3Qvc2hhcGVzL3NoYXBlLWluc2lkZS9zaGFwZS1pbnNpZGUtZmlyc3QtZml0LWNyYXNoLmh0 bWwKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMC4uNDQ2OTY2NQotLS0gL2Rldi9u dWxsCisrKyBiL0xheW91dFRlc3RzL2Zhc3Qvc2hhcGVzL3NoYXBlLWluc2lkZS9zaGFwZS1pbnNp ZGUtZmlyc3QtZml0LWNyYXNoLmh0bWwKQEAgLTAsMCArMSwyNyBAQAorPCFET0NUWVBFIGh0bWw+ Cis8aHRtbD4KKzxoZWFkPgorPCEtLQorVGhpcyBpcyBhIHJlZ3Jlc3Npb24gdGVzdCBmb3IgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyMTQ1NQorLS0+Cis8c3R5bGU+ CisjaW1hZ2Utc2hhcGUgeworICAgIGNvbG9yOiBncmVlbjsKKyAgICB3aWR0aDogMzUwcHg7Cisg ICAgaGVpZ2h0OiA2NXB4OworICAgIGZvbnQtc2l6ZTogMjBweDsKKyAgICBsaW5lLWhlaWdodDog MjRweDsKKyAgICAtd2Via2l0LXNoYXBlLWluc2lkZTogdXJsKCJzdmctcmVjdDQwMC5zdmciKTsK KyAgICBiYWNrZ3JvdW5kLWltYWdlOiB1cmwoInN2Zy1yZWN0NDAwLnN2ZyIpOworICAgIGJhY2tn cm91bmQtcmVwZWF0OiBuby1yZXBlYXQ7Cit9Cis8L3N0eWxlPgorPGJvZHk+CisgIDxkaXY+VGhp cyB0ZXN0IHNob3VsZCBub3QgY3Jhc2g8L2Rpdj4KKyAgPGRpdiBpZD0iaW1hZ2Utc2hhcGUiPkRv d24gdGhlIFJhYmJpdC1Ib2xlPC9kaXY+CisgIDxzY3JpcHQ+CisgICAgaWYgKHdpbmRvdy50ZXN0 UnVubmVyKQorICAgICAgICB0ZXN0UnVubmVyLmR1bXBBc1RleHQoKTsKKyAgPC9zY3JpcHQ+Cis8 L2JvZHk+Cis8L2h0bWw+CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgYi9T b3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZGVmNzQyOC4uMDJmZDdkOCAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZwpAQCAtMSwzICsxLDE3IEBACisyMDEzLTA5LTE2ICBIYW5zIE11bGxlciAgPGhtdWxsZXJAYWRv YmUuY29tPgorCisgICAgICAgIEJhZCBBU1NFUlQoKSBpbiBSYXN0ZXJTaGFwZUludGVydmFsczo6 Zmlyc3RJbmNsdWRlZEludGVydmFsWSgpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMjE0NTUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBDb3JyZWN0ZWQgYSBiYWQgQVNTRVJUKCkgaW50cm9kdWNlZCBpbiBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTIwMjExLgorCisgICAgICAg IFRlc3Q6IGZhc3Qvc2hhcGVzL3NoYXBlLWluc2lkZS9zaGFwZS1pbnNpZGUtZmlyc3QtZml0LWNy YXNoLmh0bWwKKworICAgICAgICAqIHJlbmRlcmluZy9zaGFwZXMvUmFzdGVyU2hhcGUuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6UmFzdGVyU2hhcGVJbnRlcnZhbHM6OmZpcnN0SW5jbHVkZWRJbnRl cnZhbFkpOgorCiAyMDEzLTA5LTE2ICBCZW0gSm9uZXMtQmV5ICA8YmpvbmVzYmVAYWRvYmUuY29t PgogCiAgICAgICAgIE1ha2UgRmxvYXRpbmdPYmplY3RzIG93biBpdCdzIEZsb2F0aW5nT2JqZWN0 IGluc3RhbmNlcwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL3NoYXBlcy9S YXN0ZXJTaGFwZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvc2hhcGVzL1Jhc3RlclNo YXBlLmNwcAppbmRleCBkN2Y0MDYxLi44N2NkOThhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9yZW5kZXJpbmcvc2hhcGVzL1Jhc3RlclNoYXBlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9y ZW5kZXJpbmcvc2hhcGVzL1Jhc3RlclNoYXBlLmNwcApAQCAtOTcsNyArOTcsNyBAQCBib29sIFJh c3RlclNoYXBlSW50ZXJ2YWxzOjpmaXJzdEluY2x1ZGVkSW50ZXJ2YWxZKGludCBtaW5ZLCBjb25z dCBJbnRTaXplJiBtaW5TaQogewogICAgIG1pblkgPSBzdGQ6Om1heDxpbnQ+KGJvdW5kcygpLnko KSwgbWluWSk7CiAKLSAgICBBU1NFUlQobWluWSA+PSAwICYmIG1pblkgKyBtaW5TaXplLmhlaWdo dCgpIDwgc2l6ZSgpKTsKKyAgICBBU1NFUlQobWluWSA+PSAwICYmIG1pblkgPCBzaXplKCkpOwog CiAgICAgaWYgKG1pblNpemUuaXNFbXB0eSgpIHx8IG1pblNpemUud2lkdGgoKSA+IGJvdW5kcygp LndpZHRoKCkpCiAgICAgICAgIHJldHVybiBmYWxzZTsK