1608282 - Export symbols imported by XPCOMUtils.defineLazyGlobalGetters and Cu.importGlobalProperties by default in the shared global

Status: RESOLVED FIXED

(Firefox :: General, task, P5)

Description

[Brian Grinstead [:bgrins]]

It sounds like Cu.importGlobalProperties might continue to work even with Bug 1432901, but we might also need to change how it works. The lazy getters won't (no global this) but depending on how/if Cu.importGlobalProperties works it might not be necessary.

Comment 1

[Masatoshi Kimura [:emk]]

Won't globalThis work, either?

Comment 2

[Ted Campbell [:tcampbell]]

globalThis works in both JSMs and ES Modules. It will refer to the BackstagePass shared by the chrome modules. Doing something like (new Function("return this"))() should give the same value, but we would like to disable dynamic code in modules already.

Whether we allow globalThis is mainly a policy decision about interactions. Disallowing in privileged modules in favour of explicit import/export would be my preference, but I'm not the expert on frontend.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 4

[Tooru Fujisawa [:arai]]

Cu.importGlobalProperties defines properties on the shared global (= globalThis)
So it keeps working in the same way in ESM, without modification
(But of course it means those properties leak to other modules...)

Comment 5

[Tooru Fujisawa [:arai]]

Looks like Cu.importGlobalProperties isn't necessary at least in some case (see bug 1771378).
Maybe better checking each symbol imported with this, and either:

• remove the call if the symbol is avalable without the call
• support the symbol in the shared global if it's common

Comment 6

[Tooru Fujisawa [:arai]]

Most of XPCOMUtils.defineLazyGlobalGetters will be removed by bug 1772360.
Remaining are fetch, crypto, and indexedDB

Comment 7

[Nika Layzell [:nika] (ni? for response)]

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

Most of XPCOMUtils.defineLazyGlobalGetters will be removed by bug 1772360.
Remaining are fetch, crypto, and indexedDB

That's a remarkably small number. Perhaps we can consider explicitly exporting these few getters by default on the shared JSM/ESM global so that we don't need to use Cu.importGlobalProperties in these shared scopes anymore.

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1608282 - Part 1: Support fetch, crypto, and indexedDB in the system global by default. r?mccr8!

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1608282 - Part 2: Update jsm environment definition to include fetch, crypto, indexedDB. r?mccr8!

Depends on D149194

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1608282 - Part 3: Remove Cu.importGlobalProperties from JSM. r?mccr8!

Depends on D149195

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1608282 - Part 4: Remove Cu.defineLazyGlobalGetters from JSM. r?mccr8!

Depends on D149196

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e26e480c7dd6
Part 1: Support fetch, crypto, and indexedDB in the system global by default. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/ced8d82cc2d2
Part 2: Update jsm environment definition to include fetch, crypto, indexedDB. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/37c970364269
Part 3: Remove Cu.importGlobalProperties from JSM. r=extension-reviewers,kmag
https://hg.mozilla.org/integration/autoland/rev/32aebc8be201
Part 4: Remove Cu.defineLazyGlobalGetters from JSM. r=webdriver-reviewers,extension-reviewers,jdescottes,kmag

Comment 13

[Atila Butkovits]

Backed out for causing hazard failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/b767ba172d0e19fa7ecf3850ecab73f05b6d7990

Push where failures started: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=b93803e048621ff2fc8c37c50929bc7d6175a48d&selectedTaskRun=aIfTl5q-QzWqygodM-ruxw.1

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9c7f7ade029
Part 1: Support fetch, crypto, and indexedDB in the system global by default. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/78fa9a7f9087
Part 2: Update jsm environment definition to include fetch, crypto, indexedDB. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/b24017ef3ee7
Part 3: Remove Cu.importGlobalProperties from JSM. r=extension-reviewers,kmag
https://hg.mozilla.org/integration/autoland/rev/7f6fabbd9dc5
Part 4: Remove Cu.defineLazyGlobalGetters from JSM. r=webdriver-reviewers,extension-reviewers,jdescottes,kmag

Comment 15

[bszekely]

https://hg.mozilla.org/mozilla-central/rev/b9c7f7ade029
https://hg.mozilla.org/mozilla-central/rev/78fa9a7f9087
https://hg.mozilla.org/mozilla-central/rev/b24017ef3ee7
https://hg.mozilla.org/mozilla-central/rev/7f6fabbd9dc5