758979 - convert mailnews/base/src/*.js to Services.jsm and MailServices.js

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[:aceman]

msgAsyncPrompter.js:  this._threadManager = Cc["@mozilla.org/thread-manager;1"]
nsMailNewsCommandLineHandler.js:        let ioService = Cc["@mozilla.org/network/io-service;1"]
nsMailNewsCommandLineHandler.js:      let windowMediator = Cc["@mozilla.org/appshell/window-mediator;1"]
nsMailNewsCommandLineHandler.js:        let windowWatcher = Cc["@mozilla.org/embedcomp/window-watcher;1"]
virtualFolderWrapper.js:    Cc["@mozilla.org/messenger/account-manager;1"]
virtualFolderWrapper.js:      Cc["@mozilla.org/messenger/services/filters;1"]

I wonder why these .js files are even here under /src. They should probably be in /content. Would it be OK to move them?

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

(In reply to :aceman from comment #0)
> I wonder why these .js files are even here under /src. They should probably
> be in /content. Would it be OK to move them?

No, they are JS modules (loaded by Components.utils.import) or JS components (loaded by XPCOM infrastructure per their manifest definitions).  If you look in Makefile.in, you will see they are listed under EXTRA_COMPONENTS or EXTRA_JS_MODULES (or PP variants).

Files in /content go there because they are scripts loaded into the global scope by 'script' tags.

Comment 2

[:aceman]

Thanks. So why not mailnews/base/util?

Comment 3

[:aceman]

Attachment: patch

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

(In reply to :aceman from comment #2)
> Thanks. So why not mailnews/base/util?

Attempts at consistency, even if we lack the dependency reasons that might have motivated the C++ directory structure.  For example virtualFolderWrapper.js deals with virtual folders, and nsMsgSearchDBView.cpp and nsMsgXFVirtualFolderDBView.cpp which implement the actual virtual folder logic live in the src/ directory, so it's more appropriate than util/ which is more a home for data-types/catch-all fallback.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 627580 [details] [diff] [review]
patch

I'm busy this week, so passing review on.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 627580 [details] [diff] [review]
patch

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

::: mailnews/base/src/msgAsyncPrompter.js
@@ +84,5 @@
>  };
>  
>  function msgAsyncPrompter() {
>    this._pendingPrompts = {};
> +  this._threadManager = Services.tm;

Is there even a point of having this be aliased? We might as well just use Services.tm everywhere, instead of _threadManager.

Comment 7

[:aceman]

Maybe for external callers, but I couldn't find any via http://mxr.mozilla.org/comm-central/search?string=_threadManager&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central .

Comment 8

[Mike Conley (:mconley) (:⚙️)]

The underscore indicates that this should only be used internally. Anybody reaching in and attempting to manipulate something starting with an underscore are playing a dangerous game.

So I think you're safe to just replace it with Services.tm.

Comment 9

[:aceman]

Attachment: patch v2

OK.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 631510 [details] [diff] [review]
patch v2

Looks good to me - thanks for your work,

-Mike

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/4ef0d79ae8b4