630703 - Provide a way to restart in 32-bit mode

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Dave Townsend [:mossop]]

Bug 628651 needs a way to restart the app into 32-bit mode. This bug will implement just that.

Comment 1

[Dave Townsend [:mossop]]

Attachment: WIP

This looks like it should work based on printf's I stuck in to verify it was following the right path, waiting on a universal build to actually test though.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Removing from b11 radar, hoping to have it on trunk for Friday.

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 508936 [details] [diff] [review]
WIP

Verified this manually now. The behaviour is that if restarting with eRestart32bit then it restarts in 32 bit, if restarting with eRestart64bit then it restarts in 64 bit and if neither it restarts in whatever architecture it is already running.

We don't really need the 64 bit bit at this point but it is easy enough for consistency.

I don't think we have a straightforward way to automatically test this do we?

Comment 4

[Robert Strong (they/them - no direct email)]

I suspect that it is possible to test most of this on Mac OS X 10.6 using a similar methodology as I used with
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/test/unit/test_0200_app_launch_apply_update.js

The parts that I can think of that are missing is verifying which arch is launched and only launching when testing with a Universal binary.

Comment 5

[Benjamin Smedberg]

Comment on attachment 508936 [details] [diff] [review]
WIP

You're going to get a mac person to review this also, right?

Comment 6

[Dave Townsend [:mossop]]

Attachment: mozmill tests

The most straightforward way I could find this was to use mozmill. The tests here verify that on first run it is in 64-bit mode, a restart leaves it in 64-bit mode, a restart to 32-bit mode works, a restart leaves it in 32-bit mode and then a restart to 64-bit mode works.

The tests all skip if running on non-OSX or if earlier than 10.6 (confusingly the system-info version property holds "9" for OSX 10.5).

Comment 7

[Josh Aas]

Comment on attachment 508936 [details] [diff] [review]
WIP

Can you do "eRestarti386" and "eRestartx86_64" instead of 32-bit and 64-bit? Ultimately you have to refer to a particular architecture and there just happens to be one choice right now...

Comment 8

[Josh Aas]

Comment on attachment 508936 [details] [diff] [review]
WIP

Everything else looks fine to me but I'd like to see a patch with 32-bit and 64-bit references changed to specific architectures in nsIAppStartup.idl and MacLaunchHelper.mm.

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 509251 [details] [diff] [review]
mozmill tests

Going to work on the test in bug 631052

Comment 10

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Patch with the naming changes

Comment 11

[Dave Townsend [:mossop]]

Landed, will look after the tests in bug 631052.

http://hg.mozilla.org/mozilla-central/rev/f00b81064d57

Comment 12

[Craig Topper]

I think this broke non-libxul builds. I'm getting this error.

Undefined symbols:
  "_gRestartMode", referenced from:
      LaunchChild(nsINativeAppSupport*, int)   in nsAppRunner.o
ld: symbol(s) not found
collect2: ld returned 1 exit status

Comment 13

[Dave Townsend [:mossop]]

(In reply to comment #12)
> I think this broke non-libxul builds. I'm getting this error.
> 
> Undefined symbols:
>   "_gRestartMode", referenced from:
>       LaunchChild(nsINativeAppSupport*, int)   in nsAppRunner.o
> ld: symbol(s) not found
> collect2: ld returned 1 exit status

See bug 631564

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Those attributes have not been documented yet on MDN. Setting dev-doc-needed flag.