1425897 - Investigate about:debugging service worker push test failing with Bug 1419771

Status: RESOLVED FIXED

(DevTools :: about:debugging, defect)

Description

[Julian Descottes [:jdescottes]]

See backlog in Bug 1419771: we still have an about:debugging test failure when rebased on top of the patches from Bug 1419771.

Right now I still need ~5 separate tests running sequentially to trigger the bug. It seems linked with multi e10s as suggested by :asuth in Bug 1386613 comment 20. When the push test fails, we get two different instances of the service worker script: the one which is installed from the test content page, and another one which receives the push. I am not sure how to log the process ID from the sw script however, so I can't tell for sure if they are running in two separate processes.

about:debugging tests are fiddling with the dom.ipc.processCount preference in every service worker test and particularly in browser_service_workers_multi_content_process.js where we force it to 2 to check that a warning message is displayed. 

As a workaround, moving browser_service_workers_multi_content_process.js to run after the other about:debugging tests seems to fix the issue. Since your patch is about preferences, and it seems the failure is linked to the dom.ipc.processCount preference not behaving as expected, I'm not sure we should go for a workaround here.

You can see the current test case I use at:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2170bf6364374e1f7061428ae0473af128a1913b

The list of tests running here are :
- browser_service_workers.js
- browser_service_workers_fetch_flag.js: just a clone of browser_service_workers.js
- browser_service_workers_multi_content_process.js: clone of browser_service_workers.js + setting dom.ipc.processCount to 2
- browser_service_workers_not_compatible.js: just an empty ok(true) test
- browser_service_workers_push.js

Comment 1

[Julian Descottes [:jdescottes]]

For some reason the failures are much more frequent when running only the about:debugging tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2de4a9c7dec54f6f8ad9b3242d822f3b501c5614

Comment 2

[Julian Descottes [:jdescottes]]

The following code might be racy: https://searchfox.org/mozilla-central/rev/ff462a7f6b4dd3c3fdc53c9bc0b328f42a5b3d2b/devtools/client/aboutdebugging/test/head.js#405-415

From the symptoms, it looks like a previous content process remains from an older test, and in the snippet above we are clearing cached processes (potentially) before setting the dom.ipc.contentProcess pref to 1. Try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0409516c3d2c4252492d7508e59a9fb033151b4

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1425897 - Fix race condition in about:debugging test helper;

Review commit: https://reviewboard.mozilla.org/r/208800/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208800/

Comment 4

[Julian Descottes [:jdescottes]]

Try is green so it looks like this was the root cause of the failures. As I said the code looked racy from the start, so I think that it is acceptable that the patches from Bug 1419771 trigger the race condition more frequently.

Global devtools try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=61f5d70e492c4cf1cbef65c6b9524fe0d9801c85

Comment 5

[Andrea Marchesini [:baku]]

Comment on attachment 8938077 [details]
Bug 1425897 - Fix race condition in about:debugging test helper;

https://reviewboard.mozilla.org/r/208800/#review214850

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae840f322103
Fix race condition in about:debugging test helper;r=baku

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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