Closed Bug 1425897 Opened 2 years ago Closed 2 years ago

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

Categories

(DevTools :: about:debugging, defect)

58 Branch
defect
Not set

Tracking

(firefox59 fixed)

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file)

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
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
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
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 on attachment 8938077 [details]
Bug 1425897 - Fix race condition in about:debugging test helper;

https://reviewboard.mozilla.org/r/208800/#review214850
Attachment #8938077 - Flags: review?(amarchesini) → review+
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae840f322103
Fix race condition in about:debugging test helper;r=baku
https://hg.mozilla.org/mozilla-central/rev/ae840f322103
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.