1603190 - Enable windowless-service-workers in Nightly

Status: RESOLVED FIXED

(DevTools :: Debugger, task, P3)

Description

[:Harald Kirschner :digitarald]

Enable devtools.debugger.features.windowless-service-workers on Nightly so we can start gathering feedback.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1603190 - Enable windowless-service-workers in Nightly

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I tested fix for Bug 1604526 and it works for me just fine now.
Enabling on Nightly

Honza

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47f8d76aa4fc
Enable windowless-service-workers in Nightly r=jlast

Comment 4

[Dorel Luca [:dluca]]

Backed out changeset 47f8d76aa4fc (bug 1603190) for Devtools failures in devtools/client/framework/test/browser_toolbox_view_source_02.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284639188&repo=autoland&lineNumber=2758

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

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

https://treeherder.mozilla.org/logviewer.html#?job_id=284640285&repo=autoland

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=47f8d76aa4fc9a21a03d7163cc0b4747a65adfe0

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

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks Brian for working on this!

Honza

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Harald, anything blocking us from enabling on Nightly?

Honza

Comment 8

[:Harald Kirschner :digitarald]

Julian, should bug 1618546 block enabling this or do you see any remaining blockers?

Comment 9

[Julian Descottes [:jdescottes]]

As far as I know, only Bug 1618546 is a critical blocker.

Comment 10

[:Harald Kirschner :digitarald]

Honza, since 1618546 is closed – shall we try again?

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to :Harald Kirschner :digitarald from comment #10)

Honza, since 1618546 is closed – shall we try again?

Yes, just checked with Julian. There are no other blockers, so let's enable it.
Julian will submit a patch.

Just for the record, enabling SW on DevEdition (bug 1603203) depends on results of an experiment, which is supposed to run on Beta soon.

Honza

Comment 12

[Julian Descottes [:jdescottes]]

Will try to land this today, I'm just testing the existing patch a bit.
It looks odd as it modifies tests. If tests need to be modified to work on Nightly (with the pref on) then they would break on Beta I suppose, which isn't acceptable.

Comment 13

[Julian Descottes [:jdescottes]]

The test modifications don't seem necessary, doing a final cleanup of the patch and re-requesting review.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/116c370683f6
Enable windowless-service-workers in Nightly r=jlast,Honza

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/116c370683f6

Comment 16

[David Walsh :davidwalsh]

Along with the 1592839 issues, I think this change also created other intermittent failures:

devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js | A promise chain failed to handle a rejection: Can not send request 'listServiceWorkerRegistrations' because front 'root' is already destroyed. - stack: generateRequestMethods/</frontProto[name]@resource://devtools/shared/protocol/Front/FrontClassWithSpec.js:29:15
1632695Intermittent devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js | A promise chain failed to handle a rejection: can't access property "send", this.transport is null - stack: send@resource://devtools/server/devtools-server-connection.js:91:
1632752Intermittent devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js | A promise chain failed to handle a rejection: Can not send request 'listServiceWorkerRegistrations' because front 'root' is already destroyed. - stack: generateRequestMethod

Comment 17

[Julian Descottes [:jdescottes]]

(In reply to David Walsh :davidwalsh from comment #16)

Along with the 1592839 issues, I think this change also created other intermittent failures:

devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js | A promise chain failed to handle a rejection: Can not send request 'listServiceWorkerRegistrations' because front 'root' is already destroyed. - stack: generateRequestMethods/</frontProto[name]@resource://devtools/shared/protocol/Front/FrontClassWithSpec.js:29:15
1632695Intermittent devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js | A promise chain failed to handle a rejection: can't access property "send", this.transport is null - stack: send@resource://devtools/server/devtools-server-connection.js:91:
1632752Intermittent devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js | A promise chain failed to handle a rejection: Can not send request 'listServiceWorkerRegistrations' because front 'root' is already destroyed. - stack: generateRequestMethod

It's entirely possible that flipping this pref made more tests intermittent. By definition, when you listen for service worker updates, you need to update the list every time a process spawns or dies, which can be pretty frequent. And often the test will end while we are trying to fetch a worker update.

To make the debugger tests less intermittent, first we should try to make process updates less "random" by setting await pushPref("dom.ipc.processPrelaunch.enabled", false); before we run any test in the debugger test suite. If that's not enough, it might be a sign that the debugger tests are not waiting properly for some event/action. We could look at individual tests and see if using waitForRequestsToSettle can help. And if it's still not enough, depending on how many tests are impacted, we might ignore promise rejections in a few tests.

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the input Julian!

To make the debugger tests less intermittent, first we should try to make process updates less "random" by setting await
pushPref("dom.ipc.processPrelaunch.enabled", false); before we run any test in the debugger test suite.

Should we file a bug for this?

Comment 19

[Julian Descottes [:jdescottes]]

Should we file a bug for this?

No need for now, I will try to do this for the intermittent Bug 1592839 which has spiked since we landed this.

Comment 20

[Julian Descottes [:jdescottes]]

(In reply to Julian Descottes [:jdescottes] from comment #19)

Should we file a bug for this?

No need for now, I will try to do this for the intermittent Bug 1592839 which has spiked since we landed this.

Sorry just looked at Bug 1592839 and it seems to be a different issue. I'll file a separate bug.