Bug 53016 – [GTK] Crash in some pages containing flash

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

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

[Carlos Garcia Campos]

Created attachment 79930 [details]
Patch to fix the crash

[Eric Seidel (no email)]

Adding gtk reviewers.

[Xan Lopez]

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.

[Martin Robinson]

Comment on attachment 79930 [details]
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

[Carlos Garcia Campos]

(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

[Carlos Garcia Campos]

Committed r76578: <http://trac.webkit.org/changeset/76578>

[WebKit Review Bot]

http://trac.webkit.org/changeset/76578 might have broken Leopard Intel Release (Tests)