53016 2011-01-24 08:52:02 -0800 [GTK] Crash in some pages containing flash 2011-01-25 01:38:57 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 cgarcia webkit-unassigned abarth eric evan mrobinson tony webkit.review.bot xan.lopez oldest_to_newest 339045 0 cgarcia 2011-01-24 08:52:02 -0800 The problem is that the flash plugin can produce X errors that the GDK default X error handler handles by aborting the process. It's reproducible with the formula1 website: http://www.f1.com 339051 1 79930 cgarcia 2011-01-24 09:00:41 -0800 Created attachment 79930 Patch to fix the crash 339221 2 eric 2011-01-24 13:15:13 -0800 Adding gtk reviewers. 339222 3 xan.lopez 2011-01-24 13:19:18 -0800 I went through this with Carlos before, it makes sense to me and seems everyone is doing something that ends up having the same effect. I'll let Martin give another r+, since this change is a bit hairy. 339226 4 79930 mrobinson 2011-01-24 13:24:16 -0800 Comment on attachment 79930 Patch to fix the crash View in context: https://bugs.webkit.org/attachment.cgi?id=79930&action=review Seems very reasonable to me! > Source/WebCore/plugins/gtk/PluginPackageGtk.cpp:113 > + "This probably reflects a bug in the flash plugin.\n" Here it should probably either say "in a plugin" or "in the Adobe Flash plugin" > Source/WebCore/plugins/gtk/PluginPackageGtk.cpp:157 > + if (!g_strcmp0(baseName.get(), "libflashplayer.so")) { g_str_equal here might be clearer, unless there's a possibility that basename may be null. > Source/WebCore/plugins/gtk/PluginPackageGtk.cpp:160 > + // custom error handler to show a warning when a X error happenswithout aborting. "happenswithout" -> happens without 339531 5 cgarcia 2011-01-25 00:38:15 -0800 (In reply to comment #4) > (From update of attachment 79930 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=79930&action=review > > Seems very reasonable to me! > > > Source/WebCore/plugins/gtk/PluginPackageGtk.cpp:113 > > + "This probably reflects a bug in the flash plugin.\n" > > Here it should probably either say "in a plugin" or "in the Adobe Flash plugin" We only do it for flash, so I'll use "in the Adobe Flash plugin" :-) > > Source/WebCore/plugins/gtk/PluginPackageGtk.cpp:157 > > + if (!g_strcmp0(baseName.get(), "libflashplayer.so")) { > > g_str_equal here might be clearer, unless there's a possibility that basename may be null. > > > Source/WebCore/plugins/gtk/PluginPackageGtk.cpp:160 > > + // custom error handler to show a warning when a X error happenswithout aborting. > > "happenswithout" -> happens without Thanks 339532 6 cgarcia 2011-01-25 00:44:14 -0800 Committed r76578: <http://trac.webkit.org/changeset/76578> 339549 7 webkit.review.bot 2011-01-25 01:38:57 -0800 http://trac.webkit.org/changeset/76578 might have broken Leopard Intel Release (Tests) 79930 2011-01-24 09:00:41 -0800 2011-01-25 00:44:40 -0800 Patch to fix the crash flash-crash.diff text/plain 2449 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA1NmQyNGIzLi43MjFkZDA2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkg QEAKKzIwMTEtMDEtMjQgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtHVEtd IENyYXNoIGluIHNvbWUgcGFnZXMgY29udGFpbmluZyBmbGFzaAorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTMwMTYKKworICAgICAgICBGbGFzaCBwbHVn aW4gY2FuIHByb2R1Y2UgWCBlcnJvcnMgdGhhdCBhcmUgaGFuZGxlZCBieSB0aGUgR0RLIFgKKyAg ICAgICAgZXJyb3IgaGFuZGxlciwgd2hpY2ggZXhpdHMgdGhlIHByb2Nlc3MuIFNpbmNlIHdlIGRv bid0IHdhbnQgdG8KKyAgICAgICAgY3Jhc2ggZHVlIHRvIGZsYXNoIGJ1Z3MsIHdlIGluc3RhbGwg YSBjdXN0b20gZXJyb3IgaGFuZGxlciB0byBzaG93CisgICAgICAgIGEgd2FybmluZyB3aGVuIGEg WCBlcnJvciBoYXBwZW5zIHdpdGhvdXQgYWJvcnRpbmcuCisKKyAgICAgICAgKiBwbHVnaW5zL2d0 ay9QbHVnaW5QYWNrYWdlR3RrLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OndlYmtpdGd0a1hFcnJv cik6CisgICAgICAgIChXZWJDb3JlOjpQbHVnaW5QYWNrYWdlOjpsb2FkKToKKwogMjAxMS0wMS0x OSAgRG9taW5pYyBDb29uZXkgIDxkb21pbmljY0Bnb29nbGUuY29tPgogCiAgICAgICAgIFJldmll d2VkIGJ5IEtlbnQgVGFtdXJhLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGx1Z2lucy9n dGsvUGx1Z2luUGFja2FnZUd0ay5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbHVnaW5zL2d0ay9QbHVn aW5QYWNrYWdlR3RrLmNwcAppbmRleCBhNzAyMjk2Li5hNmY1MjU4IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9wbHVnaW5zL2d0ay9QbHVnaW5QYWNrYWdlR3RrLmNwcAorKysgYi9Tb3VyY2Uv V2ViQ29yZS9wbHVnaW5zL2d0ay9QbHVnaW5QYWNrYWdlR3RrLmNwcApAQCAtMTA0LDYgKzEwNCwy MiBAQCBib29sIFBsdWdpblBhY2thZ2U6OmZldGNoSW5mbygpCiAjZW5kaWYKIH0KIAorI2lmIGRl ZmluZWQoWFBfVU5JWCkKK3N0YXRpYyBpbnQgd2Via2l0Z3RrWEVycm9yKERpc3BsYXkqIHhkaXNw bGF5LCBYRXJyb3JFdmVudCogZXJyb3IpCit7CisgICAgZ2NoYXIgZXJyb3JNZXNzYWdlWzY0XTsK KyAgICBYR2V0RXJyb3JUZXh0KHhkaXNwbGF5LCBlcnJvci0+ZXJyb3JfY29kZSwgZXJyb3JNZXNz YWdlLCA2Myk7CisgICAgZ193YXJuaW5nKCJUaGUgcHJvZ3JhbSAnJXMnIHJlY2VpdmVkIGFuIFgg V2luZG93IFN5c3RlbSBlcnJvci5cbiIKKyAgICAgICAgICAgICAgIlRoaXMgcHJvYmFibHkgcmVm bGVjdHMgYSBidWcgaW4gdGhlIGZsYXNoIHBsdWdpbi5cbiIKKyAgICAgICAgICAgICAgIlRoZSBl cnJvciB3YXMgJyVzJy5cbiIKKyAgICAgICAgICAgICAgIiAgKERldGFpbHM6IHNlcmlhbCAlbGQg ZXJyb3JfY29kZSAlZCByZXF1ZXN0X2NvZGUgJWQgbWlub3JfY29kZSAlZClcbiIsCisgICAgICAg ICAgICAgIGdfZ2V0X3ByZ25hbWUoKSwgZXJyb3JNZXNzYWdlLAorICAgICAgICAgICAgICBlcnJv ci0+c2VyaWFsLCBlcnJvci0+ZXJyb3JfY29kZSwKKyAgICAgICAgICAgICAgZXJyb3ItPnJlcXVl c3RfY29kZSwgZXJyb3ItPm1pbm9yX2NvZGUpOworICAgIHJldHVybiAwOworfQorI2VuZGlmCisK IGJvb2wgUGx1Z2luUGFja2FnZTo6bG9hZCgpCiB7CiAgICAgaWYgKG1faXNMb2FkZWQpIHsKQEAg LTEzNyw2ICsxNTMsMTUgQEAgYm9vbCBQbHVnaW5QYWNrYWdlOjpsb2FkKCkKIAogICAgIG1faXNM b2FkZWQgPSB0cnVlOwogCisjaWYgZGVmaW5lZChYUF9VTklYKQorICAgIGlmICghZ19zdHJjbXAw KGJhc2VOYW1lLmdldCgpLCAibGliZmxhc2hwbGF5ZXIuc28iKSkgeworICAgICAgICAvLyBGbGFz aCBwbHVnaW4gY2FuIHByb2R1Y2UgWCBlcnJvcnMgdGhhdCBhcmUgaGFuZGxlZCBieSB0aGUgR0RL IFggZXJyb3IgaGFuZGxlciwgd2hpY2gKKyAgICAgICAgLy8gZXhpdHMgdGhlIHByb2Nlc3MuIFNp bmNlIHdlIGRvbid0IHdhbnQgdG8gY3Jhc2ggZHVlIHRvIGZsYXNoIGJ1Z3MsIHdlIGluc3RhbGwg YQorICAgICAgICAvLyBjdXN0b20gZXJyb3IgaGFuZGxlciB0byBzaG93IGEgd2FybmluZyB3aGVu IGEgWCBlcnJvciBoYXBwZW5zd2l0aG91dCBhYm9ydGluZy4KKyAgICAgICAgWFNldEVycm9ySGFu ZGxlcih3ZWJraXRndGtYRXJyb3IpOworICAgIH0KKyNlbmRpZgorCiAgICAgTlBfSW5pdGlhbGl6 ZUZ1bmNQdHIgTlBfSW5pdGlhbGl6ZSA9IDA7CiAgICAgbV9OUFBfU2h1dGRvd24gPSAwOwogCg==