555234 - enable MOZ_IPC by default for i386 Mac OS X

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Josh Aas]

We should enable MOZ_IPC by default for i386 Mac OS X, but with OOPP pref'd off. We have to fix crashreporter bustage in order to do this.

For now we'll leave the test plugin as a Carbon plugin so we don't interfere with normal testing. We can switch it to Cocoa if we pref OOP on by default for i386 at some point, or we can change it so that it negotiates based on the browser pref when built as an i386 binary.

Comment 1

[Josh Aas]

Attachment: fix v1.0

Comment 2

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/3bbb3c3d3fa8

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Backed out because we have test files in x86 that aren't there for PPC.  Will try packaging the tests for both but bailing in the IPC-only tests if !OOP (as we do for mochitests).

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Enable OOPP for x86 OS X and don't export mochitests conditional on MOZ_IPC

Going through try as we speak, review is mostly for the change to test_GCRace.html.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

BTW, josh is going to add a gross hack to this patch to somehow get a file named "mozilla-runtime" into dist/bin on PPC.

Comment 6

[(not currently active) Ted Mielczarek]

It will have to be a valid Mach-O binary, because unify will assert that both files are Mach-O files if one of them is, and then it will run lipo on them to generate a universal binary. That's not to say it has to do anything useful on PPC.

Comment 7

[Josh Aas]

Attachment: fix v2.0 [Checkin: Comment 10]

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 436696 [details] [diff] [review]
fix v2.0
[Checkin: Comment 10]

This looks fine. All the rest of the IPC mochitests check the pref already, right?

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #8)
> (From update of attachment 436696 [details] [diff] [review])
> This looks fine. All the rest of the IPC mochitests check the pref already,
> right?

Yep, GCRace was the only one that didn't.

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

There was a small issue with the change to GCRace that came up on try, but green otherwise.

Pushed http://hg.mozilla.org/mozilla-central/rev/338f525fac98 on josh's behalf.

Comment 11

[Serge Gautherie (:sgautherie)]

Comment on attachment 436696 [details] [diff] [review]
fix v2.0
[Checkin: Comment 10]

>diff --git a/toolkit/toolkit-tiers.mk b/toolkit/toolkit-tiers.mk
>@@ -102,16 +102,21 @@ tier_platform_dirs += modules/lib7z
> ifdef MOZ_IPC
> tier_platform_dirs += ipc
>+else
>+# Include fake mozilla-runtime so that unify has something to unify.
>+ifeq ($(OS_ARCH)_$(TARGET_CPU),Darwin_powerpc)
>+tier_platform_dirs += ipc/app/fake
>+endif
> endif

Does this actually work when IPC is explicitly disabled (for i386)?

Comment 12

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-CC) Copy configure.in part to comm-central [Checkin: Comment 13]

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 436972 [details] [diff] [review]
(Bv1-CC) Copy configure.in part to comm-central
[Checkin: Comment 13]


http://hg.mozilla.org/comm-central/rev/c7bca06e4b24

Comment 14

[Serge Gautherie (:sgautherie)]

Comment on attachment 436696 [details] [diff] [review]
fix v2.0
[Checkin: Comment 10]

>diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in
> #ifdef XP_MACOSX
> @BINPATH@/XUL
>+@BINPATH@/mozilla-runtime@BIN_SUFFIX@

Why package it |#ifndef MOZ_IPC|?

> #else
> @BINPATH@/@DLL_PREFIX@xul@DLL_SUFFIX@
>-#endif
> #ifdef MOZ_IPC
> @BINPATH@/mozilla-runtime@BIN_SUFFIX@
> #endif
>+#endif

Comment 15

[(not currently active) Ted Mielczarek]

Because of comment 5 and comment 6. Please, read the bug if you have questions about patches.

Comment 16

[Serge Gautherie (:sgautherie)]

(In reply to comment #15)
> Please, read the bug if you have questions about patches.

I try to read, but I still don't fully understand (I'm sorry about that), so I ask questions.
To put it in other words, MacOSX SeaMonkey 2.1 (with MOZ_IPC=) seems to build fine without porting the package-manifest.in change atm, thus I wonder what the conditions for this change to be needed are...

Comment 17

[(not currently active) Ted Mielczarek]

This originally broke universal builds, because the unify step requires that both buidls (ppc/i386) have the same set of files, and the ppc half, not being built --enable-ipc, was missing mozilla-runtime. The fix is to put a fake mozilla-runtime binary there.

Comment 18

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I think  we have to have this on 1.9.2 for lorentz, right?

Comment 19

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Nevermind, it's late and I'm not thinking clearly ;)

Comment 20

[Serge Gautherie (:sgautherie)]

Ftr:


(In reply to Serge Gautherie (:sgautherie) from comment #11)
> >diff --git a/toolkit/toolkit-tiers.mk b/toolkit/toolkit-tiers.mk
> 
> Does this actually work when IPC is explicitly disabled (for i386)?

This part was later removed in bug 563747.


(In reply to Serge Gautherie (:sgautherie) from comment #14)
> >diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in
> 
> Why package it |#ifndef MOZ_IPC|?

Thanks for the explanation.
This part was eventually ported to SeaMonkey in bug 545716.