Bug 58580

Summary: Make plugins compile during WebKit GTK Windows build
Product: WebKit Reporter: Fridrich Strba <fridrich.strba>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
Patch
none
Patch updated according to the review none

Description Fridrich Strba 2011-04-14 15:02:20 PDT
Ifdef properly relevant parts so that the PluginViewGtk compiles on Window.
Comment 1 Fridrich Strba 2011-04-14 15:05:17 PDT
Created attachment 89663 [details]
Patch
Comment 2 Martin Robinson 2011-04-14 15:41:32 PDT
Comment on attachment 89663 [details]
Patch

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

Looks good, but I think it needs another iteration.

> Source/WebCore/plugins/PluginView.h:366
> +#if defined(XP_UNIX) || PLATFORM(GTK)
>          bool m_needsXEmbed;

Is the m_needsXEmbed variable really necessary for Windows?

> Source/WebCore/plugins/gtk/PluginViewGtk.cpp:83
> +#include <windows.h>
>  #include "PluginMessageThrottlerWin.h"
>  #include <gdk/gdkwin32.h>

Please put these in alphabetical order if you can.

> Source/WebCore/plugins/gtk/PluginViewGtk.cpp:667
> -            *static_cast<HGIOBJ*>(value) = GDK_WINDOW_HWND(gdkWindow);
> +            *static_cast<HGDIOBJ*>(value) = GDK_WINDOW_HWND(gdkWindow);

Whoops. I think this was my fault.

> Source/WebCore/plugins/gtk/PluginViewGtk.cpp:798
>          PluginView::setCurrentPluginView(this);
>          JSC::JSLock::DropAllLocks dropAllLocks(JSC::SilenceAssertionsOnly);
>          setCallingPlugin(true);
> +#if defined(XP_UNIX)
>          m_plugin->pluginFuncs()->getvalue(m_instance, NPPVpluginNeedsXEmbed, &m_needsXEmbed);
> +#endif

If you're going to avoid the plugin call on !XP_UNIX, it makes sense to avoid all the preparation for the call as well.
Comment 3 Fridrich Strba 2011-04-15 06:34:51 PDT
Created attachment 89776 [details]
Patch updated according to the review
Comment 4 Fridrich Strba 2011-04-15 06:36:32 PDT
Comment on attachment 89776 [details]
Patch updated according to the review

Note that the plug-ins are far from working though. This is just the first step: to have them compile and link. To make them work will be next step when time allows.
Comment 5 WebKit Commit Bot 2011-04-15 19:06:59 PDT
Comment on attachment 89776 [details]
Patch updated according to the review

Clearing flags on attachment: 89776

Committed r84071: <http://trac.webkit.org/changeset/84071>
Comment 6 WebKit Commit Bot 2011-04-15 19:07:04 PDT
All reviewed patches have been landed.  Closing bug.