Open Bug 734883 Opened 13 years ago Updated 9 years ago

Port |Bug 294260 - Safe Mode: Auto detect previous start-up failure and offer to start in safe mode| to SeaMonkey

Categories

(SeaMonkey :: Startup & Profiles, enhancement)

enhancement
Not set
normal

Tracking

(seamonkey2.10 affected)

ASSIGNED
Tracking Status
seamonkey2.10 --- affected

People

(Reporter: sgautherie, Assigned: sgautherie)

References

Details

(Keywords: helpwanted)

Attachments

(1 file, 3 obsolete files)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331529579.1331534365.8340.gz&fulltext=1 Linux comm-central-trunk debug test mochitests-3/5 on 2012/03/11 22:19:39 { WARNING: Failed to setup pref service.: file /builds/slave/comm-cen-trunk-lnx-dbg/build/mozilla/toolkit/xre/nsXREDirProvider.cpp, line 742 } All platforms, all mochitests-*. *** Do SeaMonkey needs to port (some of) the browser/* part from https://hg.mozilla.org/mozilla-central/rev/bcdc5493e6a1 ?
Target Milestone: --- → seamonkey2.10
Dunno, but it sounds like a good idea anyway.
Depends on: 734886
This does not port: *UI part: the 3 safeMode.* files. *This should be fine to do as a follow-up patch. *browser.js delayedStartup() part. *See below. And I can't test it... *** http://mxr.mozilla.org/comm-central/search?string=delayedStartup&case=on&find=%2Fsuite%2F nsSessionStore.js refers to delayedStartup() which SeaMonkey does not have (yet). Should that be ported first/too? Does this bug depends on it?
Attachment #604985 - Flags: review?(iann_bugzilla)
Blocks: 36810
(In reply to Serge Gautherie (:sgautherie) from comment #2) > http://mxr.mozilla.org/comm-central/ > search?string=delayedStartup&case=on&find=%2Fsuite%2F > nsSessionStore.js refers to delayedStartup() which SeaMonkey does not have > (yet). > Should that be ported first/too? > Does this bug depends on it? It looks like we need to at least add that trackStartupCrashEnd() call somewhere, for example to enable the pref and pass http://mxr.mozilla.org/mozilla-central/source/toolkit/components/startup/tests/browser/browser_crash_detection.js Where would that be?
(In reply to Serge Gautherie (:sgautherie) from comment #0) > WARNING: Failed to setup pref service.: file > /builds/slave/comm-cen-trunk-lnx-dbg/build/mozilla/toolkit/xre/ > nsXREDirProvider.cpp, line 742 It looks like TrackStartupCrashEnd() is already called somehow, though I don't know how that happens (on SeaMonkey) :-/ http://mxr.mozilla.org/comm-central/search?string=TrackStartupCrashEnd
Av1, with added default preference value (to enable the feature).
Attachment #604985 - Attachment is obsolete: true
Attachment #604998 - Flags: review?(iann_bugzilla)
Attachment #604985 - Flags: review?(iann_bugzilla)
Could the root cause of the warning be investigated? The warning and porting the feature should be two separate bugs. Does this warning happen on every startup?
(In reply to Matthew N. [:MattN] from comment #6) > Could the root cause of the warning be investigated? I tried: could you answer comment 0, comment 2 and comment 4? > The warning and porting the feature should be two separate bugs. Why (= genuine question)? They were added in the same bug... > Does this warning happen on every startup? Yes, see comment 0. http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331584266.1331593066.374.gz&fulltext=1 OS X 10.6 comm-central-trunk leak test build on 2012/03/12 13:31:06 is affected too.
> + .getService(Ci.nsIAppStartup); I don't think we have the Ci shortcut. in nsSuiteGlue.js
Av1a, with comment 8 suggestion(s).
Attachment #604998 - Attachment is obsolete: true
Attachment #605508 - Flags: review?(iann_bugzilla)
Attachment #604998 - Flags: review?(iann_bugzilla)
(In reply to Serge Gautherie (:sgautherie) from comment #4) > (In reply to Serge Gautherie (:sgautherie) from comment #0) > > WARNING: Failed to setup pref service.: file > > /builds/slave/comm-cen-trunk-lnx-dbg/build/mozilla/toolkit/xre/ > > nsXREDirProvider.cpp, line 742 > > It looks like TrackStartupCrashEnd() is already called somehow, though I > don't know how that happens (on SeaMonkey) :-/ > http://mxr.mozilla.org/comm-central/search?string=TrackStartupCrashEnd Bah, I got confused: this is unrelated to TrackStartupCrashEnd(), this is mozilla::Preferences::ResetAndReadUserPrefs() which fails. http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/xre/nsXREDirProvider.cpp#736 742 nsresult rv = mozilla::Preferences::ResetAndReadUserPrefs(); 743 if (NS_FAILED(rv)) NS_WARNING("Failed to setup pref service."); calls http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/src/Preferences.cpp#359 364 sPreferences->ResetUserPrefs(); 365 return sPreferences->ReadUserPrefs(nsnull); calls http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/src/Preferences.cpp#395 399 if (XRE_GetProcessType() == GeckoProcessType_Content) { 400 NS_ERROR("cannot load prefs from content process"); 401 return NS_ERROR_NOT_AVAILABLE; 402 } 403 404 nsresult rv; 405 406 if (nsnull == aFile) { 407 408 NotifyServiceObservers(NS_PREFSERVICE_READ_TOPIC_ID); 409 410 rv = UseDefaultPrefFile(); 411 UseUserPrefFile(); 412 } else { 413 rv = ReadAndOwnUserPrefFile(aFile); 414 } 415 return rv; But the nsresult value is not reported: it would need a debugger to find out what is going on...
(In reply to Serge Gautherie (:sgautherie) from comment #10) > http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/src/ > Preferences.cpp#395 > 399 if (XRE_GetProcessType() == GeckoProcessType_Content) { > 400 NS_ERROR("cannot load prefs from content process"); This error is not reported: fine. > 401 return NS_ERROR_NOT_AVAILABLE; > 402 } > 403 > 404 nsresult rv; > 405 > 406 if (nsnull == aFile) { > 407 > 408 NotifyServiceObservers(NS_PREFSERVICE_READ_TOPIC_ID); > 409 > 410 rv = UseDefaultPrefFile(); UseDefaultPrefFile() is http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp#586 593 rv = NS_GetSpecialDirectory(NS_APP_PREFS_50_FILE, getter_AddRefs(aFile)); 594 if (NS_SUCCEEDED(rv)) { 595 rv = ReadAndOwnUserPrefFile(aFile); 596 // Most likely cause of failure here is that the file didn't 597 // exist, so save a new one. mUserPrefReadFailed will be 598 // used to catch an error in actually reading the file. 599 if (NS_FAILED(rv)) { 600 rv2 = SavePrefFileInternal(aFile); 601 NS_ASSERTION(NS_SUCCEEDED(rv2), "Failed to save new shared pref file"); 602 } 603 } 604 605 return rv; The assertion is not reported either, so it fails at NS_GetSpecialDirectory() or ReadAndOwnUserPrefFile().
I used to get this but after I updated my build recently I have stopped seeing this. It might be related to the bug about the chrome folder not appearing in new profiles - I think there is a default pref file too, and now that new profiles get populated the pref service doesn't warn on startup.
(In reply to neil@parkwaycc.co.uk from comment #12) > I used to get this but after I updated my build recently I have stopped http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331678053.1331688389.14754.gz&fulltext=1 OS X 10.5 comm-central-trunk leak test build on 2012/03/13 15:34:13 still has it. > seeing this. It might be related to the bug about the chrome folder not > appearing in new profiles - I think there is a default pref file too, and > now that new profiles get populated the pref service doesn't warn on startup. Are referring to bug 728840?
(In reply to Serge Gautherie (:sgautherie) from comment #7) > (In reply to Matthew N. [:MattN] from comment #6) > > > Could the root cause of the warning be investigated? > > I tried: could you answer comment 0, comment 2 and comment 4? To implement startup crash tracking, you just need to call TrackStartupCrashEnd when you want to define the end of startup and set the pref IIRC. For Firefox, it is 30s after delayedStartup() which means that we also need to make sure that TrackStartupCrashEnd is called on shutdown (in case they shutdown before the 30s). Attachment 605508 [details] [diff] still needs work because it doesn't call TrackStartupCrashEnd at the end of startup. > > The warning and porting the feature should be two separate bugs. > > Why (= genuine question)? They were added in the same bug... Because porting the feature will not remove the warning. The warning is shown because there are no prefs to read in a new profile. It's because UseDefaultPrefFile doesn't return the successful nsresult rv2 in this case at https://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp?rev=12813323739a#588 > > Does this warning happen on every startup? > > Yes, see comment 0. I believe the test machines always use a new profile for testing and so that's why you see the warning on every one. I was talking about every startup for one profile.
Depends on: 735573
(In reply to Matthew N. [:MattN] from comment #14) > Attachment 605508 [details] [diff] still needs work > because it doesn't call TrackStartupCrashEnd at the end of startup. Thanks for the explanation/confirmation: *nsSuiteGlue.js part should be "commented out" until SeaMonkey uses a "delayedStartup()". *For now, I need (to find) an answer to comment 3 on the SeaMonkey side. > https://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/ > Preferences.cpp?rev=12813323739a#588 Right, that's what I was hinting at in comment 11. Actually, I was sure I had verified (2-3 times) that Firefox was not affected, but it is (now): https://tbpl.mozilla.org/php/getParsedLog.php?id=10044669&tree=Firefox&full=1 Rev3 Fedora 12 mozilla-central debug test mochitests-3/5 on 2012-03-13 15:55:34 PDT for push c71845b3b2a6 I filed bug 735573. Ftr, I did http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=41a6b83a864b but it doesn't reproduce useful steps for this issue :-| (But it could be done again: for Firefox...) > I believe the test machines always use a new profile for testing and so > that's why you see the warning on every one. I was talking about every Yes. > startup for one profile. Oh. (I can't check that.)
Assignee: nobody → sgautherie.bz
Severity: normal → enhancement
Status: NEW → ASSIGNED
Keywords: helpwanted
Summary: [SeaMonkey] "WARNING: Failed to setup pref service. [...] /toolkit/xre/nsXREDirProvider.cpp, line 742" → Port |Bug 294260 - Safe Mode: Auto detect previous start-up failure and offer to start in safe mode| to SeaMonkey
Target Milestone: seamonkey2.10 → seamonkey2.11
(In reply to Serge Gautherie from comment #13) > (In reply to neil@parkwaycc.co.uk from comment #12) > > I used to get this but after I updated my build recently I have stopped > > seeing this. It might be related to the bug about the chrome folder not > > appearing in new profiles - I think there is a default pref file too, and > > now that new profiles get populated the pref service doesn't warn on startup. > Are referring to bug 728840? Indeed, the lack of the chrome folder is just one symptom of the default files not getting copied to the profile folder, and I thought that there was a default pref file that we were therefore lacking thus triggering that warning.
Av1b, with (delayed) startup part. (Untested.)
Attachment #605508 - Attachment is obsolete: true
Attachment #606094 - Flags: review?(iann_bugzilla)
Attachment #605508 - Flags: review?(iann_bugzilla)
Comment on attachment 606094 [details] [diff] [review] (Av1c) Port |Bug 294260 - Safe Mode: Auto detect previous start-up failure and offer to start in safe mode| non-UI part, Add a minimal delayedStartup() function As Neil is more familiar with the safe mode code, another option would be :InvisibleSmiley both were involved with bug 573538
Attachment #606094 - Flags: review?(iann_bugzilla) → review?(neil)
Ftr, { [21:17] <IanN> sgautherie: is there an easy way to test your new code? [21:38] <sgautherie> IanN: utilityOverlay.js part is the Restart item in the Help menu I think. I guess "startup crash tracking" should trigger if you manually kill SM process within "30 seconds"... }
Comment on attachment 606094 [details] [diff] [review] (Av1c) Port |Bug 294260 - Safe Mode: Auto detect previous start-up failure and offer to start in safe mode| non-UI part, Add a minimal delayedStartup() function >+// Startup Crash Tracking: >+// Number of startup crashes that can occur before starting into safe mode automatically. >+// (This pref has no effect if more than 6 hours have passed since the last crash.) >+pref("toolkit.startup.max_resumed_crashes", 2); Maybe put this near browser.sessionstore.max_resumed_crashes? >+ // End startup crash tracking after a (30 seconds) delay to catch crashes >+ // while restoring tabs and to postpone saving the pref to disk. Unfortunately we need this to work for all windows, not just browsers. >+ Components.classes["@mozilla.org/toolkit/app-startup;1"] >+ .getService(Components.interfaces.nsIAppStartup) >+ .restartInSafeMode(Components.interfaces.nsIAppStartup.eAttemptQuit); Does this notify observers in the way Application.restart() does?
Attachment #606094 - Flags: review?(neil) → review-
(In reply to neil@parkwaycc.co.uk from comment #20) > >+ // End startup crash tracking after a (30 seconds) delay to catch crashes > >+ // while restoring tabs and to postpone saving the pref to disk. > Unfortunately we need this to work for all windows, not just browsers. Is there an existing common place to add this feature to? Or which are the other windows (file)?
No longer depends on: 294260, 734886, 735573
Depends on: 294260, 734886, 735573
(In reply to Serge Gautherie from comment #21) > (In reply to neil@parkwaycc.co.uk from comment #20) > > >+ // End startup crash tracking after a (30 seconds) delay to catch crashes > > >+ // while restoring tabs and to postpone saving the pref to disk. > > Unfortunately we need this to work for all windows, not just browsers. > Is there an existing common place to add this feature to? nsSuiteGlue, although of course you can't use setTimeout there.
Target Milestone: seamonkey2.11 → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: