Closed Bug 1517406 Opened 5 years ago Closed 5 years ago

Fix more tests related to service workers

Categories

(Core :: DOM: Service Workers, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: mrbkap, Assigned: mrbkap)

References

Details

Attachments

(5 files)

I have a few of patches fixing tests in dom/push and dom/serviceworkers. I also was able to enable the dom/serviceworkers directory and disable individual tests. I'll file follow-ups on the disabled tests tomorrow (though a few of them will be fixed by moving service workers back down to the content process because the tests are using console listeners in the wrong process).

I also threw in a couple of patches that were helpful in tracking down problems.
The push notifier needs to proxy its calls to the proper process in order to
notify service workers. With parent-intercept enabled, that means making sure
we notify in the parent and without it, that we notify in the content process.
Fortunately, we already have to do this proxying for the observer
notifications, so we can just piggyback on top of that code to make things
work for service workers.
This test listens for an observer service notification from the
`ServiceWorkerManager`, but with parent-intercept on, it listens in the wrong
process. This uses the magic of SpecialPowers to listen in the correct
process.

Unfortunately, this test still fails because it leaks.

Depends on D15634
Most of these tests pass now, so enable them and only disable the tests that
don't pass. This should make driving these tests to all passing easier to do
and prevent regressions.

Depends on D15635
This test first creates a service worker and then an iframe. It expects the
iframe's first load to be through the network (and for the page's script to
fail to load). Once the page is set up and controlled by the SW, it reloads it
(a soft reload) and the script load is intercepted by the service worker and
we load a script that calls back into the test. However, there is currently no
guarantee that the service worker won't activate before the iframe load and
end up controlling the first load of the iframe.

With this patch, we delay the service worker's activation until we start
loading the iframe, allowing the first load to always be uncontrolled, thereby
resolving the race.

Depends on D15636
Currently, if a test fails to clean up a service worker, we report every test
that runs after it as having failed to clean up their service workers as well.
That fills up the logs and makes it harder to figure out what's going on. This
patch removes the left-over service workers automatically, reducing the log
output and pointing the finger more precisely to the failing test.

I believe this patch is correct because afterCleanup is responsible for
continuing the test run by calling a function (either
`parentRunner.testFinished` or `SimpleTest.showReport`). Therefore, we can
safely make it async and wait if we need to clean up after a poorly-written
test.

Depends on D15637
Priority: -- → P2
I noticed that this failed to land and needs rebasing. asuth, would you mind helping me out?
Flags: needinfo?(bugmail)

(Landed. I took over the phabricator reviews because I hoped it would be easy to push updates to Phabricator, but... it was not. So I just pushed directly and thankfully the Phabricator reviews magically closed themselves after they had magically re-opened themselves after I commandeered the revisions.)

Flags: needinfo?(bugmail)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: