A observer watching "quit-application-requested" can be used to make tryToClose calls automatically instead of having all quit code manually iterate through windows and call tryToClose.

Still needs more testing but it should work out..

Even better would be for tryToClose to go away completely (see bug 338040 for an outdated patch). Currently you can lose data when you've already closed a few or all windows through tryToClose before either one window objects or before an extension which registered later than your new nsTryToClose component objects - i.e. if you actually close windows before quit-application-granted is broadcast. Anyway, you'd have to make sure that you don't start tryToClose'ing windows before SessionStore gets the quit-application-requested notification (and note: it registers very late!) or otherwise it'll miss some or even all windows for later restoration.

Re-read your patch and it makes more sense now: you just probe tryToClose without actually closing the windows. You can safely ignore the previous comment except for the pointer to bug 338040.

Comment on attachment 269964 [details] [diff] [review] Move tryToClose calls, kill nsICloseAllWindows >+++ b/toolkit/components/startup/src/nsAppStartup.cpp >@@ -449,27 +451,35 @@ nsAppStartup::Observe(nsISupports *aSubj > const char *aTopic, const PRUnichar *aData) > { > NS_ASSERTION(mAppShell, "appshell service notified before appshell built"); > if (!strcmp(aTopic, "profile-change-teardown")) { > nsresult rv; > EnterLastWindowClosingSurvivalArea(); > // NOTE: No early error exits because we need to execute the > // balancing ExitLastWindowClosingSurvivalArea(). >- nsCOMPtr<nsICloseAllWindows> closer = >- do_CreateInstance("@mozilla.org/appshell/closeallwindows;1", &rv); >- NS_ASSERTION(closer, "Failed to create nsICloseAllWindows impl."); >- PRBool proceedWithSwitch = PR_FALSE; >- if (closer) >- rv = closer->CloseAll(PR_TRUE, &proceedWithSwitch); >- >- if (NS_FAILED(rv) || !proceedWithSwitch) { >+ nsCOMPtr<nsIObserverService> obsServ = >+ do_GetService("@mozilla.org/observer-service;1"); >+ NS_ASSERTION(obsServ, "Failed to get observer service"); >+ >+ nsCOMPtr<nsISupportsPRBool> cancelSwitch = >+ do_CreateInstance(NS_SUPPORTS_PRBOOL_CONTRACTID); >+ PRBool abortSwitch = PR_FALSE; >+ if (obsServ && cancelSwitch) { >+ rv = obsServ->NotifyObservers(cancelSwitch, "quit-application-requested", nsnull); >+ cancelSwitch->GetData(&abortSwitch); >+ } >+ >+ if (NS_FAILED(rv) || abortSwitch) { > nsCOMPtr<nsIProfileChangeStatus> changeStatus(do_QueryInterface(aSubject)); > if (changeStatus) > changeStatus->VetoChange(); >+ } else if (obsServ) { >+ obsServ->NotifyObservers(nsnull, "quit-application-granted", nsnull); >+ CloseAllWindows(); > } In the toolkit, profile-shutdown is not a cancelable event. Therefore profile-change-teardown ought to simply fire quit-application-granted without quit-application-requested. Don't be confused by the legacy nsIProfileChangeStatus API: if you call VetoChange you will assert, see http://mxr.mozilla.org/mozilla/source/toolkit/xre/nsXREDirProvider.cpp#725 With that change, r=me

I dropped all the quit-application-requested and quit-application-granted notifications. Otherwise, we get two quit-application-granted notifications on shutdown. This update also removes the tryToClose code from the test suite's quit code, which was copied from toolkit's quit code.

Remove all remaining references to nsCloseAllWindows.js in browser/installer/*/packages-static.

Checking in browser/installer/unix/packages-static; /cvsroot/mozilla/browser/installer/unix/packages-static,v <-- packages-static new revision: 1.116; previous revision: 1.115 done Checking in browser/installer/windows/packages-static; /cvsroot/mozilla/browser/installer/windows/packages-static,v <-- packages-static new revision: 1.128; previous revision: 1.127 done Checking in layout/tools/reftest/quit.js; /cvsroot/mozilla/layout/tools/reftest/quit.js,v <-- quit.js new revision: 1.2; previous revision: 1.1 done Checking in testing/mochitest/tests/SimpleTest/quit.js; /cvsroot/mozilla/testing/mochitest/tests/SimpleTest/quit.js,v <-- quit.js new revision: 1.8; previous revision: 1.7 done Checking in toolkit/components/startup/public/Makefile.in; /cvsroot/mozilla/toolkit/components/startup/public/Makefile.in,v <-- Makefile.in new revision: 1.3; previous revision: 1.2 done Checking in toolkit/components/startup/src/Makefile.in; /cvsroot/mozilla/toolkit/components/startup/src/Makefile.in,v <-- Makefile.in new revision: 1.8; previous revision: 1.7 done Checking in toolkit/components/startup/src/nsAppStartup.cpp; /cvsroot/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,v <-- nsAppStartup.cpp new revision: 1.16; previous revision: 1.15 done Checking in toolkit/components/startup/src/nsAppStartup.h; /cvsroot/mozilla/toolkit/components/startup/src/nsAppStartup.h,v <-- nsAppStartup.h new revision: 1.6; previous revision: 1.5 done Checking in toolkit/content/globalOverlay.js; /cvsroot/mozilla/toolkit/content/globalOverlay.js,v <-- globalOverlay.js new revision: 1.33; previous revision: 1.32 done Checking in toolkit/mozapps/extensions/content/extensions.js; /cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v <-- extensions.js new revision: 1.126; previous revision: 1.125 done Checking in toolkit/mozapps/extensions/content/list.js; /cvsroot/mozilla/toolkit/mozapps/extensions/content/list.js,v <-- list.js new revision: 1.11; previous revision: 1.10 done Checking in toolkit/mozapps/update/content/updates.js; /cvsroot/mozilla/toolkit/mozapps/update/content/updates.js,v <-- updates.js new revision: 1.73; previous revision: 1.72 done Checking in toolkit/xre/nsCommandLineServiceMac.cpp; /cvsroot/mozilla/toolkit/xre/nsCommandLineServiceMac.cpp,v <-- nsCommandLineServiceMac.cpp new revision: 1.9; previous revision: 1.8 done RCS file: /cvsroot/mozilla/toolkit/components/startup/src/nsTryToClose.js,v done Checking in toolkit/components/startup/src/nsTryToClose.js; /cvsroot/mozilla/toolkit/components/startup/src/nsTryToClose.js,v <-- nsTryToClose.js initial revision: 1.1 done Removing toolkit/components/startup/public/nsICloseAllWindows.idl; /cvsroot/mozilla/toolkit/components/startup/public/nsICloseAllWindows.idl,v <-- nsICloseAllWindows.idl new revision: delete; previous revision: 1.1 done Removing toolkit/components/startup/src/nsCloseAllWindows.js; /cvsroot/mozilla/toolkit/components/startup/src/nsCloseAllWindows.js,v <-- nsCloseAllWindows.js new revision: delete; previous revision: 1.1 done

Shouldn't the file nsCloseAllWindows.js be added to browser/installer/removed-files.in?

(In reply to comment #8) > Shouldn't the file nsCloseAllWindows.js be added to > browser/installer/removed-files.in? > Sure. This patch adds it for all the removed-files.in I can find.

Looks like the following was missed http://lxr.mozilla.org/seamonkey/source/mail/installer/windows/packages-static#327

(In reply to comment #10) > Looks like the following was missed > http://lxr.mozilla.org/seamonkey/source/mail/installer/windows/packages-static#327 > Thanks for catching this. This patch fixes mail and calendar.

Checking in calendar/installer/windows/packages-static; /cvsroot/mozilla/calendar/installer/windows/packages-static,v <-- packages-static new revision: 1.39; previous revision: 1.38 done Checking in mail/installer/windows/packages-static; /cvsroot/mozilla/mail/installer/windows/packages-static,v <-- packages-static new revision: 1.65; previous revision: 1.64 done Checking in browser/installer/removed-files.in; /cvsroot/mozilla/browser/installer/removed-files.in,v <-- removed-files.in new revision: 1.28; previous revision: 1.27 done Checking in calendar/installer/removed-files.in; /cvsroot/mozilla/calendar/installer/removed-files.in,v <-- removed-files.in new revision: 1.12; previous revision: 1.11 done Checking in mail/installer/removed-files.in; /cvsroot/mozilla/mail/installer/removed-files.in,v <-- removed-files.in new revision: 1.35; previous revision: 1.34 done

This may have caused a regression; see bug 271112 comment 6, 10.

This also likely caused bug 449308.