156690 2016-04-18 02:32:20 -0700 REGRESSION(r198782): SHOULD NEVER BE REACHED failure in ImageSource::setData since r198782 2016-04-22 13:09:43 -0700 1 1 1 Unclassified WebKit Platform WebKit Local Build Unspecified Unspecified RESOLVED FIXED Gtk, Regression P2 Normal --- 155322 1 cgarcia webkit-unassigned bugs-noreply sabouhallawa simon.fraser oldest_to_newest 1184621 0 cgarcia 2016-04-18 02:32:20 -0700 The assertion is wrong, because it assumes that ImageDecoder::create() always returns a valid pointer, which is only true for the CG implementation. The non CG implementation can return nullptr if there isn't enough data to figure out the image format or if the image format is not supported. This is causing several crashes in the debug bots. 1184622 1 276626 cgarcia 2016-04-18 02:34:59 -0700 Created attachment 276626 Patch 1184720 2 276626 simon.fraser 2016-04-18 10:28:50 -0700 Comment on attachment 276626 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276626&action=review > Source/WebCore/platform/graphics/ImageSource.cpp:74 > +bool ImageSource::tryEnsureDecoderIfNeeded(const SharedBuffer& data) "try ensure" is not a good name, and "if needed" is more like "if possible". 1184934 3 cgarcia 2016-04-18 23:36:44 -0700 (In reply to comment #2) > Comment on attachment 276626 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=276626&action=review > > > Source/WebCore/platform/graphics/ImageSource.cpp:74 > > +bool ImageSource::tryEnsureDecoderIfNeeded(const SharedBuffer& data) > > "try ensure" is not a good name, and "if needed" is more like "if possible". My initial idea was to use try create instead, but normally try create methods return the created object or nullptr, but this one ensures it exists. So that's why I kept the ensure part. The "if possible" is already covered by try, if needed is not because ImageDecoder::create can fail, but because there's an early return in case the decoder has already been created. I added the if needed because I initially called it try create, but I agree the ensure also covers if needed. In any case, the ensure method is only called by setData() so I wonder if we really need an ensure method after all. 1184935 4 cgarcia 2016-04-18 23:37:44 -0700 Btw, this is causing a lot of crashes in a our debug bot, so I would like tho fix this soon, to avoid the early exit. 1184959 5 276704 cgarcia 2016-04-19 00:56:00 -0700 Created attachment 276704 Updated patch I think the code is simpler and clearer without the ensure method. In the end we can't ensure the decoder is created. 1185055 6 276704 mcatanzaro 2016-04-19 10:01:03 -0700 Comment on attachment 276704 Updated patch Please wait one day in case Said has comments. 1185351 7 cgarcia 2016-04-19 23:02:17 -0700 Committed r199764: <http://trac.webkit.org/changeset/199764> 276626 2016-04-18 02:34:59 -0700 2016-04-19 00:56:00 -0700 Patch wcore-image-decoder-crash.diff text/plain 3181 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA2ZmY2MzQ3NS4uNTg3YTNlNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDI0 IEBACiAyMDE2LTA0LTE4ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNv bT4KIAorICAgICAgICBSRUdSRVNTSU9OKHIxOTg3ODIpOiBTSE9VTEQgTkVWRVIgQkUgUkVBQ0hF RCBmYWlsdXJlIGluIEltYWdlU291cmNlOjpzZXREYXRhIHNpbmNlIHIxOTg3ODIKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1NjY5MAorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBhc3NlcnRpb24gaXMg d3JvbmcsIGJlY2F1c2UgaXQgYXNzdW1lcyB0aGF0IEltYWdlRGVjb2Rlcjo6Y3JlYXRlKCkgYWx3 YXlzIHJldHVybnMgYSB2YWxpZCBwb2ludGVyLCB3aGljaCBpcworICAgICAgICBvbmx5IHRydWUg Zm9yIHRoZSBDRyBpbXBsZW1lbnRhdGlvbi4gVGhlIG5vbiBDRyBpbXBsZW1lbnRhdGlvbiBjYW4g cmV0dXJuIG51bGxwdHIgaWYgdGhlcmUgaXNuJ3QgZW5vdWdoIGRhdGEgdG8KKyAgICAgICAgZmln dXJlIG91dCB0aGUgaW1hZ2UgZm9ybWF0IG9yIGlmIHRoZSBpbWFnZSBmb3JtYXQgaXMgbm90IHN1 cHBvcnRlZC4gVGhpcyBpcyBjYXVzaW5nIHNldmVyYWwgY3Jhc2hlcyBpbiB0aGUKKyAgICAgICAg ZGVidWcgYm90cy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL0ltYWdlU291cmNlLmNw cDoKKyAgICAgICAgKFdlYkNvcmU6OkltYWdlU291cmNlOjp0cnlFbnN1cmVEZWNvZGVySWZOZWVk ZWQpOiBSZW5hbWVkIHRvIG1ha2UgaXQgY2xlYXIgdGhhdCB0aGUgZGVjb2RlciBjcmVhdGlvbiBj YW4gZmFpbC4KKyAgICAgICAgKFdlYkNvcmU6OkltYWdlU291cmNlOjpzZXREYXRhKTogUmVtb3Zl IHRoZSBpbnZhbGlkIEFTU0VSVCBhbmQgcmV0dXJuIGVhcmx5IGlmIHRyeUVuc3VyZURlY29kZXJJ Zk5lZWRlZCgpCisgICAgICAgIHJldHVybnMgZmFsc2UuCisgICAgICAgIChXZWJDb3JlOjpJbWFn ZVNvdXJjZTo6ZW5zdXJlRGVjb2RlcklzQ3JlYXRlZCk6IERlbGV0ZWQuCisgICAgICAgICogcGxh dGZvcm0vZ3JhcGhpY3MvSW1hZ2VTb3VyY2UuaDoKKworMjAxNi0wNC0xOCAgQ2FybG9zIEdhcmNp YSBDYW1wb3MgIDxjZ2FyY2lhQGlnYWxpYS5jb20+CisKICAgICAgICAgW0dUS10gTWVudSBsaXN0 IGJ1dHRvbiBkb2Vzbid0IHVzZSB0aGUgdGV4dCBjb2xvciBmcm9tIHRoZSB0aGVtZQogICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTE4MjM0CiAKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0ltYWdlU291cmNlLmNwcCBi L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0ltYWdlU291cmNlLmNwcAppbmRleCA3 ZWQ5ZmQxLi4yNzJmYjVjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9JbWFnZVNvdXJjZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvSW1hZ2VTb3VyY2UuY3BwCkBAIC03MSwyOCArNzEsMjQgQEAgdm9pZCBJbWFnZVNvdXJjZTo6 Y2xlYXIoYm9vbCBkZXN0cm95QWxsRnJhbWVzLCBzaXplX3QgY2xlYXJCZWZvcmVGcmFtZSwgU2hh cmVkQnUKICAgICAgICAgc2V0RGF0YShkYXRhLCBhbGxEYXRhUmVjZWl2ZWQpOwogfQogCi12b2lk IEltYWdlU291cmNlOjplbnN1cmVEZWNvZGVySXNDcmVhdGVkKGNvbnN0IFNoYXJlZEJ1ZmZlciYg ZGF0YSkKK2Jvb2wgSW1hZ2VTb3VyY2U6OnRyeUVuc3VyZURlY29kZXJJZk5lZWRlZChjb25zdCBT aGFyZWRCdWZmZXImIGRhdGEpCiB7CiAgICAgaWYgKGluaXRpYWxpemVkKCkpCi0gICAgICAgIHJl dHVybjsKLSAgICAKKyAgICAgICAgcmV0dXJuIHRydWU7CisKICAgICBtX2RlY29kZXIgPSBJbWFn ZURlY29kZXI6OmNyZWF0ZShkYXRhLCBtX2FscGhhT3B0aW9uLCBtX2dhbW1hQW5kQ29sb3JQcm9m aWxlT3B0aW9uKTsKKyAgICByZXR1cm4gaW5pdGlhbGl6ZWQoKTsKIH0KIAogdm9pZCBJbWFnZVNv dXJjZTo6c2V0RGF0YShTaGFyZWRCdWZmZXIqIGRhdGEsIGJvb2wgYWxsRGF0YVJlY2VpdmVkKQog ewogICAgIGlmICghZGF0YSkKICAgICAgICAgcmV0dXJuOwotICAgIAotICAgIGVuc3VyZURlY29k ZXJJc0NyZWF0ZWQoKmRhdGEpOwotICAgIAotICAgIGlmICghaW5pdGlhbGl6ZWQoKSkgewotICAg ICAgICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsKKworICAgIGlmICghdHJ5RW5zdXJlRGVjb2Rlcklm TmVlZGVkKCpkYXRhKSkKICAgICAgICAgcmV0dXJuOwotICAgIH0KIAotICAgIGlmIChtX2RlY29k ZXIpCi0gICAgICAgIG1fZGVjb2Rlci0+c2V0RGF0YSgqZGF0YSwgYWxsRGF0YVJlY2VpdmVkKTsK KyAgICBtX2RlY29kZXItPnNldERhdGEoKmRhdGEsIGFsbERhdGFSZWNlaXZlZCk7CiB9CiAKIFN1 YnNhbXBsaW5nTGV2ZWwgSW1hZ2VTb3VyY2U6OmNhbGN1bGF0ZU1heGltdW1TdWJzYW1wbGluZ0xl dmVsKCkgY29uc3QKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L0ltYWdlU291cmNlLmggYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9JbWFnZVNv dXJjZS5oCmluZGV4IGE4OTgwNzYuLjMyZDE4ZmIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL0ltYWdlU291cmNlLmgKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vZ3JhcGhpY3MvSW1hZ2VTb3VyY2UuaApAQCAtMTQ0LDcgKzE0NCw3IEBAIHB1YmxpYzoK ICAgICAKIHByaXZhdGU6CiAgICAgdm9pZCBjbGVhckZyYW1lQnVmZmVyQ2FjaGUoc2l6ZV90KTsK LSAgICB2b2lkIGVuc3VyZURlY29kZXJJc0NyZWF0ZWQoY29uc3QgU2hhcmVkQnVmZmVyJik7Cisg ICAgYm9vbCB0cnlFbnN1cmVEZWNvZGVySWZOZWVkZWQoY29uc3QgU2hhcmVkQnVmZmVyJik7CiAg ICAgU3Vic2FtcGxpbmdMZXZlbCBjYWxjdWxhdGVNYXhpbXVtU3Vic2FtcGxpbmdMZXZlbCgpIGNv bnN0OwogICAgIHZvaWQgZHVtcChUZXh0U3RyZWFtJikgY29uc3Q7CiAgICAgCg== 276704 2016-04-19 00:56:00 -0700 2016-04-19 10:01:03 -0700 Updated patch wcore-image-source-crash.diff text/plain 2951 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA3NTVmZjBiLi4yMDk5NjFiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAg QEAKKzIwMTYtMDQtMTkgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFJFR1JFU1NJT04ocjE5ODc4Mik6IFNIT1VMRCBORVZFUiBCRSBSRUFDSEVE IGZhaWx1cmUgaW4gSW1hZ2VTb3VyY2U6OnNldERhdGEgc2luY2UgcjE5ODc4MgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU2NjkwCisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlIGFzc2VydGlvbiBpcyB3 cm9uZywgYmVjYXVzZSBpdCBhc3N1bWVzIHRoYXQgSW1hZ2VEZWNvZGVyOjpjcmVhdGUoKSBhbHdh eXMgcmV0dXJucyBhIHZhbGlkIHBvaW50ZXIsIHdoaWNoIGlzCisgICAgICAgIG9ubHkgdHJ1ZSBm b3IgdGhlIENHIGltcGxlbWVudGF0aW9uLiBUaGUgbm9uIENHIGltcGxlbWVudGF0aW9uIGNhbiBy ZXR1cm4gbnVsbHB0ciBpZiB0aGVyZSBpc24ndCBlbm91Z2ggZGF0YSB0bworICAgICAgICBmaWd1 cmUgb3V0IHRoZSBpbWFnZSBmb3JtYXQgb3IgaWYgdGhlIGltYWdlIGZvcm1hdCBpcyBub3Qgc3Vw cG9ydGVkLiBUaGlzIGlzIGNhdXNpbmcgc2V2ZXJhbCBjcmFzaGVzIGluIHRoZQorICAgICAgICBk ZWJ1ZyBib3RzLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvSW1hZ2VTb3VyY2UuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6SW1hZ2VTb3VyY2U6OnNldERhdGEpOiBSZW1vdmUgdGhlIGlu dmFsaWQgQVNTRVJUIGFuZCByZXR1cm4gZWFybHkgaWYgd2UgZmFpbCB0byBjcmVhdGUgdGhlIGRl Y29kZXIuCisgICAgICAgIChXZWJDb3JlOjpJbWFnZVNvdXJjZTo6ZW5zdXJlRGVjb2RlcklzQ3Jl YXRlZCk6IERlbGV0ZWQuCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvSW1hZ2VTb3VyY2Uu aDoKKwogMjAxNi0wNC0xOCAgQnJhZHkgRWlkc29uICA8YmVpZHNvbkBhcHBsZS5jb20+CiAKICAg ICAgICAgTW9kZXJuIElEQiAoQmxvYiBzdXBwb3J0KTogV2hlbiByZWFkaW5nIEJsb2JzIGZyb20g dGhlIGRhdGFiYXNlLCBncmFudCB0aGUgTmV0d29ya2luZyBwcm9jZXNzIHNhbmRib3ggYWNjZXNz IHRvIHRoZSBmaWxlcy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL0ltYWdlU291cmNlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0lt YWdlU291cmNlLmNwcAppbmRleCA3ZWQ5ZmQxLi43ZDM0ZjZkIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9JbWFnZVNvdXJjZS5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvSW1hZ2VTb3VyY2UuY3BwCkBAIC03MSwyOCArNzEsMTgg QEAgdm9pZCBJbWFnZVNvdXJjZTo6Y2xlYXIoYm9vbCBkZXN0cm95QWxsRnJhbWVzLCBzaXplX3Qg Y2xlYXJCZWZvcmVGcmFtZSwgU2hhcmVkQnUKICAgICAgICAgc2V0RGF0YShkYXRhLCBhbGxEYXRh UmVjZWl2ZWQpOwogfQogCi12b2lkIEltYWdlU291cmNlOjplbnN1cmVEZWNvZGVySXNDcmVhdGVk KGNvbnN0IFNoYXJlZEJ1ZmZlciYgZGF0YSkKLXsKLSAgICBpZiAoaW5pdGlhbGl6ZWQoKSkKLSAg ICAgICAgcmV0dXJuOwotICAgIAotICAgIG1fZGVjb2RlciA9IEltYWdlRGVjb2Rlcjo6Y3JlYXRl KGRhdGEsIG1fYWxwaGFPcHRpb24sIG1fZ2FtbWFBbmRDb2xvclByb2ZpbGVPcHRpb24pOwotfQot CiB2b2lkIEltYWdlU291cmNlOjpzZXREYXRhKFNoYXJlZEJ1ZmZlciogZGF0YSwgYm9vbCBhbGxE YXRhUmVjZWl2ZWQpCiB7CiAgICAgaWYgKCFkYXRhKQogICAgICAgICByZXR1cm47Ci0gICAgCi0g ICAgZW5zdXJlRGVjb2RlcklzQ3JlYXRlZCgqZGF0YSk7Ci0gICAgCisKICAgICBpZiAoIWluaXRp YWxpemVkKCkpIHsKLSAgICAgICAgQVNTRVJUX05PVF9SRUFDSEVEKCk7Ci0gICAgICAgIHJldHVy bjsKKyAgICAgICAgbV9kZWNvZGVyID0gSW1hZ2VEZWNvZGVyOjpjcmVhdGUoKmRhdGEsIG1fYWxw aGFPcHRpb24sIG1fZ2FtbWFBbmRDb2xvclByb2ZpbGVPcHRpb24pOworICAgICAgICBpZiAoIW1f ZGVjb2RlcikKKyAgICAgICAgICAgIHJldHVybjsKICAgICB9CiAKLSAgICBpZiAobV9kZWNvZGVy KQotICAgICAgICBtX2RlY29kZXItPnNldERhdGEoKmRhdGEsIGFsbERhdGFSZWNlaXZlZCk7Cisg ICAgbV9kZWNvZGVyLT5zZXREYXRhKCpkYXRhLCBhbGxEYXRhUmVjZWl2ZWQpOwogfQogCiBTdWJz YW1wbGluZ0xldmVsIEltYWdlU291cmNlOjpjYWxjdWxhdGVNYXhpbXVtU3Vic2FtcGxpbmdMZXZl bCgpIGNvbnN0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9J bWFnZVNvdXJjZS5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvSW1hZ2VTb3Vy Y2UuaAppbmRleCBhODk4MDc2Li40MDljYjRlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9JbWFnZVNvdXJjZS5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL0ltYWdlU291cmNlLmgKQEAgLTE0NCw3ICsxNDQsNiBAQCBwdWJsaWM6CiAg ICAgCiBwcml2YXRlOgogICAgIHZvaWQgY2xlYXJGcmFtZUJ1ZmZlckNhY2hlKHNpemVfdCk7Ci0g ICAgdm9pZCBlbnN1cmVEZWNvZGVySXNDcmVhdGVkKGNvbnN0IFNoYXJlZEJ1ZmZlciYpOwogICAg IFN1YnNhbXBsaW5nTGV2ZWwgY2FsY3VsYXRlTWF4aW11bVN1YnNhbXBsaW5nTGV2ZWwoKSBjb25z dDsKICAgICB2b2lkIGR1bXAoVGV4dFN0cmVhbSYpIGNvbnN0OwogICAgIAo=