[GTK] Build break when NETSCAPE_PLUGIN_API macro is off

[Mariusz Grzegorczyk]

When trying to build webkit gtk with plugin's support off build break occurs:

  CXX    Source/WebCore/plugins/libWebCore_la-PluginPackage.lo
../../Source/WebCore/plugins/npapi.cpp: In function ‘NPError NPN_GetURLNotify(NPP, const char*, const char*, void*)’:
../../Source/WebCore/plugins/npapi.cpp:74:45: error: ‘class WebCore::PluginView’ has no member named ‘getURLNotify’
../../Source/WebCore/plugins/npapi.cpp: In function ‘NPError NPN_GetURL(NPP, const char*, const char*)’:
../../Source/WebCore/plugins/npapi.cpp:79:45: error: ‘class WebCore::PluginView’ has no member named ‘getURL’


also --no-netscape-plugin-api option to build-webkit doesn't make any effect.

[Mariusz Grzegorczyk]

Qt and ports that use cmake, don't compile npapi.cpp when NETSCAPE_PLUGIN_API is off(check in Target.pri and CMakeLists.txt). What policy should be applied to Gtk?
1. Have all files added to sources(like now) and make checks in them
2. Make check in GNUMakefile.list.am and don't take inappropriate files to build(e.g. in this case npapi.cpp)

[Balazs Kelemen]

(In reply to comment #1)
> Qt and ports that use cmake, don't compile npapi.cpp when NETSCAPE_PLUGIN_API is off(check in Target.pri and CMakeLists.txt). What policy should be applied to Gtk?
> 1. Have all files added to sources(like now) and make checks in them
> 2. Make check in GNUMakefile.list.am and don't take inappropriate files to build(e.g. in this case npapi.cpp)

I think 1. is the more generally accepted.

[Philip Chimento]

Created attachment 244122 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Comment on attachment 244122 [details]
Patch

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

> Source/WebKit2/PluginProcess/unix/PluginControllerProxyUnix.cpp:30
>  #if ENABLE(PLUGIN_PROCESS)
> +#if ENABLE(NETSCAPE_PLUGIN_API)

I don't think this is correct, why using NETSCAPE_PLUGIN_API here? ENABLE_PLUGIN_PROCESS is what it should be false, I guess.

> Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:32
>  #if ENABLE(PLUGIN_PROCESS)
> +#if ENABLE(NETSCAPE_PLUGIN_API)

Ditto.

> Source/WebKit2/PluginProcess/unix/PluginProcessUnix.cpp:30
>  #if ENABLE(PLUGIN_PROCESS)
> +#if ENABLE(NETSCAPE_PLUGIN_API)

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:690
> +#if ENABLE(NETSCAPE_PLUGIN_API)
>      Vector<PluginModuleInfo> plugins = WEBKIT_WEB_CONTEXT(object)->priv->context->pluginInfoStore().plugins();
>      GList* returnValue = 0;
>      for (size_t i = 0; i < plugins.size(); ++i)
>          returnValue = g_list_prepend(returnValue, webkitPluginCreate(plugins[i]));
>      g_task_return_pointer(task, returnValue, reinterpret_cast<GDestroyNotify>(destroyPluginList));
> +#else
> +    g_task_return_pointer(task, NULL, NULL);
> +#endif

Why doing this in the thread body? If NETSCAPE_PLUGIN_API is not enabled we should call g_task_return_pointer before spawning the thread.

> Source/WebKit2/UIProcess/Launcher/gtk/ProcessLauncherGtk.cpp:68
> +#if ENABLE(NETSCAPE_PLUGIN_API)

I guess this should be #if ENABLE(PLUGIN_PROCESS)

> Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:32
>  #if ENABLE(PLUGIN_PROCESS)
> +#if ENABLE(NETSCAPE_PLUGIN_API)

Same here about duplicated if

> Source/WebKit2/WebProcess/Plugins/Netscape/unix/PluginProxyUnix.cpp:30
>  #if ENABLE(PLUGIN_PROCESS)
> +#if ENABLE(NETSCAPE_PLUGIN_API)

Ditto.

[Philip Chimento]

So what you're saying is that we should never compile with -DENABLE_NETSCAPE_PLUGIN_API=OFF and instead use -DENABLE_PLUGIN_PROCESS=OFF?

Or, in other words, having PLUGIN_PROCESS enabled without the NETSCAPE_PLUGIN_API doesn't make sense? In that case, can I just remove the extra cmake configuration parameter for the Netscape API?

[Carlos Garcia Campos]

(In reply to comment #6)
> So what you're saying is that we should never compile with
> -DENABLE_NETSCAPE_PLUGIN_API=OFF and instead use -DENABLE_PLUGIN_PROCESS=OFF?
> 
> Or, in other words, having PLUGIN_PROCESS enabled without the
> NETSCAPE_PLUGIN_API doesn't make sense? 

It doesn't make sense for us, because the only plugins supported by the GTK port are the NPAPI plugins.

> In that case, can I just remove the
> extra cmake configuration parameter for the Netscape API?

Yes, we could probably remove the cmake option, or ensure that it's OFF when the plugin process option is OFF

[Carlos Garcia Campos]

Plugins are no longer supported.