Bug 44324

Created attachment 64931 [details] Proposed patch

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.

LGTM other than the style fixes.

Comment on attachment 64931 [details] Proposed patch Clearing flags on attachment: 64931 Committed r65728: <http://trac.webkit.org/changeset/65728>

All reviewed patches have been landed. Closing bug.

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

(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.

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 :)

(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.

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

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?

yes please, we need all plugin fixes in 2.1

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