Closed
Bug 1425897
Opened 7 years ago
Closed 7 years ago
Investigate about:debugging service worker push test failing with Bug 1419771
Categories
(DevTools :: about:debugging, defect)
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
Assignee | ||
Comment 1•7 years ago
|
||
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
Assignee | ||
Comment 2•7 years ago
|
||
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 hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
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•7 years ago
|
||
mozreview-review |
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 | ||
Updated•7 years ago
|
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
Comment 7•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•