Closed Bug 337824 Opened 17 years ago Closed 17 years ago

Profile Manager completely horked

Categories

(Toolkit :: Startup and Profile System, defect, P1)

PowerPC
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: patrick.hendriks+bugzilla, Assigned: darin.moz)

References

Details

(Keywords: regression)

Attachments

(2 files)

Using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060512 Minefield/3.0a1

starting Minefield from the Mac command line via
   Minefield.app/Contents/MacOS $ ./firefox-bin -p
shows the profile manager, however, the window that appears is not functional: 
- one is not able to select any of the other profiles
- pressing START MINEFIELD with the default selected profile results in a hang
- buttons for DELETE and CREATE work only after pressing them twice
- pressing CANCEL during the profile creation ends in a loop of two or three dialogs to start creating profiles, with the final dialog being totally blank
- pressing EXIT keeps Minefield active [without windows]
- keyboard navigations seems broken

This severely limits testing possibilities and thus marking this as a Blocker ["Blocks development and/or testing work"]
Only on Trunk, wfm with 
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1a2) Gecko/20060512 BonEcho/2.0a2
Version: unspecified → Trunk
Regression window:
WORKS : Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060510 Minefield/3.0a1

BROKEN : Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060511 Minefield/3.0a1

WFM with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060512 Minefield/3.0a1, although I've not tested the keyboard navigations.
darin, could your checkins during the period mentioned in comment 2 have caused this?
Yes, this is a regression from Darin's thread changes.
See http://pastebin.ca/55487 (and bug 337550/bug 337481).
-> me
Assignee: nobody → darin
Priority: -- → P1
Target Milestone: --- → Firefox 3
Attached patch v1 patchSplinter Review
This seems to do the trick for me.  I think this will break the camino build, which could probably use similar changes.  I'll work on those once we know we are happy with this... or contributions welcome ;-)
Attachment #222133 - Flags: review?(mark)
The xpcom-shutdown observer is needed to handle the case where nsAppStartup is not used to run the event loop and no one calls nsIAppShell::Exit.  If we don't set mExiting to true, then we'll get stuck during xpcom shutdown.
Comment on attachment 222133 [details] [diff] [review]
v1 patch

Funny - I had just independently diagnosed the cause of the hanging profile manager and was about to e-mail you along with a patch with modifications for carbon and cocoa.
Attachment #222133 - Flags: review?(mark) → review+
Attached patch Cocoa patchSplinter Review
Cocoa patch.  These patches together fix bug 337550.
Attachment #222137 - Flags: review?(darin)
Comment on attachment 222133 [details] [diff] [review]
v1 patch

+  PRBool                      mRunningEventLoop;

PRPackedBool?
> +  PRBool                      mRunningEventLoop;
> 
> PRPackedBool?

I generally only use PRPackedBool when there are multiple booleans.  Either way, the structure size is going to be the same, and the CPU fetches in word sized chunks, so...
Comment on attachment 222137 [details] [diff] [review]
Cocoa patch

[NSApp updateWindows] <- how expensive is that?  we assume okay since it would be generated by [NSApp run] anyways?
Attachment #222137 - Flags: review?(darin) → review+
(In reply to comment #13)
> (From update of attachment 222137 [details] [diff] [review] [edit])
> [NSApp updateWindows] <- how expensive is that?  we assume okay since it would
> be generated by [NSApp run] anyways?

Exactly - correctness first.  The default implementation is not terribly expensive.
fixed-on-trunk
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
With this change, I'm observing a 12.5% increase in Txul on my Linux box.  Tp doesn't seem to change.  Ts is regressed by about 5.5%.  I think I need to find a better solution.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The patches have been backed out of the trunk.
Status: REOPENED → ASSIGNED
As expected, reverting the extra OnDispatchedEvent(nsnull) in NativeEventCallback() recovers the lost performance.
I went ahead and re-landed the patch with the offending bit in NativeEventCallback() commented out.  I'll file a follow-up bug on solving the problem that was trying to solve.  In the meantime, the rest of the patch solves this bug and hopefully resolves the other related Mac bugs.

Marking FIXED again.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
with the latest re-release of your patch, Camino (own build, optimized gcc 4, 10.4sdk) quits when using command-tab to switch between applications. Console log reports this:

2006-05-17 11:24:55.759 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ed81d0 of class NSCFArray autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.760 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x640b7b0 of class NSCFString autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.760 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ea5550 of class NSCFString autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.760 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x64d9a30 of class NSCFSet autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1e1cb00 of class NSCFNumber autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6443130 of class NSCFSet autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x64e0090 of class NSCFSet autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.761 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x641b3a0 of class BrowserTabViewItem autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.768 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67ca1f0 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.768 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67c9070 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.769 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67c93c0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.769 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6704c60 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.770 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x645bbf0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.771 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1e97bf0 of class BrowserTabViewItem autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.778 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1eb0fc0 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.779 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x67f68f0 of class NativeScrollbarView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.785 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6485d60 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.786 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ea46a0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.795 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x6703bd0 of class ChildView autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.795 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1e7ac40 of class NSIdEnumerator autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.796 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x69d84c0 of class TabButtonCell autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.796 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x1ef8620 of class BrowserWindowController autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.796 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x645c0e0 of class TopLevelWindowData autoreleased with no pool in place - just leaking
2006-05-17 11:24:55.797 Camino[15526] *** _NSAutoreleaseNoPool(): Object 0x643c070 of class NSCFString autoreleased with no pool in place - just leaking
Cocoa's getting mad that we're dispatching native events after [NSApp terminate:].  I've got a patch, but new problems belong on new bugs.
(In reply to comment #21)
> Cocoa's getting mad that we're dispatching native events after [NSApp
> terminate:].  I've got a patch, but new problems belong on new bugs.
> 

Sure:  bug 338249

Blocks: 337550
comment #19:
> I went ahead and re-landed the patch with the offending bit in
> NativeEventCallback() commented out.  I'll file a follow-up bug on solving the
> problem that was trying to solve.

For the reference, the bug filed is bug 338225.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.