The WebKit2 port makes the need of using the PlatformStrategies in Qt but currently PluginStrategy does not fit our needs. Qt has API to set custom plugins for a specific page. See http://doc.qt.nokia.com/4.6/qwebpage.html#setPluginFactory for more information. To be able to implement this API through PluginStrategy PluginStrategy::getPluginInfo should take a Page* argument that would be unused by other ports.
Created attachment 67752 [details] proposed patch
Comment on attachment 67752 [details] proposed patch The patch doesn't contain the Qt part, not say why the page is needed.
Attachment 67752 [details] did not build on win: Build output: http://queues.webkit.org/results/4005036
Comment on attachment 67752 [details] proposed patch Windows needs investigation.
(In reply to comment #2) > (From update of attachment 67752 [details]) > The patch doesn't contain the Qt part, not say why the page is needed. The qt part will coming soon in a separate patch but first I need to fix the windows build.
Created attachment 67780 [details] proposed patch v2
Comment on attachment 67780 [details] proposed patch v2 > + * plugins/PluginStrategy.h: Added a |const Page*| argument to initPlugins. Typo: you're adding the argument to getPluginInfo, not initPlugins. This approach looks good to me, it fixes the current crashiness and API regression. Why does the Qt part needs to be in a separate patch?
(In reply to comment #7) > (From update of attachment 67780 [details]) > > + * plugins/PluginStrategy.h: Added a |const Page*| argument to initPlugins. > Typo: you're adding the argument to getPluginInfo, not initPlugins. > > This approach looks good to me, it fixes the current crashiness and API regression. > > Why does the Qt part needs to be in a separate patch? For easier review :). My imagination was that this will be reviewed by a core developer from Apple and the qt part will be reviewed by a Qt folk. Furthermore, as I know the policy is to keep patches as small as we can. Actually, the Qt part is out-of-date by means of http://trac.webkit.org/changeset/67787. I think we should land this first and then fix our PlatformStrategies.
Comment on attachment 67780 [details] proposed patch v2 Review cq? since the Changelog should be typo fixed.
(In reply to comment #9) > (From update of attachment 67780 [details]) > Review cq? since the Changelog should be typo fixed. Remove
Comment on attachment 67780 [details] proposed patch v2 PlatformStrategies is supposed to be a global instance for global platform things. Getting additional [Qt] plugins associated with a page isn't really a platform things and might have to be done separately.
(In reply to comment #11) > (From update of attachment 67780 [details]) > PlatformStrategies is supposed to be a global instance for global platform things. Getting additional [Qt] plugins associated with a page isn't really a platform things and might have to be done separately. Maybe it is not such a bad idea anyway, as it would allow us to restrict certain plugins for certain pages. Another solution would be to create a PluginDatabaseClient for accessing the Qt page related plugins, though that seems like a bit of overkill.
http://trac.webkit.org/changeset/68038 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release The following changes are on the blame list: http://trac.webkit.org/changeset/68037 http://trac.webkit.org/changeset/68038
Committed r68038: <http://trac.webkit.org/changeset/68038>