Closed Bug 1596918 Opened 3 months ago Closed 2 months ago

Bulk replace most ContentTask.spawn calls with SpecialPowers.spawn calls

Categories

(Testing :: Mochitest, task, P2)

Version 3
task

Tracking

(Fission Milestone:M4.1, firefox73 fixed)

RESOLVED FIXED
mozilla73
Fission Milestone M4.1
Tracking Status
firefox73 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

(Blocks 1 open bug, Regressed 2 open bugs)

Details

Attachments

(18 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

SpecialPowers.spawn has a more robust implementation than ContentTask.spawn, and is significantly more broadly available and Fission-compatible. Aside from making Fissionification easier, converting existing callers to use SpecialPowers.spawn should help alleviate some of the confusion of having two relatively-similar-but-slightly-different APIs, where it may not be obvious to everyone which one should be used, and when.

That said, there are still cases where have ContentTask.spawn calls which can't be converted to SpecialPowers.spawn. The main difference at this point is the lifetime of the task, where ContentTask.spawn is bound to the lifetime of a frameloader, and may persist across same-process navigations, and SpecialPowers.spawn is bound to the lifetime of a window global, and may not. In general, those kinds of tasks should be considered deprecated, and should be rewritten in such a way as to be agnostic to whether a navigation maintains the same docShell/process as the previous document. But that is a nontrivial amount of work.

Some other callers also depend on other esoterica of ContentTask, like the fact that it has different timing constraints than SpecialPowers.spawn (which always bounces tasks off of a parent actor before sending them to a child), which this bug doesn't intend to address immediately.

ContentTask.spawn supports some common global mochitest assertion methods as
aliases for corresponding Assert methods, along with espected-fail todo
variants, and the info method for logging messages without triggering
assertions. The easiest way to mass-convert existing callers is to just add
support for these to SpecialPowers.spawn, which this patch does.

Some tests currently use an initial ContentTask.spawn call to import certain
modules into the frame script global that subsequent tasks will run in. Since
each SpecialPowers.spawn task runs in its own sandbox, this method doesn't
work for them.

This patch adds a helper, SpecialPowers.addTaskImport, which allows similar
functionality by automatically importing the given module for any task spawned
by the SpecialPowers instance it was called on.

A number of additional globals are available to ContentTask.spawn tasks
compared to SpecialPowers.spawn tasks. Most of these are available by
accident, as a side-effect of running in a shared frame script global, or
being evaled in the context of the content-task.js frame script, but several
of them are pretty broadly useful, or difficult to obtain from a Sandbox
environment without reaching into arbitrary nearby globals.

This patch adds some of the more useful ones to the default task environment.

This is somewhat unrelated, but I stumbled across it when writing part 1a, and
it annoyed me.

This patch updates the existing ContentTask.spawn rule to do similar things
for SpecialPowers.spawn calls, only with a slightly different set of globals.

This is generally pretty straightforward, and rewrites nearly all calls. It
skips the ones that it can detect using frame script globals like
sendAsyncMessage, though.

Fission Milestone: --- → M4.1
Blocks: 1597804
No longer blocks: 1597804
Priority: -- → P2
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5ef6026806c8
Part 1a - Add support for more assertion/info methods to SpecialPowers.spawn. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/c6d356833bb8
Part 1b - Allow callers to automatically import JSMs for all SpecialPowers.spawn calls. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/ec79478f58f1
Part 1c - Make some more globals available to SpecialPowers.spawn tasks. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/97ec49dbbbf3
Part 1d - Fix some weird ChromeUtils.import calls. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/99b0421015d8
Part 1e - Correctly handle query handlers throwing uncatchable exceptions. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/12283166716f
Part 2 - Add ESLint support for SpecialPowers.spawn globals. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/b586fc081374
Part 3a - Scripted rewrite of most ContentTask.spawn calls to SpecialPowers.spawn calls. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/38c11ebfb8ff
Part 3b - Run code formatters on files changed by previous patch. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/44ef8f20732e
Part 4a - Re-add eslint-disable comments removed by rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/302c8ab8391c
Part 4b - Fix more ESLint issues after rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/be770d112dd8
Part 4c - Fix callers which depend on document lifecycle changes. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/96a79d2ba614
Part 4d - Fix callers which try to return non-clonable values. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/048db9f4db7c
Part 4e - Fix callers which rely on frame message manager globals. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/7d290bcd2067
Part 4f - Fix callers which have timing issues. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/67cc63ef5d7f
Part 4g - Misc cleanup/fixes. r=mccr8
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/775f3b06a687
Follow-up: Fix bustage after rebase.
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ab87d2c1afae
Follow-up: Fix more rebase bustage.
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8c1d67a02914
Part 1a - Add support for more assertion/info methods to SpecialPowers.spawn. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/1c0c0603f79f
Part 1b - Allow callers to automatically import JSMs for all SpecialPowers.spawn calls. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/d3ca99eedd7a
Part 1c - Make some more globals available to SpecialPowers.spawn tasks. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/1244972229bc
Part 1d - Fix some weird ChromeUtils.import calls. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/5bae838f369c
Part 1e - Correctly handle query handlers throwing uncatchable exceptions. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/e2312373dc55
Part 2 - Add ESLint support for SpecialPowers.spawn globals. r=Standard8
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/de196b077000
Part 3a - Scripted rewrite of most ContentTask.spawn calls to SpecialPowers.spawn calls. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/fa0f0aeabf99
Part 3b - Run code formatters on files changed by previous patch. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/7e9304405a46
Part 4a - Re-add eslint-disable comments removed by rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/c5404a7c286d
Part 4b - Fix more ESLint issues after rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/11ec4393f23d
Part 4c - Fix callers which depend on document lifecycle changes. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/0bca4de31d40
Part 4d - Fix callers which try to return non-clonable values. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/e5fd3ee22ea1
Part 4e - Fix callers which rely on frame message manager globals. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/011eb5ce927b
Part 4f - Fix callers which have timing issues. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/415007efd8c9
Part 4g - Misc cleanup/fixes. r=mccr8
Flags: needinfo?(kmaglione+bmo)
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a6ea596e98db
Part 3a - Scripted rewrite of most ContentTask.spawn calls to SpecialPowers.spawn calls. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/ef062eb7a6ee
Part 3b - Run code formatters on files changed by previous patch. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/a770c6d08d52
Part 4a - Re-add eslint-disable comments removed by rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/2f0036486823
Part 4b - Fix more ESLint issues after rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/3a0184e0df72
Part 4c - Fix callers which depend on document lifecycle changes. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/6a135670d603
Part 4d - Fix callers which try to return non-clonable values. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/5c9d9f141c10
Part 4e - Fix callers which rely on frame message manager globals. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/db09910ffa56
Part 4f - Fix callers which have timing issues. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/45a1c42118f2
Part 4g - Misc cleanup/fixes. r=mccr8
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/89f58295fb2b
Backed out 9 changesets for causing mochitest permafailures in toolkit/content/tests/chrome/test_findbar_events.xhtml CLOSED TREE
Flags: needinfo?(kmaglione+bmo)
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7d76a0d3d402
Part 3a - Scripted rewrite of most ContentTask.spawn calls to SpecialPowers.spawn calls. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/603e887cc955
Part 3b - Run code formatters on files changed by previous patch. r=mccr8,remote-protocol-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/5a5f2c1fdb6c
Part 4a - Re-add eslint-disable comments removed by rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/76d79937e9e7
Part 4b - Fix more ESLint issues after rewrite. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/aac9e14b1066
Part 4c - Fix callers which depend on document lifecycle changes. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/dee884fd1516
Part 4d - Fix callers which try to return non-clonable values. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/c460e495ae11
Part 4e - Fix callers which rely on frame message manager globals. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/d853d724b0d4
Part 4f - Fix callers which have timing issues. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/25f2c3149eed
Part 4g - Misc cleanup/fixes. r=mccr8
Regressions: 1603869
Regressions: 1603873
Regressions: 1603014
Regressions: 1603946
Regressions: 1604237
Regressions: 1605583
No longer regressions: 1603946
Regressions: 1610709
Regressions: 1484213
You need to log in before you can comment on or make changes to this bug.