Bug 46262 – [Qt] [WebKit2] Workarounds for the flash plugin

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

[Balazs Kelemen]

My plan is to organizing the static functions from PluginPackageQt.cpp into a class with static methods.

[Balazs Kelemen]

As I went forward with the implementation I have decided to put the refactoring and the changes that needs to be done in WebKit2 in one patch. That is why I changed the bug title.

[Balazs Kelemen]

Created attachment 68510 [details]
proposed patch

[Kenneth Rohde Christiansen]

Comment on attachment 68510 [details]
proposed patch

Maybe call the class FlashPluginQuirks ?

[Balazs Kelemen]

(In reply to comment #3)
> (From update of attachment 68510 [details])
> Maybe call the class FlashPluginQuirks ?

We use the term quirks in a different meaning: handling less stupid flavour of some plugins. FlashPluginHacks?

[Kenneth Rohde Christiansen]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 68510 [details] [details])
> > Maybe call the class FlashPluginQuirks ?
> 
> We use the term quirks in a different meaning: handling less stupid flavour of some plugins. FlashPluginHacks?

I just thought Workarounds to be a bit long :-) what about FlashPluginFixes?

[Kenneth Rohde Christiansen]

Comment on attachment 68510 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68510&action=review

> WebKit2/WebProcess/Plugins/Netscape/NetscapeBrowserFuncs.cpp:447
> +#elif PLATFORM(QT)

Won't this be the same for GTK+ etc? Would it break anything having this enabled for all ports?

> WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.h:66
> +#if PLATFORM(QT)
> +    WebCore::PluginQuirkSet quirks() const { return m_quirks; }
> +#endif

Shouldn't all platforms support plugin quirks?

> WebKit2/WebProcess/Plugins/Netscape/qt/NetscapePluginQt.cpp:52
> +    } else if (FlashPluginWorkarounds::isFlashPlugin(pluginPath)) {
> +        // We have already loaded the plugin but initializeGTK
> +        // needs a QLibrary for resolving gtk_init.
> +        QLibrary flashPlugin(pluginPath);
> +        ASSERT(flashPlugin.load());
> +        FlashPluginWorkarounds::initializeGTK(&flashPlugin);

So are we loading it twice?

[Andreas Kling]

Balazs: ping

[Balazs Kelemen]

(In reply to comment #7)
> Balazs: ping

Needs more investigation. I think there is some confusion in the old code paths
(and comments) because not only the flash plugin needs the toolkit value workaround but all npwrappers. In the last 2 weeks I had no time for working on this, hopefully I can finish the patch in the next week.

[Balazs Kelemen]

(In reply to comment #6)
> (From update of attachment 68510 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68510&action=review
> 
> > WebKit2/WebProcess/Plugins/Netscape/NetscapeBrowserFuncs.cpp:447
> > +#elif PLATFORM(QT)
> 
> Won't this be the same for GTK+ etc? Would it break anything having this enabled for all ports?

Right, it should be PLATFORM(UNIX)

> 
> > WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.h:66
> > +#if PLATFORM(QT)
> > +    WebCore::PluginQuirkSet quirks() const { return m_quirks; }
> > +#endif
> 
> Shouldn't all platforms support plugin quirks?

Likely. Will be added as common member in the next patch.

> 
> > WebKit2/WebProcess/Plugins/Netscape/qt/NetscapePluginQt.cpp:52
> > +    } else if (FlashPluginWorkarounds::isFlashPlugin(pluginPath)) {
> > +        // We have already loaded the plugin but initializeGTK
> > +        // needs a QLibrary for resolving gtk_init.
> > +        QLibrary flashPlugin(pluginPath);
> > +        ASSERT(flashPlugin.load());
> > +        FlashPluginWorkarounds::initializeGTK(&flashPlugin);
> 
> So are we loading it twice?

I am almost sure about QLibrary does not load the same plugin twice (if it is even possible) but returns true if it is already loaded.

[Balazs Kelemen]

Comment on attachment 68510 [details]
proposed patch

Needs more investigation.

[Balazs Kelemen]

*** This bug has been marked as a duplicate of bug 48127 ***