1164077 - Implement WorkerActor.attach

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Eddy Bruel [:ejpbruel]]

Bug 1160199 made it possible to obtain a list of WorkerActors, one for each worker. The next step is to make it possible to attach to a WorkerActor.

Attaching to a WorkerActor allows you to listen to worker events such as close, freeze, and thaw.

The reason we want to listen to freeze and thaw events is this: when a tab navigates to a new page, the previous page is put in the bfcache whenever possible. Because they are not destroyed, any workers on the previous page are still listed for the tab, even though they do not belong to the current page.

To work around this, we use the following observation: when a page is moved into the bfcache, any workers on that page are frozen. When the page is moved out of the bfcache, the workers are thawed again. By keeping track of which workers are frozen, we can obtain the list of workers for the current page from the list of all workers for the tab, by filtering out workers in the bfcache.

The reason we want to listen to close events is simply so we can detect when a worker has closed. We cannot detect this from the worker itself, for obvious reasons.

Comment 1

[Eddy Bruel [:ejpbruel]]

Attachment: WorkerActor.attach

Hopefully, this patch is self-explanatory. If there's anything unclear, let me know.

Comment 2

[Eddy Bruel [:ejpbruel]]

Try push for this patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec4600a446f9

Comment 3

[Eddy Bruel [:ejpbruel]]

The try push showed a failing test, which is probably caused by a bug in the frame script where I don't check it a function actually returns a result before checking if it has a then property. Here's a new try push of the patch with that issue addressed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7228d608581

Comment 4

[James Long (:jlongster)]

Comment on attachment 8604736 [details] [diff] [review]
WorkerActor.attach

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

I don't see anything alarming in here, looks good

::: browser/devtools/debugger/test/browser_dbg_workeractor.js
@@ +7,5 @@
> +
> +function test() {
> +  Task.spawn(function* () {
> +    let oldMaxTotalViewers = SpecialPowers.getIntPref(MAX_TOTAL_VIEWERS);
> +    SpecialPowers.setIntPref(MAX_TOTAL_VIEWERS, 10);

What is SpecialPowers?

Comment 5

[Eddy Bruel [:ejpbruel]]

Attachment: WorkerActor.attach

I've made some last minute changes to the test in this patch, in order to deal with some issues in how we instrument workers that I discovered in a later patch.

Since the essence of the test hasn't changed, this probably doesn't require a re-review, so I'm going to carry over the r+ from jlong, and do another try push before landing just in case:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=20d555753566

Comment 6

[Eddy Bruel [:ejpbruel]]

Try run looks good, landing on fx-team:
https://hg.mozilla.org/integration/fx-team/rev/f28313729236

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f28313729236