1777486 - Migrate XPCOMUtils.jsm

Status: RESOLVED FIXED

(Core :: XPConnect, task)

Description

[Tooru Fujisawa [:arai]]

Migrate Services.jsm and XPCOMUtils.jsm to ESM as an example

Given those 2 files are widely used in tree, this bug is not going to rewrite all consumers at once, but let the shim handle the redirection.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1777486 - Part 1: Migrate Services.jsm to ESM.

Depends on D150875

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1777486 - Part 2: Migrate XPCOMUtils.jsm to ESM.

Depends on D150876

Comment 3

[Kris Maglione [:kmag]]

We actually want to just get rid of Services.jsm, and automatically define Services on system globals instead. It would be better to do that now rather than migrating it once, and then migrating it again.

Comment 4

[Tooru Fujisawa [:arai]]

okay, that's bug 1667455.
I was about to do that later, but if preferred, let's do that now.

Comment 5

[Tooru Fujisawa [:arai]]

(In reply to Kris Maglione [:kmag] from comment #3)

We actually want to just get rid of Services.jsm

Just to make sure, we cannot remove it right now, because of out-of-tree compatibility.

I think we can keep it as JSM right now, or maybe just ESMify itself alone and keep it,
and add "Services.jsm removal is planned" to the out-of-tree migration announcement,
and once the out-of-tree migration finishes, we can remove it.

in bug 1667455, I'll add the global property and remove in-tree consumers as much as possible
(special powers cases and sandbox need another fix, maybe?).

Comment 6

[Kris Maglione [:kmag]]

(In reply to Tooru Fujisawa [:arai] from comment #5)

(In reply to Kris Maglione [:kmag] from comment #3)

We actually want to just get rid of Services.jsm

Just to make sure, we cannot remove it right now, because of out-of-tree compatibility.

Exactly what out-of-tree compatibility are you worried about?

Comment 7

[Tooru Fujisawa [:arai]]

in the context of ESMification, the out-of-tree compatibility is described in bug 1766801,

(from bug 1766801 comment #0)

So far, the following will be affected:

• Firefox extensions that uses experiments
  • this requires privileged signature. so there won't be many
  • some of https://github.com/mozilla-extensions/
• AutoConfig script
  • written by users (enterprise case etc)
  • needs detailed documentation for rewrite
  • needs some cycles compatible both with before/after
• Thunderbird extensions
  • experiments API is available for all extensions
  • needs detailed documentation for rewrite (can be share the content with AutoConfig?)
  • needs some cycles compatible both with before/after

and I think the same thing applies to when removing Services.jsm.

Comment 8

[Tooru Fujisawa [:arai]]

we're trying to avoid requiring authors of out-of-tree code to rewrite multiple times during the ESMification,
so the ESMification process is designed to keep the maximum compatibility (bug 1766761 etc).

Thus, if possible I'd like to apply the same rule here.

Comment 9

[Kris Maglione [:kmag]]

(In reply to Tooru Fujisawa [:arai] from comment #7)

in the context of ESMification, the out-of-tree compatibility is described in bug 1766801,

(from bug 1766801 comment #0)

So far, the following will be affected:

• Firefox extensions that uses experiments
  • this requires privileged signature. so there won't be many
  • some of https://github.com/mozilla-extensions/

Part of the process for getting privileged signature is agreeing that you are responsible for keeping up with any changes to Firefox internals that you rely on. That's why we removed support for legacy extensions.

• AutoConfig script
  • written by users (enterprise case etc)
  • needs detailed documentation for rewrite
  • needs some cycles compatible both with before/after

Autoconfig scripts aren't meant to write privileged JS. If they're loading Services.jsm, it's unsupported.

• Thunderbird extensions
  • experiments API is available for all extensions
  • needs detailed documentation for rewrite (can be share the content with AutoConfig?)
  • needs some cycles compatible both with before/after

Thunderbird extensions are not our problem. Thunderbird continued supporting privileged code in their extensions at their own risk. When we removed support in Firefox, we stopped worrying about how code changes would affect them. That was one of our primary goals in doing so.

Comment 10

[Kris Maglione [:kmag]]

Also note that Thunderbird is built from ESR, so extensions should generally have to worry about updating for breaking changes much less frequently.

Comment 11

[Tooru Fujisawa [:arai]]

okay. I don't oppose to the Services.jsm removal.

but looks like Services.jsm is still necessary in the following cases even after exposing Services on all privileged scopes:

• require("resource://gre/modules/Services.jsm")
• SpecialPowers.ChromeUtils.import("resource://gre/modules/Services.jsm")

They need different approach to expose Services object to the global, and
I'd like to leave them to other bugs, given I don't know much about the environment.

Comment 12

[Tooru Fujisawa [:arai]]

testing the patch in https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=cacc0ce3a9d09c996297a87e6d17285d45937a9b
(to be clear, it's for bug 1667455)

Comment 13

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #0)

Given those 2 files are widely used in tree, this bug is not going to rewrite all consumers at once, but let the shim handle the redirection.

After working on bug 1667455, now I feel it's better just convert all consumers in this bug

Comment 14

[Kris Maglione [:kmag]]

(In reply to Tooru Fujisawa [:arai] from comment #11)

okay. I don't oppose to the Services.jsm removal.

but looks like Services.jsm is still necessary in the following cases even after exposing Services on all privileged scopes:

• require("resource://gre/modules/Services.jsm")

The devtools module loader has special cases to import special global symbols already:

https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/devtools/shared/loader/base-loader.js#486-500

• SpecialPowers.ChromeUtils.import("resource://gre/modules/Services.jsm")

We already have a SpecialPowers.Services getter that they should be using instead.

Comment 15

[Kris Maglione [:kmag]]

(In reply to Kris Maglione [:kmag] from comment #14)

(In reply to Tooru Fujisawa [:arai] from comment #11)

• require("resource://gre/modules/Services.jsm")

The devtools module loader has special cases to import special global symbols already:

I think the most consistent way to handle this would be to add Services to the require("chrome") exports and then do const { Services } = require("chrome")

Comment 16

[Tooru Fujisawa [:arai]]

Thanks. I'll look into fixing them.

Comment 17

[Tooru Fujisawa [:arai]]

testing batch migration
https://treeherder.mozilla.org/jobs?repo=try&revision=efb357037b627a4975cf73092bdcb27351a8dc3a&group_state=expanded
there are remaining not-migrated consumers, but that's also covered by the redirect shim (I'm going to rewrite them too tho)

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 1: Migrate XPCOMUtils.jsm to XPCOMUtils.sys.mjs. r=kmag!

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 2: Migrate XPCOMUtils.jsm consumers with automatic migration. r=kmag!

Depends on D151213

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 3: Migrate XPCOMUtils.jsm consumers with manual rewrite. r=kmag!

Migrate some files that's not covered by ./mach esmify

Depends on D151214

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 4: Migrate XPCOMUtils.jsm consumers with CommonJS require. r=ochameau!

Depends on D151215

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 5: Remove unnecessary XPCOMUtils.jsm imports. r=kmag!

Depends on D151216

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 6: Update testcase that monitors XPCOMUtils imports. r=kmag!

Depends on D151217

Comment 24

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 7: Update testcase that loads XPCOMUtils as resource:// URI example. r=kmag!

Depends on D151218

Comment 25

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 8: Update testcase comments that mentions XPCOMUtils. r=kmag!

Depends on D151219

Comment 26

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 9: Do not use XPCOMUtils in tests for JSM loader. r=kmag!

Depends on D151220

Comment 27

[Tooru Fujisawa [:arai]]

Attachment: Bug 1777486 - Part 0: Use AppConstants.jsm in Cu.getGlobalForObject consumer. r=kmag!

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0867652fc48
Part 0: Use AppConstants.jsm in Cu.getGlobalForObject consumer. r=kmag
https://hg.mozilla.org/integration/autoland/rev/9ce3a6496a49
Part 1: Migrate XPCOMUtils.jsm to XPCOMUtils.sys.mjs. r=kmag
https://hg.mozilla.org/integration/autoland/rev/eea573d3a9f9
Part 2: Migrate XPCOMUtils.jsm consumers with automatic migration. r=webdriver-reviewers,geckoview-reviewers,preferences-reviewers,application-update-reviewers,pip-reviewers,kmag,owlish,whimboo
https://hg.mozilla.org/integration/autoland/rev/0559c53cc668
Part 3: Migrate XPCOMUtils.jsm consumers with manual rewrite. r=kmag
https://hg.mozilla.org/integration/autoland/rev/0c4ea0b9416b
Part 4: Migrate XPCOMUtils.jsm consumers with CommonJS require. r=devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/e938b601ba15
Part 5: Remove unnecessary XPCOMUtils.jsm imports. r=kmag
https://hg.mozilla.org/integration/autoland/rev/6d758fab5a3e
Part 6: Update testcase that monitors XPCOMUtils imports. r=kmag
https://hg.mozilla.org/integration/autoland/rev/d5fd8173d62d
Part 7: Update testcase that loads XPCOMUtils as resource:// URI example. r=kmag
https://hg.mozilla.org/integration/autoland/rev/20c746fb1648
Part 8: Update testcase comments that mentions XPCOMUtils. r=kmag
https://hg.mozilla.org/integration/autoland/rev/195cc2de8433
Part 9: Do not use XPCOMUtils in tests for JSM loader. r=kmag

Comment 29

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1777486 - Part 10: Migrate more XPCOMUtils.jsm consumer.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6c4c386f1a6
Part 10: Migrate more XPCOMUtils.jsm consumer CLOSED TREE

Comment 31

[Andreea Pavel [:apavel]]

Backed out for failing bc at browser_startup.js

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=195cc2de8433636327734892450437ea803e33a9&selectedTaskRun=NL3YUI_oTcWav7Zeu0wAAA.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=383977667&repo=autoland&lineNumber=9543

Backout: https://hg.mozilla.org/integration/autoland/rev/e3f2586776345018a48280a29969ebcc1c81ccea

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a0dd72b4fc8
Part 0: Use AppConstants.jsm in Cu.getGlobalForObject consumer. r=kmag
https://hg.mozilla.org/integration/autoland/rev/e635d9fe6694
Part 1: Migrate XPCOMUtils.jsm to XPCOMUtils.sys.mjs. r=kmag
https://hg.mozilla.org/integration/autoland/rev/a4c798d7c7a4
Part 2: Migrate XPCOMUtils.jsm consumers with automatic migration. r=webdriver-reviewers,geckoview-reviewers,preferences-reviewers,application-update-reviewers,pip-reviewers,kmag,owlish,whimboo
https://hg.mozilla.org/integration/autoland/rev/c835a48d9fed
Part 3: Migrate XPCOMUtils.jsm consumers with manual rewrite. r=kmag
https://hg.mozilla.org/integration/autoland/rev/23d68f59a55b
Part 4: Migrate XPCOMUtils.jsm consumers with CommonJS require. r=devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/bc9958735137
Part 5: Remove unnecessary XPCOMUtils.jsm imports. r=kmag
https://hg.mozilla.org/integration/autoland/rev/55d14dac3081
Part 6: Update testcase that monitors XPCOMUtils imports. r=kmag
https://hg.mozilla.org/integration/autoland/rev/8565cf14feb4
Part 7: Update testcase that loads XPCOMUtils as resource:// URI example. r=kmag
https://hg.mozilla.org/integration/autoland/rev/1e3337d7fa6c
Part 8: Update testcase comments that mentions XPCOMUtils. r=kmag
https://hg.mozilla.org/integration/autoland/rev/85f4c091208b
Part 9: Do not use XPCOMUtils in tests for JSM loader. r=kmag

Comment 33

[bszekely]

https://hg.mozilla.org/mozilla-central/rev/0a0dd72b4fc8
https://hg.mozilla.org/mozilla-central/rev/e635d9fe6694
https://hg.mozilla.org/mozilla-central/rev/a4c798d7c7a4
https://hg.mozilla.org/mozilla-central/rev/c835a48d9fed
https://hg.mozilla.org/mozilla-central/rev/23d68f59a55b
https://hg.mozilla.org/mozilla-central/rev/bc9958735137
https://hg.mozilla.org/mozilla-central/rev/55d14dac3081
https://hg.mozilla.org/mozilla-central/rev/8565cf14feb4
https://hg.mozilla.org/mozilla-central/rev/1e3337d7fa6c
https://hg.mozilla.org/mozilla-central/rev/85f4c091208b