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

Status: RESOLVED FIXED

(Testing :: Mochitest, task, P2)

Description

[Kris Maglione [:kmag]]

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.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 1a - Add support for more assertion/info methods to SpecialPowers.spawn. r=mccr8

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.

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 1b - Allow callers to automatically import JSMs for all SpecialPowers.spawn calls. r=mccr8

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.

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 1c - Make some more globals available to SpecialPowers.spawn tasks. r=mccr8

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.

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 1d - Fix some weird ChromeUtils.import calls. r=mccr8

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

Comment 5

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 1e - Correctly handle query handlers throwing uncatchable exceptions. r=mccr8

Comment 6

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 2 - Add ESLint support for SpecialPowers.spawn globals. r=Standard8

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

Comment 7

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 3a - Scripted rewrite of most ContentTask.spawn calls to SpecialPowers.spawn calls. r=mccr8

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.

Comment 8

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 3b - Run code formatters on files changed by previous patch. r=mccr8

Comment 9

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 4a - Re-add eslint-disable comments removed by rewrite. r=mccr8

Comment 10

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 4b - Fix more ESLint issues after rewrite. r=mccr8

Comment 11

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 4c - Fix callers which depend on document lifecycle changes. r=mccr8

Comment 12

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 4d - Fix callers which try to return non-clonable values. r=mccr8

Comment 13

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 4e - Fix callers which rely on frame message manager globals. r=mccr8

Comment 14

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 4f - Fix callers which have timing issues. r=mccr8

Comment 15

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Part 4g - Misc cleanup/fixes. r=mccr8

Comment 16

[Pulsebot]

Pushed by [e-mail]:
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

Comment 17

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Follow-up: Fix bustage after rebase.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/775f3b06a687
Follow-up: Fix bustage after rebase.

Comment 19

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Follow-up: Fix more rebase bustage.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab87d2c1afae
Follow-up: Fix more rebase bustage.

Comment 21

[Kris Maglione [:kmag]]

Attachment: Bug 1596918: Follow-up: Fix even more rebase bustage.

Comment 22

[Daniel Varga [:dvarga]]

Backed out 17 changesets (Bug 1596918) for multiple browser-chrome and dev-tools failures.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=280077183&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=67cc63ef5d7faafe22b6c6cb7d34bcdfdb3f7ef1

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=280077183&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=1a1c7ccfe04caeb4d9e3ee4ffbe5460767fcbc20

Failure logs:

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280087351&repo=autoland&lineNumber=3082

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280087408&repo=autoland&lineNumber=4247

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280087377&repo=autoland&lineNumber=3215

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280087419&repo=autoland&lineNumber=1845

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280087404&repo=autoland&lineNumber=12069

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280087394&repo=autoland&lineNumber=2569

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280087694&repo=autoland&lineNumber=4931

Comment 23

[Pulsebot]

Pushed by [e-mail]:
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

Comment 24

[Pulsebot]

Pushed by [e-mail]:
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

Comment 25

[Noemi Erli[:noemi_erli]]

Backed out 9 changesets (Bug 1596918) for causing multiple browser-chrome failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=415007efd8c9f98d8a4ce20a64537c8dc779d44f&selectedJob=280140721

Failure logs: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280141245&repo=autoland&lineNumber=1555
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280140721&repo=autoland&lineNumber=13094
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280140321&repo=autoland&lineNumber=9215

Backout: https://hg.mozilla.org/integration/autoland/rev/60951c2dc41ec463d0620c11b39c50352d23be13

Comment 26

[Pulsebot]

Pushed by [e-mail]:
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

Comment 27

[Stefan Hindli [:stefan_hindli]]

Backed out 9 changesets (Bug 1596918) for causing mochitest permafailures in toolkit/content/tests/chrome/test_findbar_events.xhtml CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=4f54a53cd1dae622d1693861e1ae6de26043a012&selectedJob=280594285

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280594285&repo=autoland&lineNumber=6791

Comment 28

[Pulsebot]

Backout by [e-mail]:
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

Comment 29

[Cristian Brindusan [:cbrindusan]]

There are also ss failures:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280602804&repo=autoland&lineNumber=2793

Comment 30

[Pulsebot]

Pushed by [e-mail]:
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

Comment 31

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/7d76a0d3d402
https://hg.mozilla.org/mozilla-central/rev/603e887cc955
https://hg.mozilla.org/mozilla-central/rev/5a5f2c1fdb6c
https://hg.mozilla.org/mozilla-central/rev/76d79937e9e7
https://hg.mozilla.org/mozilla-central/rev/aac9e14b1066
https://hg.mozilla.org/mozilla-central/rev/dee884fd1516
https://hg.mozilla.org/mozilla-central/rev/c460e495ae11
https://hg.mozilla.org/mozilla-central/rev/d853d724b0d4
https://hg.mozilla.org/mozilla-central/rev/25f2c3149eed