Provide a way to restart in 32-bit mode

RESOLVED FIXED in mozilla2.0b12

Status

()

defect
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

({dev-doc-needed})

Trunk
mozilla2.0b12
All
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?
in-litmus -

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [hardblocker])

Attachments

(1 attachment, 2 obsolete attachments)

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

Updated

9 years ago
blocking2.0: --- → betaN+
Assignee

Updated

9 years ago
Assignee: nobody → dtownsend
Posted patch WIP (obsolete) — Splinter Review
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.
Assignee

Updated

9 years ago
Attachment #508936 - Flags: feedback?(benjamin)
Assignee

Updated

9 years ago
Whiteboard: [hardblocker] → [hardblocker][b11]
Removing from b11 radar, hoping to have it on trunk for Friday.
Whiteboard: [hardblocker][b11] → [hardblocker][ETA Feb-04]
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?
Attachment #508936 - Flags: feedback?(benjamin) → review?(benjamin)
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.
Assignee

Updated

9 years ago
Whiteboard: [hardblocker][ETA Feb-04] → [hardblocker][has patch][needs review bsmedberg][needs tests][ETA Feb-04]

Comment 5

9 years ago
Comment on attachment 508936 [details] [diff] [review]
WIP

You're going to get a mac person to review this also, right?
Attachment #508936 - Flags: review?(benjamin) → review+
Assignee

Updated

9 years ago
Attachment #508936 - Flags: review?(joshmoz)
Assignee

Updated

9 years ago
Whiteboard: [hardblocker][has patch][needs review bsmedberg][needs tests][ETA Feb-04] → [hardblocker][has patch][needs review josh][needs tests][ETA Feb-04]
Posted patch mozmill tests (obsolete) — Splinter Review
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).
Attachment #509251 - Flags: review?(hskupin)
Assignee

Updated

9 years ago
Whiteboard: [hardblocker][has patch][needs review josh][needs tests][ETA Feb-04] → [hardblocker][has patch][needs review josh][needs review whimboo][ETA Feb-04]

Comment 7

9 years ago
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

9 years ago
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.
Attachment #508936 - Flags: review?(joshmoz) → review-
Comment on attachment 509251 [details] [diff] [review]
mozmill tests

Going to work on the test in bug 631052
Attachment #509251 - Attachment is obsolete: true
Attachment #509251 - Flags: review?(hskupin)
Assignee

Updated

9 years ago
Depends on: 631052
Whiteboard: [hardblocker][has patch][needs review josh][needs review whimboo][ETA Feb-04] → [hardblocker][has patch][needs review josh][ETA Feb-04]
Posted patch patch rev 2Splinter Review
Patch with the naming changes
Attachment #508936 - Attachment is obsolete: true
Attachment #509294 - Flags: review?(joshmoz)

Updated

9 years ago
Attachment #509294 - Flags: review?(joshmoz) → review+
Assignee

Updated

9 years ago
Whiteboard: [hardblocker][has patch][needs review josh][ETA Feb-04] → [hardblocker][has patch][ETA Feb-04]
Landed, will look after the tests in bug 631052.

http://hg.mozilla.org/mozilla-central/rev/f00b81064d57
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite?
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [hardblocker][has patch][ETA Feb-04] → [hardblocker]
Target Milestone: --- → mozilla2.0b12

Updated

9 years ago
Depends on: 631564

Comment 12

9 years ago
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
(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
Those attributes have not been documented yet on MDN. Setting dev-doc-needed flag.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.