[WK2] Introduce an option for no NPAPI support

• Component: Platform
• Status: RESOLVED FIXED

[Laszlo Gombos]

http://trac.webkit.org/changeset/80007 broke the Symbian build for WK2. To resolve the build-break I propose a simple PLUGIN_ARCHITECTURE(NONE) option. Patch will follow.

[Laszlo Gombos]

Created attachment 84876 [details]
1st try

[Yael]

(In reply to comment #0)
> http://trac.webkit.org/changeset/80007 broke the Symbian build for WK2. To resolve the build-break I propose a simple PLUGIN_ARCHITECTURE(NONE) option. Patch will follow.

It broke the Qt on mac build with webkit2 as well. :(

[Laszlo Gombos]

(In reply to comment #2)
> (In reply to comment #0)
> > http://trac.webkit.org/changeset/80007 broke the Symbian build for WK2. To resolve the build-break I propose a simple PLUGIN_ARCHITECTURE(NONE) option. Patch will follow.
> 
> It broke the Qt on mac build with webkit2 as well. :(

Yael, are you suggesting turning on NONE for QtWebKit mac as well ?

[Yael]

(In reply to comment #3)
> Yael, are you suggesting turning on NONE for QtWebKit mac as well ?
Until we have the mac specific bits for plugins in place, I think we should do that.

[Balazs Kelemen]

I would better like to handle this by the build system mostly because
the ENABLE style macros confusing the IDE (i.e. files that guarded
with such a macro are ignored to be parsed and indexed). Generally
we do it that way for platform specific stuff in WebCore.
Besides that PLUGIN_ARCHITECTURE(NONE) doesn't sounds really straightforward.

[Balazs Kelemen]

(In reply to comment #5)
> I would better like to handle this by the build system mostly because
> the ENABLE style macros confusing the IDE (i.e. files that guarded
> with such a macro are ignored to be parsed and indexed). Generally
> we do it that way for platform specific stuff in WebCore.
> Besides that PLUGIN_ARCHITECTURE(NONE) doesn't sounds really straightforward.

After discussed it with Laszlo I realized that my argument against
ENABLE style macros is less important than keeping the build system
straightforward so I'm ok with this patch. Maybe one day I will
fix my problems in qtcreator :)

[Balazs Kelemen]

Created attachment 86666 [details]
Patch

[Balazs Kelemen]

This also affects GTK. The new patch add the new files to GNUMakefile.am as
well as WebKit2.pro. It has been tested as it is and with adding an "#if 0"
before the NETSCAPE_PLUGIN_X11 branch (so it should build and work
on Qt-Symbian and Qt-Mac as well).

[Balazs Kelemen]

Created attachment 86701 [details]
Patch

updated

[Kenneth Rohde Christiansen]

Comment on attachment 86701 [details]
Patch

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

> Source/WebKit2/ChangeLog:28
> +        * WebProcess/Plugins/Netscape/NetscapePluginNone.cpp: Added.

wouldn't NetscapePluginEmpty be a better name. That is already used for some similar case in WebKit, just search for *Empty.cpp

> Source/WebKit2/ChangeLog:45
> +        * config.h: Introduce PLUGIN_ARCHITECTURE(NONE) as another option.

What about UNSUPPORTED?

[Balazs Kelemen]

(In reply to comment #10)
> (From update of attachment 86701 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=86701&action=review
> 
> > Source/WebKit2/ChangeLog:28
> > +        * WebProcess/Plugins/Netscape/NetscapePluginNone.cpp: Added.
> 
> wouldn't NetscapePluginEmpty be a better name. That is already used for some similar case in WebKit, just search for *Empty.cpp

Let's grep!

$ find Source/ -name "*Empty*"
Source/WebCore/WebCore.gyp/mac/Empty.cpp                                                                                                    
Source/WebCore/platform/mac/EmptyProtocolDefinitions.h                                                                                      
Source/WebCore/loader/EmptyClients.h

$ find Source/ -name "*None*"
Source/JavaScriptCore/wtf/ThreadingNone.cpp
Source/WebCore/plugins/PluginDataNone.cpp
Source/WebCore/plugins/PluginViewNone.cpp
Source/WebCore/plugins/PluginPackageNone.cpp
Source/WebCore/platform/text/TextEncodingDetectorNone.cpp
Source/WebCore/platform/text/LocalizedNumberNone.cpp
Source/WebCore/platform/KillRingNone.cpp

This shows me that the name Empty is used in the context of the abstract client pattern while None is used for stubbed implementation.
This patch is fall into the second category.

> 
> > Source/WebKit2/ChangeLog:45
> > +        * config.h: Introduce PLUGIN_ARCHITECTURE(NONE) as another option.
> 
> What about UNSUPPORTED?

Good point. It's a more exact word for this case. Hopefully the mismatch between the macro name and the file name pattern is not really confusing.

[Balazs Kelemen]

Created attachment 86874 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 86874 [details]
Patch

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

LGTM, r=me. Please fix the typo in the changelog before landing.

> Source/WebKit2/ChangeLog:48
> +        * config.h: Introduce PLUGIN_ARCHITECTURE(UNSOPPORTED) as another

nit: s/UNSOPPORTED/UNSUPPORTED

[Balazs Kelemen]

Committed r82577: <http://trac.webkit.org/changeset/82577>