44628 2010-08-25 11:31:08 -0700 [Skia] canvas/philip/tests/2d.imageData.get.source.outside.html crashes test_shell in Chromium Linux debug 2013-04-09 12:53:31 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC Linux RESOLVED WONTFIX P2 Normal --- 20553 0 dominicc webkit-unassigned brettw darin eric schenney oldest_to_newest 269392 0 dominicc 2010-08-25 11:31:08 -0700 canvas/philip/tests/2d.imageData.get.source.outside.html asserts in test_shell in Chromium Linux debug: StackTrace::StackTrace() [0x590d5e] logging::LogMessage::~LogMessage() [0x536799] SkDebugf_FileLine() [0x4c06d5] SkBitmap::getAddr32() [0x468725] WebCore::getImageData<>() [0xd09daa] WebCore::ImageBuffer::getUnmultipliedImageData() [0xd09617] WebCore::CanvasRenderingContext2D::getImageData() [0x10fd10a] WebCore::CanvasRenderingContext2DInternal::getImageDataCallback() [0x130a68c] v8::internal::HandleApiCallHelper<>() [0x68cb60] v8::internal::Builtin_Impl_HandleApiCall() [0x68a795] v8::internal::Builtin_HandleApiCall() [0x68a76e] 0x7f72d75e61aa 269404 1 65452 dominicc 2010-08-25 11:46:07 -0700 Created attachment 65452 Patch 269429 2 65452 agl 2010-08-25 12:08:23 -0700 Comment on attachment 65452 Patch LGTM once the point below has been addressed. Note: I am not a WebKit Reviewer. You need a real review from someone else before landing this patch. > + In Chromium Skia getImageData don't retrieve the pointer into the > + canvas bitmap when there are no columns to copy, because this > + asserts when the source is outside the canvas bounds. This sentence doesn't make sense. 269489 3 65472 dominicc 2010-08-25 14:07:32 -0700 Created attachment 65472 Patch 270869 4 eric 2010-08-27 18:21:44 -0700 Who wrote chromium's getImageData? I suspect this fixes the crash, but is it the right fix? 271499 5 65472 senorblanco 2010-08-30 10:31:08 -0700 Comment on attachment 65472 Patch > WebCore/platform/graphics/skia/ImageBufferSkia.cpp:181 > + int numRows = numColumns > 0 ? endY - originY : 0; If this function is meant to do nothing in this case (and I'm guessing it is), I'd put an early-return below the computation of numColumns, rather than making the numRows computation more confusing. 271666 6 65964 dominicc 2010-08-30 15:03:57 -0700 Created attachment 65964 Patch with early return. 271674 7 65964 eric 2010-08-30 15:10:20 -0700 Comment on attachment 65964 Patch with early return. Looks fine, but why wouldn't we update test_expectations.txt in the same commit? 275392 8 65964 jamesr 2010-09-07 16:39:05 -0700 Comment on attachment 65964 Patch with early return. > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > + > + Fixes getImageData in the Chromium Skia port so that it doesn't > + retrieve the pointer into the canvas bitmap when there are no > + columns to copy. Retrieving the pointer asserts when the source is > + outside the canvas bounds. > + The double negative here is confusing. Can you please reword this sentence? Otherwise, looks great. canvas/philip/ is still marked as SKIP in the chromium test_expectations so there's not much point in updating it at this point. 279427 9 65964 eric 2010-09-15 00:14:59 -0700 Comment on attachment 65964 Patch with early return. Tests? 279433 10 dominicc 2010-09-15 00:29:19 -0700 I plan to baseline philip in a separate CL, then add updated expectations into this patch. But not actively working on this right now. 65452 2010-08-25 11:46:07 -0700 2010-08-25 14:07:28 -0700 Patch bug-44628-20100826034605.patch text/plain 1628 dominicc ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OGY2NjJlNzM0OTg4YTE5MzE3NjJjMjk0Y2UwNTZkODZjNDAyZmI4Mi4uMjllMWM1N2IzMGMwZTdl NWRjYzMyNTZlZWIwMDJjM2ZiYjBjZGRiMSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOSBAQAorMjAxMC0wOC0yNSAgRG9t aW5pYyBDb29uZXkgIDxkb21pbmljY0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEluIENocm9taXVtIFNraWEgZ2V0SW1hZ2VEYXRh IGRvbid0IHJldHJpZXZlIHRoZSBwb2ludGVyIGludG8gdGhlCisgICAgICAgIGNhbnZhcyBiaXRt YXAgd2hlbiB0aGVyZSBhcmUgbm8gY29sdW1ucyB0byBjb3B5LCBiZWNhdXNlIHRoaXMKKyAgICAg ICAgYXNzZXJ0cyB3aGVuIHRoZSBzb3VyY2UgaXMgb3V0c2lkZSB0aGUgY2FudmFzIGJvdW5kcy4K KworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDQ2MjgK KworICAgICAgICBUZXN0IGNhbnZhcy9waGlsaXAvdGVzdHMvMmQuaW1hZ2VEYXRhLmdldC5zb3Vy Y2Uub3V0c2lkZS5odG1sCisgICAgICAgIGNvdmVycyB0aGlzIGZpeC4gRXhwZWN0IHRoZSB0ZXN0 IHRvIHBhc3MgaW4gQ2hyb21pdW0gTGludXggZGVidWcuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9n cmFwaGljcy9za2lhL0ltYWdlQnVmZmVyU2tpYS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpnZXRJ bWFnZURhdGEpOgorCiAyMDEwLTA4LTI1ICBTaW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBw bGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhbiBCZXJuc3RlaW4uCmRpZmYgLS1naXQg YS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvSW1hZ2VCdWZmZXJTa2lhLmNwcCBiL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9JbWFnZUJ1ZmZlclNraWEuY3BwCmluZGV4IDk0 MDM0MDYzMDBhN2ZlNzlmZTA0ZmUyMWNmMTQ5NTVlNzJhMjU5ZGIuLmUyZGI2ODU2ZmFmNTVkN2Nm NzYzNGQ1ODJlZDA1N2ZkMjVhZTQ1NTMgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3Mvc2tpYS9JbWFnZUJ1ZmZlclNraWEuY3BwCisrKyBiL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3Mvc2tpYS9JbWFnZUJ1ZmZlclNraWEuY3BwCkBAIC0xNzgsNyArMTc4LDcgQEAgUGFzc1Jl ZlB0cjxJbWFnZURhdGE+IGdldEltYWdlRGF0YShjb25zdCBJbnRSZWN0JiByZWN0LCBjb25zdCBT a0JpdG1hcCYgYml0bWFwLAogICAgIGludCBlbmRZID0gcmVjdC55KCkgKyByZWN0LmhlaWdodCgp OwogICAgIGlmIChlbmRZID4gc2l6ZS5oZWlnaHQoKSkKICAgICAgICAgZW5kWSA9IHNpemUuaGVp Z2h0KCk7Ci0gICAgaW50IG51bVJvd3MgPSBlbmRZIC0gb3JpZ2luWTsKKyAgICBpbnQgbnVtUm93 cyA9IG51bUNvbHVtbnMgPiAwID8gZW5kWSAtIG9yaWdpblkgOiAwOwogCiAgICAgQVNTRVJUKGJp dG1hcC5jb25maWcoKSA9PSBTa0JpdG1hcDo6a0FSR0JfODg4OF9Db25maWcpOwogICAgIFNrQXV0 b0xvY2tQaXhlbHMgYml0bWFwTG9jayhiaXRtYXApOwo= 65472 2010-08-25 14:07:32 -0700 2010-08-30 15:03:52 -0700 Patch bug-44628-20100826060730.patch text/plain 1675 dominicc ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OGY2NjJlNzM0OTg4YTE5MzE3NjJjMjk0Y2UwNTZkODZjNDAyZmI4Mi4uMGJhZjc0ZTljOWQ5OTc5 N2NhNWM4ZDQ4NGQ4MzA1N2E2MWQxMzI4ZCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMCBAQAorMjAxMC0wOC0yNSAgRG9t aW5pYyBDb29uZXkgIDxkb21pbmljY0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEZpeGVzIGdldEltYWdlRGF0YSBpbiB0aGUgQ2hy b21pdW0gU2tpYSBwb3J0IHNvIHRoYXQgaXQgZG9lc24ndAorICAgICAgICByZXRyaWV2ZSB0aGUg cG9pbnRlciBpbnRvIHRoZSBjYW52YXMgYml0bWFwIHdoZW4gdGhlcmUgYXJlIG5vCisgICAgICAg IGNvbHVtbnMgdG8gY29weS4gUmV0cmlldmluZyB0aGUgcG9pbnRlciBhc3NlcnRzIHdoZW4gdGhl IHNvdXJjZSBpcworICAgICAgICBvdXRzaWRlIHRoZSBjYW52YXMgYm91bmRzLgorCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00NDYyOAorCisgICAgICAg IFRlc3QgY2FudmFzL3BoaWxpcC90ZXN0cy8yZC5pbWFnZURhdGEuZ2V0LnNvdXJjZS5vdXRzaWRl Lmh0bWwKKyAgICAgICAgY292ZXJzIHRoaXMgZml4LiBFeHBlY3QgdGhlIHRlc3QgdG8gcGFzcyBp biBDaHJvbWl1bSBMaW51eCBkZWJ1Zy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3Nr aWEvSW1hZ2VCdWZmZXJTa2lhLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmdldEltYWdlRGF0YSk6 CisKIDIwMTAtMDgtMjUgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBhcHBsZS5jb20+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgRGFuIEJlcm5zdGVpbi4KZGlmZiAtLWdpdCBhL1dlYkNvcmUv cGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9JbWFnZUJ1ZmZlclNraWEuY3BwIGIvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9za2lhL0ltYWdlQnVmZmVyU2tpYS5jcHAKaW5kZXggOTQwMzQwNjMwMGE3 ZmU3OWZlMDRmZTIxY2YxNDk1NWU3MmEyNTlkYi4uZTJkYjY4NTZmYWY1NWQ3Y2Y3NjM0ZDU4MmVk MDU3ZmQyNWFlNDU1MyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9za2lh L0ltYWdlQnVmZmVyU2tpYS5jcHAKKysrIGIvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9za2lh L0ltYWdlQnVmZmVyU2tpYS5jcHAKQEAgLTE3OCw3ICsxNzgsNyBAQCBQYXNzUmVmUHRyPEltYWdl RGF0YT4gZ2V0SW1hZ2VEYXRhKGNvbnN0IEludFJlY3QmIHJlY3QsIGNvbnN0IFNrQml0bWFwJiBi aXRtYXAsCiAgICAgaW50IGVuZFkgPSByZWN0LnkoKSArIHJlY3QuaGVpZ2h0KCk7CiAgICAgaWYg KGVuZFkgPiBzaXplLmhlaWdodCgpKQogICAgICAgICBlbmRZID0gc2l6ZS5oZWlnaHQoKTsKLSAg ICBpbnQgbnVtUm93cyA9IGVuZFkgLSBvcmlnaW5ZOworICAgIGludCBudW1Sb3dzID0gbnVtQ29s dW1ucyA+IDAgPyBlbmRZIC0gb3JpZ2luWSA6IDA7CiAKICAgICBBU1NFUlQoYml0bWFwLmNvbmZp ZygpID09IFNrQml0bWFwOjprQVJHQl84ODg4X0NvbmZpZyk7CiAgICAgU2tBdXRvTG9ja1BpeGVs cyBiaXRtYXBMb2NrKGJpdG1hcCk7Cg== 65964 2010-08-30 15:03:57 -0700 2010-09-15 00:14:59 -0700 Patch with early return. bug-44628-20100831070355.patch text/plain 1637 dominicc ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OGY2NjJlNzM0OTg4YTE5MzE3NjJjMjk0Y2UwNTZkODZjNDAyZmI4Mi4uMGJhZjc0ZTljOWQ5OTc5 N2NhNWM4ZDQ4NGQ4MzA1N2E2MWQxMzI4ZCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMCBAQAorMjAxMC0wOC0yNSAgRG9t aW5pYyBDb29uZXkgIDxkb21pbmljY0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEZpeGVzIGdldEltYWdlRGF0YSBpbiB0aGUgQ2hy b21pdW0gU2tpYSBwb3J0IHNvIHRoYXQgaXQgZG9lc24ndAorICAgICAgICByZXRyaWV2ZSB0aGUg cG9pbnRlciBpbnRvIHRoZSBjYW52YXMgYml0bWFwIHdoZW4gdGhlcmUgYXJlIG5vCisgICAgICAg IGNvbHVtbnMgdG8gY29weS4gUmV0cmlldmluZyB0aGUgcG9pbnRlciBhc3NlcnRzIHdoZW4gdGhl IHNvdXJjZSBpcworICAgICAgICBvdXRzaWRlIHRoZSBjYW52YXMgYm91bmRzLgorCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00NDYyOAorCisgICAgICAg IFRlc3QgY2FudmFzL3BoaWxpcC90ZXN0cy8yZC5pbWFnZURhdGEuZ2V0LnNvdXJjZS5vdXRzaWRl Lmh0bWwKKyAgICAgICAgY292ZXJzIHRoaXMgZml4LiBFeHBlY3QgdGhlIHRlc3QgdG8gcGFzcyBp biBDaHJvbWl1bSBMaW51eCBkZWJ1Zy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3Nr aWEvSW1hZ2VCdWZmZXJTa2lhLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmdldEltYWdlRGF0YSk6 CisKIDIwMTAtMDgtMjUgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBhcHBsZS5jb20+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgRGFuIEJlcm5zdGVpbi4KZGlmZiAtLWdpdCBhL1dlYkNvcmUv cGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9JbWFnZUJ1ZmZlclNraWEuY3BwIGIvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9za2lhL0ltYWdlQnVmZmVyU2tpYS5jcHAKaW5kZXggOTQwMzQwNjMwMGE3 ZmU3OWZlMDRmZTIxY2YxNDk1NWU3MmEyNTlkYi4uYzk0MjgxODE0OGUwMDg2NjcwNzIyMDYwN2Qy YjczOTJkYmMyYTAwYSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9za2lh L0ltYWdlQnVmZmVyU2tpYS5jcHAKKysrIGIvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9za2lh L0ltYWdlQnVmZmVyU2tpYS5jcHAKQEAgLTE4MCw2ICsxODAsOSBAQCBQYXNzUmVmUHRyPEltYWdl RGF0YT4gZ2V0SW1hZ2VEYXRhKGNvbnN0IEludFJlY3QmIHJlY3QsIGNvbnN0IFNrQml0bWFwJiBi aXRtYXAsCiAgICAgICAgIGVuZFkgPSBzaXplLmhlaWdodCgpOwogICAgIGludCBudW1Sb3dzID0g ZW5kWSAtIG9yaWdpblk7CiAKKyAgICBpZiAobnVtQ29sdW1ucyA8PSAwIHx8IG51bVJvd3MgPD0g MCkKKyAgICAgICAgcmV0dXJuIHJlc3VsdDsgLy8gbm8gcGl4ZWxzIHRvIGNvcHkKKwogICAgIEFT U0VSVChiaXRtYXAuY29uZmlnKCkgPT0gU2tCaXRtYXA6OmtBUkdCXzg4ODhfQ29uZmlnKTsKICAg ICBTa0F1dG9Mb2NrUGl4ZWxzIGJpdG1hcExvY2soYml0bWFwKTsKIAo=