darwin/x11 gtk build fails in PluginInstanceParent.h

RESOLVED FIXED in Firefox 7

Status

()

Core
Plug-ins
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Jeremy Huddleston, Assigned: BenWa)

Tracking

({regression})

Trunk
mozilla7
All
Mac OS X
regression
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox7 fixed)

Details

(Whiteboard: [qa-])

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

7 years ago
../../../../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...
(Reporter)

Comment 1

7 years ago
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)
 };
(Reporter)

Comment 2

7 years ago
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
(Reporter)

Comment 3

7 years ago
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
Blocks: 555281
Keywords: regression
(Assignee)

Comment 4

7 years ago
(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?
(Reporter)

Comment 5

7 years ago
(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
Created attachment 487069 [details] [diff] [review]
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.
(Reporter)

Updated

7 years ago
Attachment #487069 - Flags: review?
(Reporter)

Comment 7

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

Updated

7 years ago
Attachment #487069 - Flags: review? → review?(sgreenlay)
(Reporter)

Comment 8

7 years ago
Created attachment 493526 [details] [diff] [review]
updated to apply to 4.0b7
Attachment #493526 - Flags: review?
(Reporter)

Updated

7 years ago
Attachment #493526 - Flags: review? → review?(sgreenlay)
(Reporter)

Comment 9

7 years ago
Created attachment 493533 [details] [diff] [review]
Use MOZ_WIDGET_COCOA instead of OS_MACOSX

Missed one spot.  This compiles, and firefox runs.
Attachment #487069 - Attachment is obsolete: true
Attachment #493526 - Attachment is obsolete: true
Attachment #493533 - Flags: review?(sgreenlay)
Attachment #487069 - Flags: review?(sgreenlay)
Attachment #493526 - Flags: review?(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?
Attachment #493533 - Flags: review?(sgreenlay) → review-
(Reporter)

Comment 11

7 years ago
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
Attachment #493533 - Flags: review- → review?(sgreenlay)
(Reporter)

Comment 12

7 years ago
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.
(Reporter)

Comment 13

7 years ago
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.
(Reporter)

Comment 14

7 years ago
Created attachment 499147 [details] [diff] [review]
Use MOZ_WIDGET_COCOA instead of OS_MACOSX updated for 4.0b8
Attachment #493533 - Attachment is obsolete: true
Attachment #499147 - Flags: review?(scott)
Attachment #493533 - Flags: review?(scott)
(Reporter)

Comment 15

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

Comment 16

7 years ago
This is still not in 4.0b11 ... Why is there such a delay here?
(Reporter)

Comment 17

7 years ago
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
Attachment #499147 - Flags: review?(scott) → review?(benjamin)
(Reporter)

Comment 18

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

Updated

7 years ago
Attachment #499147 - Flags: review?(benjamin) → review+
(Reporter)

Comment 19

6 years ago
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!
(Reporter)

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → jeremyhu
Please attach an updated patch, this one doesn't apply anymore.
Keywords: checkin-needed
(Reporter)

Comment 21

6 years ago
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.
(Assignee)

Comment 22

6 years ago
(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!
Assignee: jeremyhu → bgirard
(Assignee)

Comment 23

6 years ago
Created attachment 541932 [details] [diff] [review]
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.
Attachment #499147 - Attachment is obsolete: true
Attachment #541932 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Comment 24

6 years ago
(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.
(Reporter)

Comment 25

6 years ago
Thanks for your help.  Sorry for venting.
(Assignee)

Comment 26

6 years ago
(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.
(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.
(Assignee)

Comment 28

6 years ago
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.
Keywords: checkin-needed
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.
(Reporter)

Comment 30

6 years ago
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).
Keywords: checkin-needed
(Reporter)

Comment 31

6 years ago
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... ;)
http://hg.mozilla.org/integration/mozilla-inbound/rev/9a2eca570c4a
Status: NEW → ASSIGNED
Flags: in-testsuite-
Keywords: checkin-needed
Whiteboard: [inbound]
Target Milestone: --- → mozilla7
(Assignee)

Comment 33

6 years ago
(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.
Merged:
http://hg.mozilla.org/mozilla-central/rev/9a2eca570c4a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [inbound]
(Assignee)

Comment 35

6 years ago
Created attachment 542189 [details] [diff] [review]
Part 2: Use MOZ_WIDGET_COCOA instead of OS_MACOSX

One replacement was missed: Comment 27
Attachment #542189 - Flags: review?(benjamin)
(Assignee)

Updated

6 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

6 years ago
Attachment #542189 - Flags: review?(benjamin) → review+
(Assignee)

Updated

6 years ago
Attachment #541932 - Flags: checkin+
(Assignee)

Comment 36

6 years ago
checkin required for part 2
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
status-firefox7: --- → fixed
Keywords: checkin-needed
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/34e102b130e9
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]

Comment 38

6 years ago
How can this be verified? Can you please provide some STR or a testcase.
Thanks
You don't need to verify this bug. It's about a build error in a non-default configuration.
(Reporter)

Comment 40

6 years ago
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;
    ^
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 41

6 years ago
Any chance you can provide a patch? I can make sure it get landed ASAP.
(Reporter)

Comment 42

6 years ago
It looks like the commits were just not cherry picked into the 6.0 release ... oh well.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Reporter)

Comment 43

6 years ago
and unfortunately the changes don't apply cleanly to 6.0 =/
Created attachment 554684 [details] [diff] [review]
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).
(Assignee)

Comment 45

6 years ago
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?
(Reporter)

Comment 46

6 years ago
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.
(Reporter)

Comment 47

6 years ago
trunk builds, as does 6.0 with the patch in comment #44, thanks.
QA will not be tracking this bug for verification.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.