Last Comment Bug 659817 - [10.7 SDK] Build error in PluginInterposeOSX.h due to Cursor type not being defined
: [10.7 SDK] Build error in PluginInterposeOSX.h due to Cursor type not being d...
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Steven Michaud [:smichaud] (Retired)
:
:
Mentors:
Depends on:
Blocks: 661638
  Show dependency treegraph
 
Reported: 2011-05-25 16:31 PDT by Jesse Ruderman
Modified: 2011-06-11 06:39 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch for Workaround (Not a Fix) (392 bytes, patch)
2011-05-31 08:25 PDT, Stefan Arentz [:st3fan]
no flags Details | Diff | Splinter Review
Fix (2.14 KB, patch)
2011-06-02 15:07 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Splinter Review
Fix rev1 (fix problems) (4.18 KB, patch)
2011-06-10 08:35 PDT, Steven Michaud [:smichaud] (Retired)
b56girard: review+
Details | Diff | Splinter Review
Copy of rev1 patch for someone else to land (4.40 KB, patch)
2011-06-10 14:06 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details | Diff | Splinter Review

Description Jesse Ruderman 2011-05-25 16:31:44 PDT
I'm trying to build a (64-bit) Firefox on Mac OS X 10.7 (DP) with XCode 4.1 (DP), and I've worked around bug 655339.

I get an error at PluginInterposeOSX.h:72 :(

http://hg.mozilla.org/mozilla-central/annotate/5e1ee7f192ad/dom/plugins/ipc/PluginInterposeOSX.h#l72

> g++-4.2 -o PPluginBackgroundDestroyerParent.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=\"Darwin11.0.0\" -DOSARCH=Darwin -DEXCLUDE_SKIA_DEPENDENCIES -DCHROMIUM_MOZILLA_BUILD  -DOS_MACOSX=1 -DOS_POSIX=1  -I../../ipc/ipdl/_ipdlheaders -I/Users/jruderman/mozilla-central/ipc/chromium/src -I/Users/jruderman/mozilla-central/ipc/glue -I../../ipc/ipdl/_ipdlheaders  -I/Users/jruderman/mozilla-central/ipc/ipdl -I. -I../../dist/include -I../../dist/include/nsprpub  -I/Users/jruderman/mozilla-central/obj-ff-dbg-64/dist/include/nspr -I/Users/jruderman/mozilla-central/obj-ff-dbg-64/dist/include/nss       -fPIC  -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 -fno-strict-aliasing -fno-common -fshort-wchar -pthread -DNO_X11 -pipe  -DDEBUG -D_DEBUG -DTRACING -g -DNO_X11   -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MF .deps/PPluginBackgroundDestroyerParent.pp /Users/jruderman/mozilla-central/obj-ff-dbg-64/ipc/ipdl/PPluginBackgroundDestroyerParent.cpp
> In file included from ../../dist/include/mozilla/plugins/PluginMessageUtils.h:60,
>                  from ../../ipc/ipdl/_ipdlheaders/mozilla/plugins/PPluginInstance.h:19,
>                  from ../../ipc/ipdl/_ipdlheaders/mozilla/plugins/PPluginInstanceParent.h:9,
>                  from /Users/jruderman/mozilla-central/obj-ff-dbg-64/ipc/ipdl/PPluginBackgroundDestroyerParent.cpp:9:
> ../../dist/include/mozilla/plugins/PluginInterposeOSX.h:72: error: expected ‘,’ or ‘...’ before ‘*’ token
> ../../dist/include/mozilla/plugins/PluginInterposeOSX.h:72: error: ISO C++ forbids declaration of ‘Cursor’ with no type
> make[6]: *** [PPluginBackgroundDestroyerParent.o] Error 1
> make[5]: *** [libs] Error 2
> make[4]: *** [libs_tier_platform] Error 2
> make[3]: *** [tier_platform] Error 2
> make[2]: *** [default] Error 2
> make[1]: *** [realbuild] Error 2
> make: *** [build] Error 2

Seems like the "Cursor" type doesn't exist. Where is it defined on Snow Leopard?

This happens in code that was added yesterday in bug 621117 (http://hg.mozilla.org/mozilla-central/rev/6bf3ffd66eed)
Comment 1 Steven Michaud [:smichaud] (Retired) 2011-05-25 16:37:14 PDT
I had no problems building this on SnowLeopard -- 32-bit and 64-bit.

And it'll be a long time before we use Lion (10.7) to do any official builds (even trunk nightlies) -- they almost certainly won't run on OS X 10.5.X, which we still need to support.

So this bug doesn't appear to be urgent.

But I will try to figure out what's going on.
Comment 2 Steven Michaud [:smichaud] (Retired) 2011-05-25 16:50:23 PDT
> Seems like the "Cursor" type doesn't exist. Where is it defined on
> Snow Leopard?

It's defined in QuickdrawTypes.h on OS X 10.6, which is one of the
header files for the QD framework (under the ApplicationServices
framework).  I'm not entirely sure how it gets included on 10.6.

But on 10.7, though the QD framework still exists, and still has
header files, none of them defines Cursor.

I'll need some time to figure out what to do about this.
Comment 3 Steven Michaud [:smichaud] (Retired) 2011-05-25 17:01:25 PDT
Have you tried doing a 32-bit build?

Do you get similar errors with that?
Comment 4 Jesse Ruderman 2011-05-25 20:11:40 PDT
With a 32-bit build, I hit bug 659851. I don't know if that happens "before" or "after" this one, so I don't know whether this bug affects 32-bit builds.
Comment 5 Steven Michaud [:smichaud] (Retired) 2011-05-26 10:20:04 PDT
I don't see bug 659851 or this bug doing a 32-bit build -- which is a
relief.

Now I'll try to reproduce (and address) the problem using 64-bit
builds.
Comment 6 Steven Michaud [:smichaud] (Retired) 2011-05-26 11:34:44 PDT
> Now I'll try to reproduce (and address) the problem using 64-bit
> builds.

I'm not able to reproduce this bug doing a 64-bit build, either.  I
suspect it's because I've been using the 10.6 SDK ("ac_add_options
--with-macos-sdk=/Developer/SDKs/MacOSX10.6.sdk").
Comment 7 Steven Michaud [:smichaud] (Retired) 2011-05-26 11:49:49 PDT
I suspect this bug only happens if you either use the 10.7 SDK or
don't specify an SDK (which would make the build use currently
installed system files).

Currently we default (on the trunk) to using the 10.5 SDK to do 32-bit
builds, and the 10.6 SDK to do 64-bit builds -- the 32-bit binaries
need to support OS X 10.5 and up, and the 64-bit binaries need to
support OS X 10.6 and up.

XCode 4.1 for OS X 10.7 doesn't have a 10.5 SDK.  So it'll be a while
before we can use it to do any kind of official build (even
nightlies).  And it'll be even longer before we can use the 10.7 SDK
on OS X 10.7.

We'll need to deal with this bug eventually, but it can't be
considered a blocker.
Comment 8 Steven Michaud [:smichaud] (Retired) 2011-05-26 13:44:02 PDT
When building with the 10.7 SDK, I see this bug doing both 64-bit and 32-bit builds.
Comment 9 Nomis101 2011-05-28 10:55:46 PDT
I also see this error if I build with the 10.7 SDK. It builds fine if I use the 10.6 SDK. The error is a bit different with GCC and Clang.

With GCC I get:

In file included from ../../dist/include/mozilla/plugins/PluginMessageUtils.h:60,
                 from ../../ipc/ipdl/_ipdlheaders/mozilla/plugins/PPluginScriptableObject.h:21,
                 from ../../ipc/ipdl/_ipdlheaders/mozilla/plugins/PPluginScriptableObjectParent.h:9,
                 from /Volumes/Developer/temp/src/mozilla/obj-x86_64-apple-darwin11.0.0/ipc/ipdl/PPluginScriptableObjectParent.cpp:7:
../../dist/include/mozilla/plugins/PluginInterposeOSX.h:72: error: expected ‘,’ or ‘...’ before ‘*’ token
../../dist/include/mozilla/plugins/PluginInterposeOSX.h:72: error: ISO C++ forbids declaration of ‘Cursor’ with no type
make[6]: *** [PPluginIdentifierParent.o] Error 1
make[6]: *** Waiting for unfinished jobs....


With Clang I get:

In file included from ../../ipc/ipdl/_ipdlheaders/mozilla/plugins/PPluginInstance.h:19:
In file included from ../../dist/include/mozilla/plugins/PluginMessageUtils.h:60:
../../dist/include/mozilla/plugins/PluginInterposeOSX.h:72:22: error: unknown
      type name 'Cursor'
  NSCursorInfo(const Cursor* aCursor);
                     ^
6 warnings and 1 error generated.
make[6]: *** [PStreamNotifyParent.o] Error 1
make[5]: *** [libs] Error 2
make[4]: *** [libs_tier_platform] Error 2
make[3]: *** [tier_platform] Error 2
make[2]: *** [default] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2
Comment 10 Stefan Arentz [:st3fan] 2011-05-30 19:35:09 PDT
This is the workaround to get a build going:

I commented out the constructor that takes the Cursor and also the implementation. Also changed the signature of mac_plugin_interposing_child_OnSetCursor to take "const void* cursor" and commented out the code in it.

So this is interesting. The plugin API depends on QuickDraw APIs. But those are deprecated in 10.7.
Comment 11 Nomis101 2011-05-31 00:26:03 PDT
Nice that you were able to workround this. Do you have a patch with your workaround, so I can test if this also will work for me (or maybe a fix)?
Comment 12 Steven Michaud [:smichaud] (Retired) 2011-05-31 07:23:35 PDT
I've got a patch for this ... though it needs a bit more work.

SetCursor() is not just deprecated, but actually missing, on OS X 10.7 (even in 32-bit mode).  Any patch for this bug needs to work around that.

I'll get back to work on this after I get back from vacation.
Comment 13 Stefan Arentz [:st3fan] 2011-05-31 08:25:01 PDT
Created attachment 536308 [details] [diff] [review]
Patch for Workaround (Not a Fix)

Workaround for the Cursor problem in PluginInterposeOSX. This is only needed when compiling against the 10.7 SDK.
Comment 14 Nomis101 2011-06-02 13:53:47 PDT
(In reply to comment #13)
> Created attachment 536308 [details] [diff] [review] [review]
> Patch for Workaround (Not a Fix)
> 
> Workaround for the Cursor problem in PluginInterposeOSX. This is only needed
> when compiling against the 10.7 SDK.

Sorry, this doesn't work for me. :(

/Volumes/Developer/temp/src/mozilla/dom/plugins/ipc/PluginInterposeOSX.mm:161:34: error: 
      unknown type name 'Cursor'
NSCursorInfo::NSCursorInfo(const Cursor* aCursor)
                                 ^
/Volumes/Developer/temp/src/mozilla/dom/plugins/ipc/PluginInterposeOSX.mm:1069:48: error: 
      unknown type name 'Cursor'
mac_plugin_interposing_child_OnSetCursor(const Cursor* cursor)
                                               ^
6 warnings and 2 errors generated.
make[6]: *** [PluginInterposeOSX.o] Error 1
make[5]: *** [libs] Error 2
make[4]: *** [libs_tier_platform] Error 2
make[3]: *** [tier_platform] Error 2
make[2]: *** [default] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2
Comment 15 Stefan Arentz [:st3fan] 2011-06-02 14:36:41 PDT
Looks like the patch that I uploaded is missing some changes. I will upload a new patch later tonight.
Comment 16 Steven Michaud [:smichaud] (Retired) 2011-06-02 15:07:35 PDT
Created attachment 536995 [details] [diff] [review]
Fix

> SetCursor() is not just deprecated, but actually missing, on OS X
> 10.7 (even in 32-bit mode).  Any patch for this bug needs to work
> around that.

Turns out I was wrong -- SetCursor() *is* present in the 32-bit binary
of the QD framework, and can be linked to.  It's just that it (and
many other QuickDraw APIs) are missing from OS X 10.7's QuickDraw
header files.  (I've confirmed that SetCursor() is not only present
but functional -- the Silverlight plugin uses it successfully on OS X
10.7.  See bug 633034 comment #0 for a testcase.)

Which is just as well, since it makes this bug easier to fix.

Benoit:  I'd normally ask Josh to review this patch, but I know he's
very busy with his new responsibilities.

If you don't feel comfortable reviewing this, I can still pass it to
Josh -- this bug isn't particularly urgent, so it doesn't matter if
the review takes a while.
Comment 17 Steven Michaud [:smichaud] (Retired) 2011-06-02 15:20:10 PDT
Comment on attachment 536995 [details] [diff] [review]
Fix

Oops, this doesn't work on versions of OS X other than 10.7.

I'll post another patch later -- probably tomorrow.
Comment 18 Steven Michaud [:smichaud] (Retired) 2011-06-10 08:35:13 PDT
Created attachment 538522 [details] [diff] [review]
Fix rev1 (fix problems)
Comment 19 Benoit Girard (:BenWa) 2011-06-10 10:12:00 PDT
Comment on attachment 538522 [details] [diff] [review]
Fix rev1 (fix problems)

Review of attachment 538522 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/ipc/interpose/plugin_child_interpose.mm
@@ +117,5 @@
>  }
>  
>  static void MacPluginChildSetCursor(const Cursor* cursor)
>  {
> +  if (::SetCursor) {

I think this isn't required but it can't hurt.
Comment 20 Steven Michaud [:smichaud] (Retired) 2011-06-10 14:06:27 PDT
Created attachment 538603 [details] [diff] [review]
Copy of rev1 patch for someone else to land
Comment 21 Steven Michaud [:smichaud] (Retired) 2011-06-10 14:10:50 PDT
>>  }
>>
>>  static void MacPluginChildSetCursor(const Cursor* cursor)
>>  {
>> +  if (::SetCursor) {
>
> I think this isn't required but it can't hurt.

I agree.

Thanks, Benoit!
Comment 22 Dão Gottwald [:dao] 2011-06-11 04:37:53 PDT
http://hg.mozilla.org/mozilla-central/rev/1b97769e6549

Note You need to log in before you can comment on or make changes to this bug.