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)
SeaMonkey
Startup & Profiles
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
?
Assignee | ||
Updated•13 years ago
|
Target Milestone: --- → seamonkey2.10
Comment 1•13 years ago
|
||
Dunno, but it sounds like a good idea anyway.
Assignee | ||
Comment 2•13 years ago
|
||
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)
Assignee | ||
Comment 3•13 years ago
|
||
(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?
Assignee | ||
Comment 4•13 years ago
|
||
(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
Assignee | ||
Comment 5•13 years ago
|
||
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)
Comment 6•13 years ago
|
||
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?
Assignee | ||
Comment 7•13 years ago
|
||
(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.
Comment 8•13 years ago
|
||
> + .getService(Ci.nsIAppStartup);
I don't think we have the Ci shortcut. in nsSuiteGlue.js
Assignee | ||
Comment 9•13 years ago
|
||
Av1a, with comment 8 suggestion(s).
Attachment #604998 -
Attachment is obsolete: true
Attachment #605508 -
Flags: review?(iann_bugzilla)
Attachment #604998 -
Flags: review?(iann_bugzilla)
Assignee | ||
Comment 10•13 years ago
|
||
(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...
Assignee | ||
Comment 11•13 years ago
|
||
(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().
Comment 12•13 years ago
|
||
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.
Assignee | ||
Comment 13•13 years ago
|
||
(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?
Comment 14•13 years ago
|
||
(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.
Assignee | ||
Comment 15•13 years ago
|
||
(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
status-seamonkey2.10:
--- → affected
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
Comment 16•13 years ago
|
||
(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.
Assignee | ||
Comment 17•13 years ago
|
||
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 18•13 years ago
|
||
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)
Assignee | ||
Comment 19•13 years ago
|
||
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 20•13 years ago
|
||
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-
Assignee | ||
Comment 21•13 years ago
|
||
(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)?
Assignee | ||
Updated•13 years ago
|
Comment 22•13 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•