1449940 - Merge nsSetDefaultBrowser.js into nsBrowserContentHandler.js

Status: RESOLVED FIXED

(Firefox :: General, enhancement)

Description

[Florian Quèze [:florian]]

Like all command line parameter handlers, nsSetDefaultBrowser.js is an xpcom component created at each startup even when the related command line parameter isn't provided by the user on the command line.

Loading the component has overhead. Here is a profile on a slow netbook: https://perfht.ml/2uwo4Da

nsSetDefaultBrowser.js also imports ShellService.jsm synchronously instead of using a lazy getter.

This file is so trivial that we could easily merge it into nsBrowserContentHandler.js which handles the other Firefox-specific command line parameters.

Comment 1

[Ludovic Hirlimann [:Usul]]

Attachment: 1449940.patch

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 8963931 [details] [diff] [review]
1449940.patch

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

Looks good. It would be nice to add a line in browser_startup.js to verify that we are no longer importing ShellService.jsm early.

For the next version, please provide 8 lines of context and include a commit message in the patch.

::: browser/components/nsBrowserContentHandler.js
@@ +475,4 @@
>  
>    get helpInfo() {
>      let info =
> +              "  --setDefaultBrowser Set this app as the default browser.\n" +

nit: This list seems mostly alphabetically sorted, I would put this line after the --search one.

Comment 3

[Ludovic Hirlimann [:Usul]]

Attachment: patch1449940_3.patch

Comment 4

[Florian Quèze [:florian]]

Comment on attachment 8964114 [details] [diff] [review]
patch1449940_3.patch

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

I fixed the obvious issue before pushing to try https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dfd8a29cbc77c35a7211da3ee5537f0bc19a126 and try seems green.

::: browser/components/nsBrowserContentHandler.js
@@ +478,5 @@
>                "  --browser          Open a browser window.\n" +
>                "  --new-window <url> Open <url> in a new window.\n" +
>                "  --new-tab <url>    Open <url> in a new tab.\n" +
> +              "  --private-window <url> Open <url> in a new private window.\n"
> +              "  --setDefaultBrowser Set this app as the default browser.\n"; 

This change is broken:
- trailing whitespace
- no '+' operator between the 2 lines.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1422e74a0a87
Merge nsSetDefaultBrowser.js into nsBrowserContentHandler.js to remove one xpcom component loaded at startup, r=florian.

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/1422e74a0a87