587370 - darwin/x11 gtk build fails in PluginInstanceParent.h

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jeremy Huddleston]

../../../../dist/include/mozilla/plugins/PluginInstanceParent.h:303: error: 'CGColorSpaceRef' does not name a type
../../../../dist/include/mozilla/plugins/PluginInstanceParent.h:305: error: ISO C++ forbids declaration of 'nsIOSurface' with no type
../../../../dist/include/mozilla/plugins/PluginInstanceParent.h:305: error: expected ';' before '*' token

My guess is there is a new check added which assumes darwin is quartz...

Comment 1

[Jeremy Huddleston]

Yeah, here are the changes to PluginInstanceParent from 1.9.2.8 to firefox4.0b3:

Note that it's using CoreAnimation based on OS_MACOSX rather than based on toolkit:

--- work/mozilla-1.9.2/dom/plugins/PluginInstanceParent.h	2010-07-22 14:50:52.000000000 -0700
+++ ../xulrunner-devel/work/mozilla-central/dom/plugins/PluginInstanceParent.h	2010-08-05 18:09:01.000000000 -0700
@@ -43,6 +43,8 @@
 #include "mozilla/plugins/PluginScriptableObjectParent.h"
 #if defined(OS_WIN)
 #include "mozilla/gfx/SharedDIBWin.h"
+#elif defined(OS_MACOSX)
+#include "nsCoreAnimationSupport.h"
 #endif
 
 #include "npfunctions.h"
@@ -66,6 +68,7 @@ class PluginInstanceParent : public PPlu
 public:
     PluginInstanceParent(PluginModuleParent* parent,
                          NPP npp,
+                         const nsCString& mimeType,
                          const NPNetscapeFuncs* npniface);
 
     virtual ~PluginInstanceParent();
@@ -126,6 +129,12 @@ public:
     virtual bool
     AnswerNPN_SetValue_NPPVpluginTransparent(const bool& transparent,
                                              NPError* result);
+    virtual bool
+    AnswerNPN_SetValue_NPPVpluginDrawingModel(const int& drawingModel,
+                                             NPError* result);
+    virtual bool
+    AnswerNPN_SetValue_NPPVpluginEventModel(const int& eventModel,
+                                             NPError* result);
 
     virtual bool
     AnswerNPN_GetURL(const nsCString& url, const nsCString& target,
@@ -157,11 +166,10 @@ public:
     RecvNPN_InvalidateRect(const NPRect& rect);
 
     virtual bool
-    AnswerNPN_PushPopupsEnabledState(const bool& aState,
-                                     bool* aSuccess);
+    AnswerNPN_PushPopupsEnabledState(const bool& aState);
 
     virtual bool
-    AnswerNPN_PopPopupsEnabledState(bool* aSuccess);
+    AnswerNPN_PopPopupsEnabledState();
 
     NS_OVERRIDE virtual bool
     AnswerNPN_GetValueForURL(const NPNURLVariable& variable,
@@ -183,6 +191,17 @@ public:
                                     nsCString* password,
                                     NPError* result);
 
+    NS_OVERRIDE virtual bool
+    AnswerNPN_ConvertPoint(const double& sourceX,
+                           const bool&   ignoreDestX,
+                           const double& sourceY,
+                           const bool&   ignoreDestY,
+                           const NPCoordinateSpace& sourceSpace,
+                           const NPCoordinateSpace& destSpace,
+                           double *destX,
+                           double *destY,
+                           bool *result);
+
     NPError NPP_SetWindow(const NPWindow* aWindow);
 
     NPError NPP_GetValue(NPPVariable variable, void* retval);
@@ -225,9 +244,23 @@ public:
     }
 
     virtual bool
-    AnswerPluginGotFocus();
+    AnswerPluginFocusChange(const bool& gotFocus);
+
+#if defined(OS_MACOSX)
+    void Invalidate();
+#endif // definied(OS_MACOSX)
 
 private:
+    // Quirks mode support for various plugin mime types
+    enum PluginQuirks {
+        // OSX: Don't use the refresh timer for plug-ins
+        // using this quirk. These plug-in most have another
+        // way to refresh the window.
+        COREANIMATION_REFRESH_TIMER = 1,
+    };
+
+    void InitQuirksModes(const nsCString& aMimeType);
+
     bool InternalGetValueForNPObject(NPNVariable aVariable,
                                      PPluginScriptableObjectParent** aValue,
                                      NPError* aResult);
@@ -237,6 +270,7 @@ private:
     NPP mNPP;
     const NPNetscapeFuncs* mNPNIface;
     NPWindowType mWindowType;
+    int mQuirks;
 
     nsDataHashtable<nsVoidPtrHashKey, PluginScriptableObjectParent*> mScriptableObjects;
 
@@ -261,6 +295,15 @@ private:
     WNDPROC            mPluginWndProc;
     bool               mNestedEventState;
 #endif // defined(XP_WIN)
+#if defined(OS_MACOSX)
+private:
+    Shmem              mShSurface; 
+    size_t             mShWidth;
+    size_t             mShHeight;
+    CGColorSpaceRef    mShColorSpace;
+    int16_t            mDrawingModel;
+    nsIOSurface       *mIOSurface;
+#endif // definied(OS_MACOSX)
 };

Comment 2

[Jeremy Huddleston]

If this code is intended to work on OS X, then it is missing an include on CoreGraphics/... to declare CGColorSpaceRef ... if it is intended to just work with Cocoa, then the pre-processor magic needs to be changed to MOZ_WIDGET_COCOA

Comment 3

[Jeremy Huddleston]

Ok, so it should be getting the CG headers from nsCoreAnimationSupport.h, but that header has #ifdef guarding on it using XP_MACOSX rather than OS_MACOSX ... XP_MACOSX isn't defined (should it be, and if so, where)

Next, there should be some guarding of 'Cursor' when including <ApplicationServices/ApplicationServices.h> to prevent conflict with X11's Cursor type.

#define Cursor OSX_Cursor
#include ...
#undef Cursor

And then, NPCocoaEvent isn't defined...

So it looks like this is intended just for Cocoa...  please change the #ifdef-foo to use MOZ_WIDGET_COCOA

Comment 4

[Benoit Girard (:BenWa)]

(In reply to comment #3)
> Ok, so it should be getting the CG headers from nsCoreAnimationSupport.h, but
> that header has #ifdef guarding on it using XP_MACOSX rather than OS_MACOSX 

Aren't XP_MACOSX/OS_MACOSX equivalent?

Comment 5

[Jeremy Huddleston]

(In reply to comment #4)
> (In reply to comment #3)
> > Ok, so it should be getting the CG headers from nsCoreAnimationSupport.h, but
> > that header has #ifdef guarding on it using XP_MACOSX rather than OS_MACOSX 
> 
> Aren't XP_MACOSX/OS_MACOSX equivalent?

One would think, but XP_MACOSX wasn't being set... which may be a separate issue... the issue here is that the check should be for MOZ_WIDGET_COCOA

Comment 6

[Hanspeter Niederstrasser]

Attachment: Use MOZ_WIDGET_COCOA instead of OS_MACOSX

The IPC code uses OS_MACOSX for some reason (probably inherited from upstream) and I think it's only set within dom/plugins and not exported to other locations in the tree.

This patch changes some locations to use MOZ_WIDGET_COCOA around nsNPAPIPlugin.cpp and other files that then failed to compile after that first #ifdef.  The new flags in toolkit/library/Makefile.in are needed to get missing symbols when linking libxul.dylib.  Both cairo-cocoa and cairo-gtk on Darwin build successfully after this patch is applied.

Comment 7

[Jeremy Huddleston]

This patch has been available for a month now.  Can someone please review and commit?

Comment 8

[Jeremy Huddleston]

Attachment: updated to apply to 4.0b7

Comment 9

[Jeremy Huddleston]

Attachment: Use MOZ_WIDGET_COCOA instead of OS_MACOSX

Missed one spot.  This compiles, and firefox runs.

Comment 10

[Scott Greenlay [:sgreenlay]]

Comment on attachment 493533 [details] [diff] [review]
Use MOZ_WIDGET_COCOA instead of OS_MACOSX

This doesn't apply cleanly on trunk.

> +#ifdef MOZ_WIDGET_COCOA
>      while (iter.HasMore()) {
>          iter.GetNext()->Invalidate();
>      }
> +#endif // MOZ_WIDGET_COCOA

Why did you choose to add MOZ_WIDGET_COCOA to this section?

Comment 11

[Jeremy Huddleston]

Comment on attachment 493533 [details] [diff] [review]
Use MOZ_WIDGET_COCOA instead of OS_MACOSX

because that API (Invalidate()) is specific for MOZ_WIDGET_COCOA ... see the changes in dom/plugins/PluginInstanceParent.h

Comment 12

[Jeremy Huddleston]

As for not applying to trunk, sorry... it applies to 4.0b7 ... I'll wait until 4.0b8 comes out and update it again against that one.

Comment 13

[Jeremy Huddleston]

The patch attached applied to 4.0b8 for me, so I don't know why you had trouble applying it.  I'll update it to apply with no fuzz or offset.

Comment 14

[Jeremy Huddleston]

Attachment: Use MOZ_WIDGET_COCOA instead of OS_MACOSX updated for 4.0b8

Comment 15

[Jeremy Huddleston]

The attached patch is still not in 4.0b9.  Please review and integrate.  What is delaying this fix which was available back in November?

Comment 16

[Jeremy Huddleston]

This is still not in 4.0b11 ... Why is there such a delay here?

Comment 17

[Jeremy Huddleston]

Comment on attachment 499147 [details] [diff] [review]
Use MOZ_WIDGET_COCOA instead of OS_MACOSX updated for 4.0b8

>diff -Naurp mozilla-central.orig/dom/plugins/PluginInstanceChild.cpp mozilla-central/dom/plugins/PluginInstanceChild.cpp
>--- dom/plugins/PluginInstanceChild.cpp	2010-12-14 17:02:43.000000000 -0800
>+++ dom/plugins/PluginInstanceChild.cpp	2010-12-21 14:04:20.000000000 -0800
>@@ -126,7 +126,7 @@ PluginInstanceChild::PluginInstanceChild
>     , mWinlessHiddenMsgHWND(0)
> #endif // OS_WIN
>     , mAsyncCallMutex("PluginInstanceChild::mAsyncCallMutex")
>-#if defined(OS_MACOSX)
>+#if defined(MOZ_WIDGET_COCOA)
> #if defined(__i386__)
>     , mEventModel(NPEventModelCarbon)
> #endif
>@@ -180,7 +180,7 @@ PluginInstanceChild::~PluginInstanceChil
> #if defined(OS_WIN)
>     NS_ASSERTION(!mPluginWindowHWND, "Destroying PluginInstanceChild without NPP_Destroy?");
> #endif
>-#if defined(OS_MACOSX)
>+#if defined(MOZ_WIDGET_COCOA)
>     if (mShColorSpace) {
>         ::CGColorSpaceRelease(mShColorSpace);
>     }
>diff -Naurp mozilla-central.orig/dom/plugins/PluginInstanceChild.h mozilla-central/dom/plugins/PluginInstanceChild.h
>--- dom/plugins/PluginInstanceChild.h	2010-12-14 17:02:43.000000000 -0800
>+++ dom/plugins/PluginInstanceChild.h	2010-12-21 14:04:20.000000000 -0800
>@@ -44,7 +44,7 @@
> #include "mozilla/plugins/StreamNotifyChild.h"
> #if defined(OS_WIN)
> #include "mozilla/gfx/SharedDIBWin.h"
>-#elif defined(OS_MACOSX)
>+#elif defined(MOZ_WIDGET_COCOA)
> #include "nsCoreAnimationSupport.h"
> #include "base/timer.h"
> #endif
>@@ -370,7 +370,7 @@ private:
>       HBITMAP         bmp;
>     } mAlphaExtract;
> #endif // defined(OS_WIN)
>-#if defined(OS_MACOSX)
>+#if defined(MOZ_WIDGET_COCOA)
> private:
> #if defined(__i386__)
>     NPEventModel          mEventModel;
>diff -Naurp mozilla-central.orig/dom/plugins/PluginInstanceParent.cpp mozilla-central/dom/plugins/PluginInstanceParent.cpp
>--- dom/plugins/PluginInstanceParent.cpp	2010-12-14 17:02:43.000000000 -0800
>+++ dom/plugins/PluginInstanceParent.cpp	2010-12-21 14:04:20.000000000 -0800
>@@ -108,7 +108,7 @@ PluginInstanceParent::PluginInstancePare
> void
> PluginInstanceParent::InitQuirksModes(const nsCString& aMimeType)
> {
>-#ifdef OS_MACOSX
>+#ifdef MOZ_WIDGET_COCOA
>     NS_NAMED_LITERAL_CSTRING(flash, "application/x-shockwave-flash");
>     // Flash sends us Invalidate events so we will use those
>     // instead of the refresh timer.
>@@ -127,7 +127,7 @@ PluginInstanceParent::~PluginInstancePar
>     NS_ASSERTION(!(mPluginHWND || mPluginWndProc),
>         "Subclass was not reset correctly before the dtor was reached!");
> #endif
>-#if defined(OS_MACOSX)
>+#if defined(MOZ_WIDGET_COCOA)
>     if (mShWidth != 0 && mShHeight != 0) {
>         DeallocShmem(mShSurface);
>     }
>@@ -1427,7 +1427,7 @@ PluginInstanceParent::AnswerPluginFocusC
> #endif
> }
> 
>-#ifdef OS_MACOSX
>+#ifdef MOZ_WIDGET_COCOA
> void
> PluginInstanceParent::Invalidate()
> {
>diff -Naurp mozilla-central.orig/dom/plugins/PluginInstanceParent.h mozilla-central/dom/plugins/PluginInstanceParent.h
>--- dom/plugins/PluginInstanceParent.h	2010-12-14 17:02:43.000000000 -0800
>+++ dom/plugins/PluginInstanceParent.h	2010-12-21 14:04:20.000000000 -0800
>@@ -43,7 +43,7 @@
> #include "mozilla/plugins/PluginScriptableObjectParent.h"
> #if defined(OS_WIN)
> #include "mozilla/gfx/SharedDIBWin.h"
>-#elif defined(OS_MACOSX)
>+#elif defined(MOZ_WIDGET_COCOA)
> #include "nsCoreAnimationSupport.h"
> #endif
> 
>@@ -256,9 +256,9 @@ public:
>     virtual bool
>     AnswerPluginFocusChange(const bool& gotFocus);
> 
>-#if defined(OS_MACOSX)
>+#ifdef MOZ_WIDGET_COCOA
>     void Invalidate();
>-#endif // definied(OS_MACOSX)
>+#endif // definied(MOZ_WIDGET_COCOA)
> 
>     nsresult AsyncSetWindow(NPWindow* window);
>     nsresult GetSurface(gfxASurface** aSurface);
>@@ -308,7 +308,7 @@ private:
>     WNDPROC            mPluginWndProc;
>     bool               mNestedEventState;
> #endif // defined(XP_WIN)
>-#if defined(OS_MACOSX)
>+#if defined(MOZ_WIDGET_COCOA)
> private:
>     Shmem              mShSurface; 
>     size_t             mShWidth;
>@@ -316,7 +316,7 @@ private:
>     CGColorSpaceRef    mShColorSpace;
>     int16_t            mDrawingModel;
>     nsIOSurface       *mIOSurface;
>-#endif // definied(OS_MACOSX)
>+#endif // definied(MOZ_WIDGET_COCOA)
> 
>     // ObjectFrame layer wrapper
>     nsRefPtr<gfxASurface>    mFrontSurface;
>diff -Naurp mozilla-central.orig/dom/plugins/PluginModuleChild.cpp mozilla-central/dom/plugins/PluginModuleChild.cpp
>--- dom/plugins/PluginModuleChild.cpp	2010-12-14 17:02:43.000000000 -0800
>+++ dom/plugins/PluginModuleChild.cpp	2010-12-21 14:04:20.000000000 -0800
>@@ -73,7 +73,7 @@
> #include "nsWindowsDllInterceptor.h"
> #endif
> 
>-#ifdef OS_MACOSX
>+#ifdef MOZ_WIDGET_COCOA
> #include "PluginInterposeOSX.h"
> #include "PluginUtilsOSX.h"
> #endif
>@@ -1556,7 +1556,7 @@ _popupcontextmenu(NPP instance, NPMenu* 
>     PLUGIN_LOG_DEBUG_FUNCTION;
>     AssertPluginThread();
> 
>-#ifdef OS_MACOSX
>+#ifdef MOZ_WIDGET_COCOA
>     double pluginX, pluginY; 
>     double screenX, screenY;
> 
>@@ -2191,7 +2191,7 @@ PluginModuleChild::ResetEventHooks()
> }
> #endif
> 
>-#ifdef OS_MACOSX
>+#ifdef MOZ_WIDGET_COCOA
> void
> PluginModuleChild::ProcessNativeEvents() {
>     CallProcessSomeEvents();    
>diff -Naurp mozilla-central.orig/dom/plugins/PluginModuleChild.h mozilla-central/dom/plugins/PluginModuleChild.h
>--- dom/plugins/PluginModuleChild.h	2010-12-14 17:02:43.000000000 -0800
>+++ dom/plugins/PluginModuleChild.h	2010-12-21 14:04:20.000000000 -0800
>@@ -193,7 +193,7 @@ public:
>     static NPUTF8* NP_CALLBACK NPN_UTF8FromIdentifier(NPIdentifier aIdentifier);
>     static int32_t NP_CALLBACK NPN_IntFromIdentifier(NPIdentifier aIdentifier);
> 
>-#ifdef OS_MACOSX
>+#ifdef MOZ_WIDGET_COCOA
>     void ProcessNativeEvents();
>     
>     void PluginShowWindow(uint32_t window_id, bool modal, CGRect r) {
>diff -Naurp mozilla-central.orig/dom/plugins/PluginModuleParent.cpp mozilla-central/dom/plugins/PluginModuleParent.cpp
>--- dom/plugins/PluginModuleParent.cpp	2010-12-14 17:02:44.000000000 -0800
>+++ dom/plugins/PluginModuleParent.cpp	2010-12-21 14:04:20.000000000 -0800
>@@ -915,9 +915,11 @@ CAUpdate(nsITimer *aTimer, void *aClosur
>     nsTObserverArray<PluginInstanceParent*> *ips =
>         static_cast<nsTObserverArray<PluginInstanceParent*> *>(aClosure);
>     nsTObserverArray<PluginInstanceParent*>::ForwardIterator iter(*ips);
>+#ifdef MOZ_WIDGET_COCOA
>     while (iter.HasMore()) {
>         iter.GetNext()->Invalidate();
>     }
>+#endif // MOZ_WIDGET_COCOA
> }
> 
> void
>diff -Naurp mozilla-central.orig/modules/plugin/base/src/nsNPAPIPlugin.cpp mozilla-central/modules/plugin/base/src/nsNPAPIPlugin.cpp
>--- modules/plugin/base/src/nsNPAPIPlugin.cpp	2010-12-14 17:03:31.000000000 -0800
>+++ modules/plugin/base/src/nsNPAPIPlugin.cpp	2010-12-21 14:04:20.000000000 -0800
>@@ -83,7 +83,7 @@
> #include "nsIObserverService.h"
> #include <prinrval.h>
> 
>-#ifdef XP_MACOSX
>+#ifdef MOZ_WIDGET_COCOA
> #include <Carbon/Carbon.h>
> #include <ApplicationServices/ApplicationServices.h>
> #include <OpenGL/OpenGL.h>
>diff -Naurp mozilla-central.orig/toolkit/library/Makefile.in mozilla-central/toolkit/library/Makefile.in
>--- toolkit/library/Makefile.in	2010-12-14 17:03:37.000000000 -0800
>+++ toolkit/library/Makefile.in	2010-12-21 14:04:20.000000000 -0800
>@@ -194,6 +194,9 @@ EXTRA_DSO_LDOPTS += \
> 	-framework CoreAudio \
> 	-framework AudioToolbox \
> 	-framework AudioUnit \
>+	-framework IOKit \
>+	-framework Foundation \
>+	-framework AppKit \
> 	$(NULL)
> endif
> endif

Comment 18

[Jeremy Huddleston]

ugg... sorry for the spammy comment.  I was just trying to change the reviewer.

Comment 19

[Jeremy Huddleston]

This is a reviewed patch.  Why hasn't this been committed?  4.0, 4.0.1, and now 5.0 have blown by without this.  Please commit!

Comment 20

[Dão Gottwald [:dao]]

Please attach an updated patch, this one doesn't apply anymore.

Comment 21

[Jeremy Huddleston]

It applies to 5.0 just fine!  We're applying this patch against 5.0 in MacPorts.

I'm not going to bother wasting my time to update the fuzz yet again only to have it bitrot here for another 6 months waiting for someone to apply it (only to have it yet-again not apply!)  That is ludicrous.  I'm sure you can resolve any conflicts that are there.

Comment 22

[Benoit Girard (:BenWa)]

(In reply to comment #21)
> It applies to 5.0 just fine!  We're applying this patch against 5.0 in
> MacPorts.
> 
> I'm not going to bother wasting my time to update the fuzz yet again only to
> have it bitrot here for another 6 months waiting for someone to apply it
> (only to have it yet-again not apply!)  That is ludicrous.  I'm sure you can
> resolve any conflicts that are there.

I'm going to make sure it gets checked in from here, don't worry about the rot.

I'm sorry this is taking so long to get in, part of the delay was because it never received the right |checkin-needed| flag up until recently. This lead to the bug falling under the radar so I understand your frustration. Thanks for contributing we really do appreciate!

Comment 23

[Benoit Girard (:BenWa)]

Attachment: Use MOZ_WIDGET_COCOA instead of OS_MACOSX updated for trunk

Fixed bit rot, carrying forward r=benjamin.

Just for your info, the problem was not the bitrot, it was the diff format used. The paths omitted the |ipc| folder.

Comment 24

[Benoit Girard (:BenWa)]

(In reply to comment #23)
> Just for your info, the problem was not the bitrot, it was the diff format
> used. The paths omitted the |ipc| folder.

Actually now that I think of it the files must of been recently moved with the refactoring. Disregard that.

Comment 25

[Jeremy Huddleston]

Thanks for your help.  Sorry for venting.

Comment 26

[Benoit Girard (:BenWa)]

(In reply to comment #25)
> Thanks for your help.  Sorry for venting.

No problem, I hope this wont stop you from contributing again. 

I think there are a few patches that are going to land soon that are also incorrectly using XP_MACOSX part of async plugin. Please let me know if you encounter this problem and CC me on the bug and I'll make sure a fix gets landed quickly.

Comment 27

[Hanspeter Niederstrasser]

(In reply to comment #23)
> Created attachment 541932 [details] [diff] [review] [review]
> Use MOZ_WIDGET_COCOA instead of OS_MACOSX updated for trunk
> 
> Fixed bit rot, carrying forward r=benjamin.

This new patch applies cleanly here (unsurprisingly) to mozilla-central, but the build fails:

g++-4.2 -arch i386 -o PluginIdentifierChild.o -c  -fvisibility=hidden -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DOSTYPE=\"Darwin9.8.0\" -DOSARCH=Darwin -DEXCLUDE_SKIA_DEPENDENCIES -DCHROMIUM_MOZILLA_BUILD  -DOS_MACOSX=1 -DOS_POSIX=1  -DFORCE_PR_LOG -I/src/mozilla-central/dom/plugins/ipc/../base -I/src/mozilla-central/xpcom/base/  -I/src/mozilla-central/ipc/chromium/src -I/src/mozilla-central/ipc/glue -I../../../ipc/ipdl/_ipdlheaders  -I/src/mozilla-central/dom/plugins/ipc -I. -I../../../dist/include -I../../../dist/include/nsprpub  -I/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/dist/include/nspr -I/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/dist/include/nss -I/sw/lib/libjpeg8-turbo/include      -fPIC -I/sw/lib/libjpeg8-turbo/include -I/sw/include -I/usr/X11R6/include -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fno-common -fshort-wchar -pthread  -DNDEBUG -DTRIMMED -g -I/sw/include/gtk-2.0 -I/sw/lib/gtk-2.0/include -I/sw/include/atk-1.0 -I/sw/include/cairo -I/sw/include/pango-1.0 -I/sw/include/glib-2.0 -I/sw/lib/glib-2.0/include -I/sw/include/freetype2 -I/sw/include -I/sw/include/gtk-unix-print-2.0 -I/usr/X11R6/include -I/usr/X11/include  -I/sw/lib/libjpeg8-turbo/include -I/sw/include -I/usr/X11R6/include -DMOZILLA_CLIENT -include ../../../mozilla-config.h -MD -MF .deps/PluginIdentifierChild.pp /src/mozilla-central/dom/plugins/ipc/PluginIdentifierChild.cpp
In file included from /src/mozilla-central/dom/plugins/ipc/PluginModuleChild.h:59,
                 from /src/mozilla-central/dom/plugins/ipc/PluginIdentifierChild.cpp:41:
/src/mozilla-central/dom/plugins/ipc/PluginInterposeOSX.h:53: error: using typedef-name 'Cursor' after 'struct'
/usr/X11R6/include/X11/X.h:103: error: 'Cursor' has a previous declaration here
/src/mozilla-central/dom/plugins/ipc/PluginInterposeOSX.h:58: error: using typedef-name 'Cursor' after 'struct'
/usr/X11R6/include/X11/X.h:103: error: 'Cursor' has a previous declaration here
/src/mozilla-central/dom/plugins/ipc/PluginInterposeOSX.h:58: error: invalid type in declaration before ';' token
/src/mozilla-central/dom/plugins/ipc/PluginInterposeOSX.h:58: error: conflicting declaration 'typedef int Cursor'
/usr/X11R6/include/X11/X.h:103: error: 'Cursor' has a previous declaration as 'typedef XID Cursor'
/src/mozilla-central/dom/plugins/ipc/PluginInterposeOSX.h:121: error: 'CGRect' has not been declared
In file included from ../../../ipc/ipdl/_ipdlheaders/mozilla/plugins/PPluginModule.h:20,
                 from ../../../ipc/ipdl/_ipdlheaders/mozilla/plugins/PPluginModuleChild.h:9,
                 from /src/mozilla-central/dom/plugins/ipc/PluginModuleChild.h:62,
                 from /src/mozilla-central/dom/plugins/ipc/PluginIdentifierChild.cpp:41:
../../../dist/include/mozilla/plugins/PluginMessageUtils.h:62: error: redefinition of 'class mac_plugin_interposing::NSCursorInfo'
/src/mozilla-central/dom/plugins/ipc/PluginInterposeOSX.h:65: error: previous definition of 'class mac_plugin_interposing::NSCursorInfo'
make[6]: *** [PluginIdentifierChild.o] Error 1


dom/plugins/ipc/PluginModuleChild.h:58 uses OS_MACOSX, so I changed that to MOZ_WIDGET_COCOA to match the rest and the build finished then successfully.

*******************

diff --git a/dom/plugins/ipc/PluginModuleChild.h b/dom/plugins/ipc/PluginModuleChild.h
--- a/dom/plugins/ipc/PluginModuleChild.h
+++ b/dom/plugins/ipc/PluginModuleChild.h
@@ -50,17 +50,17 @@
 #include "npapi.h"
 #include "npfunctions.h"
 
 #include "nsAutoPtr.h"
 #include "nsDataHashtable.h"
 #include "nsTHashtable.h"
 #include "nsHashKeys.h"
 
-#ifdef OS_MACOSX
+#ifdef MOZ_WIDGET_COCOA
 #include "PluginInterposeOSX.h"
 #endif
 
 #include "mozilla/plugins/PPluginModuleChild.h"
 #include "mozilla/plugins/PluginInstanceChild.h"
 #include "mozilla/plugins/PluginIdentifierChild.h"
 
 // NOTE: stolen from nsNPAPIPlugin.h

*******************

For what it's worth, I can't view PDFs in browser anymore.  In firefox4, I use mozplugger to load xpdf inside the browser (my only other darwin/x11 plugin to test is gnash, but that's currently broken for other reasons), but with firefox5 as well as with mozilla-central, the browser becomes unresponsive for 5-10 seconds, and then I just get a solid black content area.

Comment 28

[Benoit Girard (:BenWa)]

Removing checkin-needed until we get this issue fixed.

I'm not entirely sure what the darwin/x11 configuration looks like but you wont be able to run out-of-process plugin until it's been implemented properly for that platform. Is it trying to run plug-ins out of process? If so you can try to disable them from the |dom.ipc.plugins.enabled.*| prefs.

Comment 29

[Hanspeter Niederstrasser]

Aha.  On fx4, dom.ipc.plugins.enabled defaults to false.  On fx5+, it defaults to true.  Changing to false loads the plugin w/out any problems.

Yeah, I was pretty sure that OOP would not work on darwin/x11 w/out some serious work under the hood, so that wasn't much of an issue for me, but I did not realize that there was a pref that could control it.

Comment 30

[Jeremy Huddleston]

Benoit, I doubt out-or-process plugins ever worked on darwin/x11, so there's no reason to think that this build fix causes such a regression.  Please get this checked in (including the fix for the newer regressions mentioned by Hanspeter in the comment above at the same time would be good).

Comment 31

[Jeremy Huddleston]

In other words, there is no reason that an existing functionality problem (plugin support on darwin/x11) should block a build fix.  If we can't even build firefox on darwin/x11, why would we care about the plugin support... ;)

Comment 32

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/9a2eca570c4a

Comment 33

[Benoit Girard (:BenWa)]

(In reply to comment #31)
> In other words, there is no reason that an existing functionality problem
> (plugin support on darwin/x11) should block a build fix.  If we can't even
> build firefox on darwin/x11, why would we care about the plugin support... ;)

My intention was not to wait for plugins to be fixed. It was just to wait for until I had a patch with the issue in Comment 27 before this got check-in.

I'll prepare a follow up patch for the missing change in Comment 27 Monday and get that landed.

Comment 34

[Mounir Lamouri (:mounir)]

Merged:
http://hg.mozilla.org/mozilla-central/rev/9a2eca570c4a

Comment 35

[Benoit Girard (:BenWa)]

Attachment: Part 2: Use MOZ_WIDGET_COCOA instead of OS_MACOSX

One replacement was missed: Comment 27

Comment 36

[Benoit Girard (:BenWa)]

checkin required for part 2

Comment 37

[Joe Drew (not getting mail)]

http://hg.mozilla.org/mozilla-central/rev/34e102b130e9

Comment 38

[Vlad [QA]]

How can this be verified? Can you please provide some STR or a testcase.
Thanks

Comment 39

[(not currently active) Ted Mielczarek]

You don't need to verify this bug. It's about a build error in a non-default configuration.

Comment 40

[Jeremy Huddleston]

Still broken in 6.0:


In file included from /opt/local/var/macports/build/_Users_jeremy_src_macports_trunk_dports_www_firefox-x11-devel/firefox-x11-devel/work/mozilla-release/dom/plugins/base/nsNPAPIPlugin.cpp:116:
In file included from ../../../dist/include/mozilla/plugins/PluginModuleParent.h:56:
../../../dist/include/mozilla/plugins/PluginInstanceParent.h:358:5: error: unknown type name 'CGColorSpaceRef'
    CGColorSpaceRef    mShColorSpace;
    ^
../../../dist/include/mozilla/plugins/PluginInstanceParent.h:360:5: error: unknown type name 'nsIOSurface'
    nsIOSurface       *mIOSurface;
    ^

Comment 41

[Benoit Girard (:BenWa)]

Any chance you can provide a patch? I can make sure it get landed ASAP.

Comment 42

[Jeremy Huddleston]

It looks like the commits were just not cherry picked into the 6.0 release ... oh well.

Comment 43

[Jeremy Huddleston]

and unfortunately the changes don't apply cleanly to 6.0 =/

Comment 44

[Hanspeter Niederstrasser]

Attachment: patch for firefox-6.0

This is my revised patch that patches cleanly into firefox 6 release.  If I remember the way the current update channels are going, the original patch went into mozilla-central into what will become firefox7 (definitely will be in firefox8).

Comment 45

[Benoit Girard (:BenWa)]

Ohh alright. I though there was further problems with current trunk. I don't think we will be able to get approval to land changes on a release branch unfortunately. Can you confirm that there are no problems with the current trunk so that we can land patches to fix them now if required?

Comment 46

[Jeremy Huddleston]

Thanks for the 6.0 patch.

Release tarballs have been better than trunk due to autoconf nightmares, but it would be good to figure all of that out.  I'm trying to build trunk, but it's difficult to work with.  Thanks.

Comment 47

[Jeremy Huddleston]

trunk builds, as does 6.0 with the patch in comment #44, thanks.

Comment 48

[u279076]

QA will not be tracking this bug for verification.