Closed Bug 1588618 Opened Last month Closed 11 days ago

Make browser_xhr_onchange_leak.js Fission-compatible

Categories

(Core :: DOM: Core & HTML, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla72
Fission Milestone M4
Tracking Status
firefox72 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

browser_xhr_onchange_leak.js is a test I wrote a while ago for a leak fix in bug 1336811. The idea is that you load a certain page, then navigate away from it and end the test. If there's a leak, then the "leaked window until shutdown" detector will find it.

Right now the test does a cross-origin navigation, which breaks with Fission because we start tearing down the old process, then attempt to use the message manager in the old process. A simple fix for this is to change the navigation to be same-origin.

Amazingly, if you revert the leak fix part of bug 1336811, in non-Fission you can still reproduce a leak. However, with my fix for Fission in the previous paragraph, it no longer actually leaks. This appears to be due to the fact that Fission does not support the dom.ipc.keepProcessesAlive.* preference (bug 1580212) because if I hack that up to affect all processes, then I can reproduce the leak. Presumably what happens is that without that pref when you close the tab that contains the test it just tears everything down.

Probably I should rewrite this test to manually check if the page went away after the navigation. Either that or I think I could hack up dom.ipc.keepProcessesAlive.* to work in Fission, and then set the pref. This also makes me worry about what kind of test coverage we might be losing by dropping that pref.

I should also take a look at the tests I changed in bug 1572781, which are similar-ish leak tests.

I usually handle these sorts of things by attaching a finalization witness object to the window, either directly or via a weak map, and then loop (triggering GC/CC each iteration) until I get the observer notification that it went away. It turns out that you sometimes wind up needing to do a shrinking GC sometimes if the object winds up being held alive by an IC, though...

Depends on: 1580212

The way this test works is, it loads a page, then it navigates to
another page and exits. The mochitest browser-chrome leak detector
will report a leak if the first page isn't freed by the end of the
test.

To make this Fission-compatible, I first changed the page we navigate
to to be same-origin. This means that we won't tear down the original
process immediately when we navigate, which will break the various
test harness stuff.

Secondly, I enable the dom.ipc.keepProcessesAlive.webIsolated
preference, which means that the content process won't be torn down
immediately. If this pref is not set, then the usual process reuse
ends up destroying the window before the test ends, whether or not it
leaks.

I verified that this test fails when the patch from bug 1336811 is
backed out, both with and without Fission.

Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/53b75b879af6
Make browser_xhr_onchange_leak.js Fission-compatible. r=smaug
Status: NEW → RESOLVED
Closed: 11 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

Retroactively moving fixed bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to an appropriate Fission Milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → M4
You need to log in before you can comment on or make changes to this bug.