Bug 89451 – [WK2][X11] Fix build break when building webkit2 without plugin process.

• Status: RESOLVED
• Resolution: INVALID
• Component: WebKit2

[Mariusz Grzegorczyk]

When trying to build WK2 by running:
./Tools/Scripts/build-webkit --gtk --enable-plugin-process=no
,build break occurs:

../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp: In member function ‘bool WebKit::NetscapePluginModule::getPluginInfoForLoadedPlugin(WebKit::RawPluginMetaData&)’:
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:115:17: error: invalid use of incomplete type ‘struct WebKit::RawPluginMetaData’
../../Source/WebKit2/Shared/Plugins/Netscape/NetscapePluginModule.h:40:7: error: forward declaration of ‘struct WebKit::RawPluginMetaData’
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:119:17: error: invalid use of incomplete type ‘struct WebKit::RawPluginMetaData’
../../Source/WebKit2/Shared/Plugins/Netscape/NetscapePluginModule.h:40:7: error: forward declaration of ‘struct WebKit::RawPluginMetaData’
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:125:13: error: invalid use of incomplete type ‘struct WebKit::RawPluginMetaData’
../../Source/WebKit2/Shared/Plugins/Netscape/NetscapePluginModule.h:40:7: error: forward declaration of ‘struct WebKit::RawPluginMetaData’
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp: In static member function ‘static bool WebKit::NetscapePluginModule::getPluginInfo(const WTF::String&, WebKit::PluginModuleInfo&)’:
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:132:23: error: aggregate ‘WebKit::RawPluginMetaData metaData’ has incomplete type and cannot be defined
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:133:10: error: ‘PluginProcessProxy’ has not been declared
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp: In static member function ‘static bool WebKit::NetscapePluginModule::scanPlugin(const WTF::String&)’:
../../Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:185:23: error: aggregate ‘WebKit::RawPluginMetaData metaData’ has incomplete type and cannot be defined

[Balazs Kelemen]

This is by design, I don't think we want to support plugins without plugin process.

[Mariusz Grzegorczyk]

(In reply to comment #1)
> This is by design, I don't think we want to support plugins without plugin process.

So what for is this macro? I saw a lot of common code which checks if plugin process is on, and depends on it calls are made directly(same process) or via IPC. If you want to build webkit2 always with plugin process, maybe this setting should be removed, or event compilation shouldn't start.

[Balazs Kelemen]

(In reply to comment #2)
> (In reply to comment #1)
> > This is by design, I don't think we want to support plugins without plugin process.
> 
> So what for is this macro? I saw a lot of common code which checks if plugin process is on, and depends on it calls are made directly(same process) or via IPC. If you want to build webkit2 always with plugin process, maybe this setting should be removed, or event compilation shouldn't start.

I guess the Mac port still supports disabling it and keeping it guarded could help in the future if we decide to change on this. Furthermore, the Qt build system does neither try to sanitize the custom build options you use, nor detect if you turn on something that is not supported, and we don't want to do this. I don't know what is the policy for Gtk.

[Mariusz Grzegorczyk]

Anyway I think this kind of option can be useful. There is already infrastructure to do this in webcore(direct calls or via IPC depending on PLUGIN PROCESS macro). In my opinion each port should follow this rule, and support building with or without plugin process, or at least make port compilable with all parameters passed to build-webkit script.

[Mariusz Grzegorczyk]

I discovered this issue while trying to support plugins to EFL's WK2. In the past I prepared such patch which works with/without plugin process. In the past there was no problem but now NetscapePluginModuleX11.cpp uses structs and functions which are defined only when PLUGIN_PROCESS is on.
So because of this issue support for plugins in EFL's WK2 MUST use plugin process.
What else Qt for linux also uses this file, so they cannot simply change their policy.

[Balazs Kelemen]

(In reply to comment #5)
> I discovered this issue while trying to support plugins to EFL's WK2. In the past I prepared such patch which works with/without plugin process. In the past there was no problem but now NetscapePluginModuleX11.cpp uses structs and functions which are defined only when PLUGIN_PROCESS is on.
> So because of this issue support for plugins in EFL's WK2 MUST use plugin process.
> What else Qt for linux also uses this file, so they cannot simply change their policy.

Why do you think you are not allowed to change on this? If there are stuff necessary to build plugin code but not belongs to plugin process, you can fix the guards and make them independent from plugin process. Why don't you upload a patch that fix this?
However, I wonder why would you disable plugin process. One of the biggest advantages of WebKit2 is stability, but you virtually lose this as soon as you load a plugin into the UI process. I don't expect that it's hard to port it to EFL, given that it's already a cross-platform feature.

[Jonas Heinrich]

Created attachment 176062 [details]
NetscapePluginModuleX11 fails to compile

I'm trying to build the latest webkitgtk nightly build 1.11.2 using the offical ArchLinux PKGBUILD:

[...]
cd "$srcdir/webkitgtk-$pkgver" 
PYTHON=/usr/bin/python2 ./configure --prefix=/usr \ 
  --enable-introspection \ 
  --disable-silent-rules \
  --libexecdir=/usr/lib/webkitgtk2 \ 
  --with-gstreamer=0.10 \ 
  --with-gtk=2.0 \ 
  --disable-webkit2 
make all stamp-po 
[...]

Now I'm getting the same error as described above (see attached log file). Is there any workaround to this? I can't get it compile and I don't know if I even need this Netscape plugin thing.

Please help!

[Carlos Garcia Campos]

(In reply to comment #7)
> Created an attachment (id=176062) [details]
> NetscapePluginModuleX11 fails to compile
> 
> I'm trying to build the latest webkitgtk nightly build 1.11.2 using the offical ArchLinux PKGBUILD:
> 
> [...]
> cd "$srcdir/webkitgtk-$pkgver" 
> PYTHON=/usr/bin/python2 ./configure --prefix=/usr \ 
>   --enable-introspection \ 
>   --disable-silent-rules \
>   --libexecdir=/usr/lib/webkitgtk2 \ 
>   --with-gstreamer=0.10 \ 
>   --with-gtk=2.0 \ 
>   --disable-webkit2 
> make all stamp-po 
> [...]
> 
> Now I'm getting the same error as described above (see attached log file). Is there any workaround to this? I can't get it compile and I don't know if I even need this Netscape plugin thing.
> 
> Please help!

I think this is a different bug because you are disabling webkit2 completely, this bug is about building with webkit2 but with plugin process disabled. I wonder why --libexecdir=/usr/lib/webkitgtk2 is passed and --disable-webkit2, though

[Mariusz Grzegorczyk]

Created attachment 190923 [details]
patch

[Balazs Kelemen]

Comment on attachment 190923 [details]
patch

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

> Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:195
>  }
>  
>  bool NetscapePluginModule::scanPlugin(const String& pluginPath)
> +#else
> +bool NetscapePluginModule::scanPlugin(const String& pluginPath, RawPluginMetaData& metaData)
> +#endif
>  {
> +#if ENABLE(PLUGIN_PROCESS)
>      RawPluginMetaData metaData;
> -
> +#endif
>      {
>          // Don't allow the plugin to pollute the standard output.
>          StdoutDevNullRedirector stdOutRedirector;

Instead of using such ugly ifdefs, I would just factor out the code that you really need from here into a separate function and just call it when plugin process is disabled. Let's call it something like | queryPluginInfo(const String& pluginPath, RawPluginMetaData&) | which is just the block of stdOutRedirector from scanPlugin. It would look much better I think.

[Mariusz Grzegorczyk]

Created attachment 190944 [details]
updated with Balazs' comments

[Anders Carlsson]

Comment on attachment 190944 [details]
updated with Balazs' comments

ENABLE(PLUGIN_PROCESS) has been removed now, I believe this bug is invalid.