Bug 44324 – [Qt] Initialize GDK before loading plugins

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Andreas Kling]

Adobe's Flash plugin has a tendency to crash and freeze here & there.
We can alleviate some of this by making sure GDK is initialized before loading the plugin.
This would replace the section calling gtk_init() in PluginPackageQt.

[Andreas Kling]

Created attachment 64931 [details]
Proposed patch

[WebKit Review Bot]

Attachment 64931 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/plugins/qt/PluginPackageQt.cpp:107:  gdk_init_check is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Girish Ramakrishnan]

LGTM other than the style fixes.

[Andreas Kling]

Comment on attachment 64931 [details]
Proposed patch

Clearing flags on attachment: 64931

Committed r65728: <http://trac.webkit.org/changeset/65728>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.

[Dawit A.]

Now I get more frequent and random flash related crashes than before. Why was the change necessary ?

[Andreas Kling]

(In reply to comment #6)
> Now I get more frequent and random flash related crashes than before. Why was the change necessary ?

Derp! It was freezing immediately when going to e.g http://translate.google.no/ and typing something in the text box. A number of people confirmed that this fixed it for them.

Problem was that we couldn't resolve "gtk_init" from the loaded plugin. The suggestion to use gdk_init() instead of gtk_init() was from Girish.

[Girish Ramakrishnan]

Mmm, I don't understand how this patch solves translate.google.no freeze.

The initial code invoked gtk_init. This presumably solves the problem that Flash assumes that gtk_init has already been called by the browser.

The fix here was made for Flash that was loaded through nspluginwraper. nspluginwrapper does not depend on gtk or gdk and hence code that we had didn't call gtk_init. We made it now call gdk_init and some problem is solved (I don't know what exactly it solved other than translate.google.no freezing).

I checked nspluginwrapper code and the thing is it does't use gtk or gdk. It loads Flash in a separate process and communicates using IPC. If anything, we should call gtk/gdk_init in that _new_ process. How does calling gtk/gdk_init in the browser help here? Obviously, it seems to help since it seems to fix the freeze but I fail to see how :)

[Dawit A.]

(In reply to comment #8)
> Mmm, I don't understand how this patch solves translate.google.no freeze.

Neither do I... Specially since I tried that site with the previous fix many times and in different ways and I still was not able to get it to freeze on me.

> The initial code invoked gtk_init. This presumably solves the problem that Flash assumes that gtk_init has already been called by the browser.

Actually without that patch the new 10.1 series of flash player plugins from Adobe simply crash when used from a non-Gtk application. You can see the gory details in http://webkit.org/b/40567. This however is not the first time Adobe's releases have caused this particular issue. There were similar problems in some versions of of previous releases as well. That is why there are workarounds rendering engines and browsers that do not rely on Gtk, e.g. khtml & opera.

> The fix here was made for Flash that was loaded through nspluginwraper. nspluginwrapper does not depend on gtk or gdk and hence code that we had 
> didn't call gtk_init. We made it now call gdk_init and some problem is solved (I don't know what exactly it solved other than translate.google.no 
> freezing).
>
> I checked nspluginwrapper code and the thing is it does't use gtk or gdk. It loads Flash in a separate process and communicates using IPC. If 
> anything, we should call gtk/gdk_init in that _new_ process. How does calling gtk/gdk_init in the browser help here? Obviously, it seems to help since 
> it seems to fix the freeze but I fail to see how :)

Ahhh... I now see where the entire misunderstanding is coming from. You guys are looking at this issue from the prespective of using the nspluginwrapper which is completely different from using the Adobe flash player plugin directly! Obviously, in the case of the nspluginwrapper the attempt to resolve the "gtk_init" symbol from the library will fail unless of course it uses Gtk, which as you stated above is not the case. 

So then can we safely say that the problem with the original patch (40567) is its failure to limit the impact of the workaround to Adobe's flash player plugin only ? If that is the case, then the fix is really simple instead of what you guys did here. In fact, in addition to nspluginwrapper, this workaround should be unnecessary for other flash players like gnash and lightspark. I guess I will go ahead and open a new ticket and post my patch to address the issue discussed here.

[Andreas Kling]

*** Bug 35684 has been marked as a duplicate of this bug. ***

[Ademar Reis]

Although there's some dispute on the real need of this patch, it's now part of trunk and is still marked as a candidate for webkit-2.0 (it's now too late, but anyway).

I would like to cherry-pick it to QtWebkit-2.1 (the fork was made a few commits before this patch landed). Should I? Or is there a better fix/workaround available?

[Kenneth Rohde Christiansen]

yes please, we need all plugin fixes in 2.1

[Ademar Reis]

Revision r65728 cherry-picked into qtwebkit-2.1 with commit a74784a <http://gitorious.org/webkit/qtwebkit/commit/a74784a>