735333 - Use Services.prefs instead of preferences-service / gPrefService, in SeaMonkey code

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

+++ This bug was initially created as a clone of Bug #707786 +++

http://mxr.mozilla.org/comm-central/search?string=gPrefService&case=1&find=%2Fsuite%2F
"Found 10 matching lines in 3 files"

http://mxr.mozilla.org/comm-central/search?string=preferences-service&case=1&find=%2Fsuite%2F
"Found 75 matching lines in 42 files"

Comment 1

[Alan Yeo [:alanyeojw]]

Attachment: replace preferences-service / gPrefService with Services.prefs in SeaMonkey code

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 606181 [details] [diff] [review]
replace preferences-service / gPrefService with Services.prefs in SeaMonkey code

Review of attachment 606181 [details] [diff] [review]:
-----------------------------------------------------------------

I suggest to do gPrefService in a first patch, then preferences-service in a second.

::: suite/browser/nsBrowserContentHandler.js
@@ +151,5 @@
>  }
>  
>  function getURLToLoad()
>  {
> +  var prefs = Services.prefs.getService(nsIPrefBranch);

This is wrong:
1) Services.prefs already includes getService(nsIPrefBranch).
2) prefs var can be removed (= inlined) too.

Comment 3

[Philip Chee]

> +++ b/suite/browser/nsBrowserContentHandler.js
> +++ b/suite/browser/nsTypeAheadFind.js
These are components running in their own global context so you'll need to import Services.jsm somewhere near the top of the file:
Components.utils.import("resource://gre/modules/Services.jsm");

I think for components unless the preference service is used repeatedly, importing Services.jsm for one or two users is not necessary.

> +++ b/suite/browser/tabbrowser.xml
> +++ b/suite/browser/urlbarBindings.xml

> -        Components.classes["@mozilla.org/preferences-service;1"]
> -                  .getService(Components.interfaces.nsIPrefBranch2);
> +        Services.prefs.getService(Components.interfaces.nsIPrefBranch2);

Ah Services.prefs already does a QueryInterface(Ci.nsIPrefBranch)
so just |Services.prefs| will do here. I don't think this is necessary since Services.pref is a lazy getter but you're forcing it to instantiate here thus defeating the whole purpose of using Services.

I gave up half way. But please make sure that Services is in the current scope before trying to use it. It also helps if you build SeaMonkey with your patch to see how many things break ;)

So please attach an updated patch once you've done this.

Comment 4

[[e-mail]]

Comment on attachment 606181 [details] [diff] [review]
replace preferences-service / gPrefService with Services.prefs in SeaMonkey code

Don't feel you have to do everything in one giant patch. You might want to start with simple cases where prefs are only used in two or three places in the file, and save the harder files for separate patches.

>diff --git a/suite/browser/nsBrowserContentHandler.js b/suite/browser/nsBrowserContentHandler.js
>index 64c04b1..48a8fe9 100644
>--- a/suite/browser/nsBrowserContentHandler.js
>+++ b/suite/browser/nsBrowserContentHandler.js
>@@ -147,18 +147,17 @@ function needHomePageOverride(prefs)
> 
>   prefs.setCharPref("browser.startup.homepage_override.mstone", mstone);
> 
>   return true;
> }
> 
> function getURLToLoad()
> {
>-  var prefs = Components.classes["@mozilla.org/preferences-service;1"]
>-                        .getService(nsIPrefBranch);
>+  var prefs = Services.prefs.getService(nsIPrefBranch);
>   var formatter = Components.classes["@mozilla.org/toolkit/URLFormatterService;1"]
>                             .getService(Components.interfaces.nsIURLFormatter);
> 
>   if (needHomePageOverride(prefs)) {
nsBrowserContentHandler originally predated Services which is why it gets prefs once here and then hands it out when it's needed. If you want to switch this to use Services.prefs then you would get rid of all the prefs arguments. (This is in addition to removing all the other prefs variables that people have probably already mentioned.)

>       <field name="mPrefs" readonly="true">
>-        Components.classes["@mozilla.org/preferences-service;1"]
>-                  .getService(Components.interfaces.nsIPrefBranch2);
>+        Services.prefs.getService(Components.interfaces.nsIPrefBranch2);
>       </field>
Please don't change tabbrowser.xml, I don't want it to use Services. (I know it does but I didn't notice KaiRo/Callek adding them. Oops.) In fact, that goes for all the .xml files for now, I need to decide on a case-by-case basis.

>   var pref = null;
>-  pref = Components.classes["@mozilla.org/preferences-service;1"]
>-                   .getService(Components.interfaces.nsIPrefBranch);
>+  pref = Services.prefs.getService(Components.interfaces.nsIPrefBranch);
Wow, I wonder whether we still have people using pref. (I know it's hard to tell with such a generic variable name; even something like sPrefs would be easier to find.) Ideally they would use Services.prefs instead and then we could just get rid of pref.

Comment 5

[Alan Yeo [:alanyeojw]]

Attachment: replace gPrefService with Services.prefs

Comment 6

[Alan Yeo [:alanyeojw]]

Attachment: replace preferences-service with Services.prefs in suite/feeds/src/

Comment 7

[Alan Yeo [:alanyeojw]]

Attachment: replace preferences-service with Services.prefs in suite/modules/Sanitizer.jsm

get _prefs() {
     delete this._prefs;
-    return this._prefs = Components.classes["@mozilla.org/preferences-service;1"]
-                                   .getService(Components.interfaces.nsIPrefService)
-                                   .getBranch("privacy.sanitize.");
+    return this._prefs = Services.prefs.getBranch("privacy.sanitize.");
   },

Is "delete this._prefs;" still necessary since Services.prefs is lazily loaded?

Comment 8

[[e-mail]]

(In reply to Alan Yeo from comment #7)
>    get _prefs() {
>      delete this._prefs;
> -    return this._prefs =
> Components.classes["@mozilla.org/preferences-service;1"]
> -                                  
> .getService(Components.interfaces.nsIPrefService)
> -                                   .getBranch("privacy.sanitize.");
> +    return this._prefs = Services.prefs.getBranch("privacy.sanitize.");
>    },
> 
> Is "delete this._prefs;" still necessary since Services.prefs is lazily
> loaded?
This is a lazy caching branch getter, so it needs to do that to work properly.

Comment 9

[Alan Yeo [:alanyeojw]]

Attachment: suite/mailnews/

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 611870 [details] [diff] [review]
replace preferences-service with Services.prefs in suite/modules/Sanitizer.jsm

Review of attachment 611870 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/modules/Sanitizer.jsm
@@ +45,1 @@
>  var EXPORTED_SYMBOLS = ["Sanitizer"];

Maybe better to keep EXPORTED_SYMBOLS on top.

@@ +45,4 @@
>  var EXPORTED_SYMBOLS = ["Sanitizer"];
>  
>  var Sanitizer = {
>    get _prefs() {

It looks like Services.prefs could be used directly at the 3 call lines...

@@ +67,5 @@
>        this.items[itemName].willClear = false;
>    },
>  
>    readSettings: function(aParentWindow) {
> +    var itemPrefs = Services.prefs.getBranch("privacy.item.");

(I have a doubt.)
Did you confirm that this works?
As getBranch() is part of nsIPrefService, not nsIPrefBranch.

@@ +196,1 @@
>            branch.deleteBranch("");

While here, merge 'branch' var too.

Wouldn't
Services.prefs.deleteBranch("geo.wifi.access_token.")
even do the same job?

@@ +227,2 @@
>          try {
> +          Services.prefs.clearUserPref("general.open_location.last_url");

While here,
"general.open_location.last_url" should be a property of 'urlbar' (if not a const in the file).

Comment 11

[Serge Gautherie (:sgautherie)]

(In reply to Serge Gautherie (:sgautherie) from comment #10)
> (I have a doubt.)
> Did you confirm that this works?
> As getBranch() is part of nsIPrefService, not nsIPrefBranch.

This is wildly used, so no more doubt ;->
http://mxr.mozilla.org/comm-central/search?string=Services.prefs.getBranch&case=on

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 611849 [details] [diff] [review]
replace gPrefService with Services.prefs

Let's deal with one patch at a time, ftb...

Comment 13

[Philip Chee]

Hello Alan, are you still working on this?

Comment 14

[Alan Yeo [:alanyeojw]]

Nope, I haven't been able to find time to continue working on this. I'll go ahead and unassign myself.

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: switchung preference calls to Services.jsm in all of /suite/

This patch addresses the former review comments except it also switches to Services.jsm in .xml files. Is there any reason why you want the old code there?

Comment 16

[Philip Chee]

Comment on attachment 664179 [details] [diff] [review]
switchung preference calls to Services.jsm in all of /suite/

> except it also switches to Services.jsm
> in .xml files. Is there any reason why you want the old code there?
The SeaMonkey module owner doesn't want any dependency on Services in our XBL. Besides that's not the way to import a JSM into a XBL binding.

Also separate patches for each SeaMonkey module in order to make reviewing easier. Please. Thank you very much.

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite backend

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite bookmarks and history

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite browser

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite common

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite compose (mailnews)

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite downloads

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite feeds

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite mailnews preferences

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite message display (mailnews)

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite preferences

Comment 27

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite profiles

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite sanitizer.jsm

Comment 29

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite session restore

Comment 30

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite sidebar

Comment 31

[Misak Khachatryan]

Comment on attachment 665025 [details] [diff] [review]
Switch to Services.jsm: suite session restore

Review of attachment 665025 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/common/aboutSessionRestore.js
@@ +1,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +Components.utils.import("resource://gre/modules/Services.jsm");

This is already done at line 91, where all import performed.

Comment 32

[[e-mail]]

Comment on attachment 665022 [details] [diff] [review]
Switch to Services.jsm: suite preferences

>+Components.utils.import("resource://gre/modules/Services.jsm");
As it happens, Services is already available from this file...

>     Services.obs.notifyObservers(window, "app-handler-pane-loaded", null);
... so you don't need to load it yourself. r=me with that fixed.

Comment 33

[[e-mail]]

Comment on attachment 665024 [details] [diff] [review]
Switch to Services.jsm: suite sanitizer.jsm

r=me assuming the tests still pass.

Comment 34

[[e-mail]]

Comment on attachment 665015 [details] [diff] [review]
Switch to Services.jsm: suite common

>-  var pref = null;
>-  pref = Components.classes["@mozilla.org/preferences-service;1"]
>-                   .getService(Components.interfaces.nsIPrefBranch);
Careful here because some other file may be accidentally relying on this copy of pref, so you should probably check this patch in last.

>+  Components.utils.import("resource://gre/modules/Services.jsm");
[I think any file including contentAreaClick or nsContextMenu will include utilityOverlay.js which will give it Services anyway.]

Comment 35

[[e-mail]]

Comment on attachment 665014 [details] [diff] [review]
Switch to Services.jsm: suite browser

nsBrowserContentListener and nsBrowserStatusHandler already have access to Services. nsBrowserContentHandler and nsTypeAheadFind don't. Confused yet?

Comment 36

[Misak Khachatryan]

Comment on attachment 665025 [details] [diff] [review]
Switch to Services.jsm: suite session restore

Review of attachment 665025 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, ignore my previous comment, I was looking to other file.

Comment 37

[[e-mail]]

Comment on attachment 665026 [details] [diff] [review]
Switch to Services.jsm: suite sidebar

>+Components.utils.import("resource://gre/modules/Services.jsm");
>+
As usual, don't need this.

Comment 38

[[e-mail]]

Comment on attachment 665018 [details] [diff] [review]
Switch to Services.jsm: suite downloads

You seem to have forgotten prefs.getIntPref(PREF_DM_BEHAVIOR)...

Comment 39

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite downloads, v2

Comment 40

[Ian Neal]

Comment on attachment 665011 [details] [diff] [review]
Switch to Services.jsm: suite backend

>+++ b/suite/mailnews/mail-offline.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This isn't needed as Services.jsm is already imported by other scripts that the callers of mail-offline.js use.

>+    switch(Services.prefs.getIntPref("offline.send.unsent_messages")) { 
Nit: trailing space.

>+    switch(Services.prefs.getIntPref("offline.download.download_messages")) {	
Nit: trailing space.

>+++ b/suite/mailnews/phishingDetector.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This isn't needed as Services.jsm is already imported by other scripts that the callers of phishingDetector.js use.

r=me with those fixed.

Comment 41

[Ian Neal]

Comment on attachment 665012 [details] [diff] [review]
Switch to Services.jsm: suite bookmarks and history

>+++ b/suite/common/bookmarks/bm-props.js
>@@ -53,16 +53,18 @@
>  *     - "siteLocation"
>  *     - "folderPicker" - hides both the tree and the menu.
>  *   @ readOnly (Boolean) - optional, states if the panel should be read-only
>  *
>  * window.arguments[0].performed is set to true if any transaction has
>  * been performed by the dialog.
>  */
> 
>+Components.utils.import("resource://gre/modules/Services.jsm");
This shouldn't be needed as you are importing in editBookmarkOverlay.js so will available to this file.

>-      prefs.setBoolPref("browser.bookmarks.editDialog.expandTags",
>+      Services.prefs.setBoolPref("browser.bookmarks.editDialog.expandTags",
>                         !this._element("tagsSelectorRow").collapsed);
You need to indent this so it lines up the " above.
>     if (!this._element("folderRow").collapsed)
>-      prefs.setBoolPref("browser.bookmarks.editDialog.expandFolders",
>+      Services.prefs.setBoolPref("browser.bookmarks.editDialog.expandFolders",
>                         !this._element("folderTreeRow").collapsed);
You need to indent this so it lines up the " above.

>+++ b/suite/common/history/controller.js

>+Components.utils.import("resource://gre/modules/Services.jsm");
>+
This isn't needed as placesOverlay.xul's caller already imports Services.jsm (other scripts called by placesOverlay.xul already make use of Services).

>+++ b/suite/common/history/utils.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This isn't needed as placesOverlay.xul's caller already imports Services.jsm (other scripts called by placesOverlay.xul already make use of Services).

>+    return this.ellipsis = Services.prefs.getComplexValue("intl.ellipsis",
>                                                 Components.interfaces.nsIPrefLocalizedString).data;
You need to indent this so it lines up the " above, alternatively write something like:
    return this.ellipsis =
      Services.prefs.getComplexValue("intl.ellipsis",
                                     Components.interfaces.nsIPrefLocalizedString).data;

r=me with those fixed.

Comment 42

[Ian Neal]

Comment on attachment 665014 [details] [diff] [review]
Switch to Services.jsm: suite browser

>+++ b/suite/browser/nsBrowserContentListener.js

>+Components.utils.import("resource://gre/modules/Services.jsm");
This import isn't required.

>+++ b/suite/browser/nsBrowserStatusHandler.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import isn't required.

r=me with that fixed.

Comment 43

[Ian Neal]

Comment on attachment 665015 [details] [diff] [review]
Switch to Services.jsm: suite common

>+++ b/suite/common/about.xhtml
>+      Components.utils.import("resource://gre/modules/Services.jsm");
You don't need to import this.

>+++ b/suite/common/contentAreaClick.js
>+  Components.utils.import("resource://gre/modules/Services.jsm");
You don't need to import this.

>+++ b/suite/common/nsContextMenu.js

>+Components.utils.import("resource://gre/modules/Services.jsm");
You don't need to import this.

r=me with those fixed. As Neil mentioned this patch probably needs to be checked in last.

Comment 44

[Ian Neal]

Comment on attachment 665647 [details] [diff] [review]
Switch to Services.jsm: suite downloads, v2

>+++ b/suite/common/downloads/downloadmanager.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not required.

>+++ b/suite/common/downloads/progressDialog.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not required.

r=me with those fixed.

Comment 45

[Ian Neal]

Comment on attachment 665020 [details] [diff] [review]
Switch to Services.jsm: suite mailnews preferences

>+++ b/suite/mailnews/prefs/pref-tags.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

r=me with that fixed.

Comment 46

[Ian Neal]

Comment on attachment 665021 [details] [diff] [review]
Switch to Services.jsm: suite message display (mailnews)

>+++ b/suite/mailnews/mail3PaneWindowCommands.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/mailContextMenus.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/mailTasksOverlay.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/mailWindow.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is probably not needed.

>+++ b/suite/mailnews/messageWindow.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/msgHdrViewOverlay.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/msgMail3PaneWindow.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/msgViewNavigation.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/tabmail.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

>+++ b/suite/mailnews/threadPane.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
This import is not needed.

r=me with that fixed.

Comment 47

[Karsten Düsterloh]

Comment on attachment 665017 [details] [diff] [review]
Switch to Services.jsm: suite compose (mailnews)

>+Components.utils.import("resource://gre/modules/Services.jsm");
>+

You don't need that here, it's already imported elsewhere.

Comment 48

[Karsten Düsterloh]

Oh, and you can assume moa+ for suite/mailnews for this if either Neil, IanN or I have seen it.

Comment 49

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: updated SeaMonkey Services.jsm patches

Hi, these are the updated versions of the patches. It would be nice if someone can build SeaMonkey with these and run the tests. This fails on Thunderbird-Try, see https://bugzilla.mozilla.org/show_bug.cgi?id=798789#c10

Comment 50

[Ian Neal]

Comment on attachment 669495 [details]
updated SeaMonkey Services.jsm patches

Not sure what the attachment is, but doesn't appear to be a patch. Could you try again?

Comment 51

[Philip Chee]

It's a ZIP file containing several patches. Archaeopteryx, aside from unpacking them and attaching them individually, could you make them changeset patches with the normal Hg metadata?

Comment 52

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite backend, v3

Comment 53

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite bookmarks and history, v3

Comment 54

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite browser, v3

Comment 55

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite common, v3

Comment 56

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite compose (mailnews), v3

Comment 57

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite downloads, v4

Comment 58

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite feeds, v2

Comment 59

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite mailnews preferences, v3

Comment 60

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite message display (mailnews), v3

Comment 61

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite preferences, v3

Comment 62

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite profiles, v2

Comment 63

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite sanitizer.jsm, v2

Comment 64

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite session restore, v2

Comment 65

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Switch to Services.jsm: suite sidebar, v3

Comment 66

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Philip Chee from comment #51)
> It's a ZIP file containing several patches. Archaeopteryx, aside from
> unpacking them and attaching them individually, could you make them
> changeset patches with the normal Hg metadata?

Done. To advance, we need a compile-and-test run sponsored by someone with a performing machine (Windows would be optimal for debugging). mcsmurf and I didn't manage to use Thunderbird-Try for this (directory issues with the extensions).

Comment 67

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Philip Chee from comment #51)
> It's a ZIP file containing several patches. Archaeopteryx, aside from
> unpacking them and attaching them individually, could you make them
> changeset patches with the normal Hg metadata?

Done. To advance, we need a compile-and-test run sponsored by someone with a performing machine (Windows would be optimal for debugging). mcsmurf and I didn't manage to use Thunderbird-Try for this (directory issues with the extensions).

Comment 68

[Frank Wein [:mcsmurf]]

I wonder what to actually look for in the mochitest log. Failing tests only or also JavaScript errors in the log? The latter is more difficult to find out as JS errors are not "tagged" (like TEST-UNEXPECTED-FAIL) in the output log. I wonder as I saw this error in mochitest-browser-chrome log:
[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED)
 [nsIPrefBranch.getBoolPref]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  loc
ation: "JS frame :: file:///c:/mozilla/tree-hg/objdirs/seamonkey-objdir-debug-te
sts/mozilla/dist/bin/components/nsSessionStartup.js :: sss_init :: line 64"  dat
a: no]
line 64 is the "Services.prefs.getBoolPref("sessionstore.resume_session_once")" call.

Comment 69

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite bookmarks and history, v4

Replaces Attachment 683229 [details] [diff], old patch did not apply anymore.

Comment 70

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite common, v4

Replaces Attachment 683236 [details] [diff], old patch did not apply anymore.

Comment 71

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite message display (mailnews), v4

Replaces Attachment 683242 [details] [diff], old patch did not apply anymore.

Comment 72

[Frank Wein [:mcsmurf]]

Oh, I see :) the pref name is brower.sessionstore.resume_session_once, not sessionstore.resume_session_once.

Comment 73

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Sorry for this. Please also update that line below by adding "browser.":
+    var resumeFromCrash = Services.prefs.getBoolPref("sessionstore.resume_from_crash");

Comment 74

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite session restore, v3

Replaces Attachment 683249 [details] [diff], fixes the pref issue.

Comment 75

[Frank Wein [:mcsmurf]]

Comment on attachment 696569 [details] [diff] [review]
Switch to Services.jsm: suite message display (mailnews), v4

Need to remove the GetPrefService() call from mailWindow.js (as the function went away with this patch).

Comment 76

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite message display (mailnews), v5

In addition to the previous patch this one removes the GetPrefService() call from "function CreateMailWindowGlobals()".

Comment 77

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite downloads, v5

Fix bitrot from Bug 833015

Comment 78

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite downloads, v6

Updated patch for checkin (yes, it's happening really soon now)

Comment 79

[Frank Wein [:mcsmurf]]

Attachment: Switch to Services.jsm: suite feeds, v3

Patch updated for checkin.

Comment 80

[Frank Wein [:mcsmurf]]

I pushed all the changes to comm-central after checking for any obvious "Exception" messages and test failures in our tests:
https://hg.mozilla.org/comm-central/rev/72c20852b189 (backend patch)
https://hg.mozilla.org/comm-central/rev/033e8c1a0f19 (bookmarks and history patch)
https://hg.mozilla.org/comm-central/rev/954533734998 (browser patch)
https://hg.mozilla.org/comm-central/rev/7521116629d4 (common patch)
https://hg.mozilla.org/comm-central/rev/e84996845fab (compose patch)
https://hg.mozilla.org/comm-central/rev/f6139533313a (downloads patch)
https://hg.mozilla.org/comm-central/rev/1abd011504d4 (feeds patch)
https://hg.mozilla.org/comm-central/rev/9469da28530b (mailnews prefs patch)
https://hg.mozilla.org/comm-central/rev/3012cc6c16a4 (message display patch)
https://hg.mozilla.org/comm-central/rev/47e3113cae26 (preferences patch)
https://hg.mozilla.org/comm-central/rev/23c3a3219b7a (profiles patch)
https://hg.mozilla.org/comm-central/rev/e08a9159f21c (sanitizer patch)
https://hg.mozilla.org/comm-central/rev/018b75933621 (session restore patch)
https://hg.mozilla.org/comm-central/rev/578c72b610fc (sidebar patch)

Thanks for all the patches! I think this bug can be resolved as FIXED then? :-)