Historically we've had a lot of trouble with addons and our addon APIs leaking. Since we are implementing a new addon system in WebExtensions it would be great to start testing for leaks early in the process. I see we have a number of browser-chrome tests which does provide its own window leak detection. These tests, however, seem to mostly unload their addons before completing the test. I don't think this is really representative of what users will see since most people don't unload their addons that frequently. It would be great if we could add some WebExtension tests like the one I'm introducing in bug 1267693 for addon-sdk. This test does some addon work, opens/closes windows, verifies windows are collected, and then finally unloads the test addon.

Can you tell us what P2 means, please, Shell?

P2 to our next MVP - which is getting the next most widely used functionality into webextensions by 51 aurora. So it's not a blocker, but when we are looking to pull work - we pull first from P1 & P2 for this MVP. definitions here https://wiki.mozilla.org/Add-ons#Product_Backlog Is this something you have info on this being needed sooner than later? otherwise it's likely end of Q3 or into Q4 work.

I think that timeline is fine and am happy it's on your radar. Thanks.

Kris, do you know of any effort around adding tests around leaks in the APIs for Web Extensions APIs? With our traditional and SDK based add-ons we've consistently been bitten by add-ons calling into APIs and leaking stuff, so I'm trying to see if this is already on someone's radar. Thanks!

It's definitely on my radar, but I don't have any particularly good ideas at this point. We try to write APIs in a way that makes it hard to leak, and we run tests with normal leak checks turned on, but we don't have a good way to detect non-shutdown leaks. My best idea so far is to try to get telemetry on data in use by or held alive by extension compartments, and try to correlate data on things like ghost windows with extension usage. A lot of my focus next quarter will be on things like performance and telemetry, so I'll investigate more then.

One thing that billm mentioned to me last week was the fast that most WebExtension tests run as browser-chrome tests, where we have DOM window leak checking, so at least we effectively have some automated testing for DOM window leak checking.

Most run as browser-chrome or plain mochitests, yeah, but I've been migrating more of them to xpcshell as out mochitest runtimes have gotten longer. Those, unfortunately, still don't have leak checks :( I've been thinking about attaching finalization witnesses to certain objects and windows in debug mode tests, and adding checkpoints where we expect them to be freed. Will investigate how feasible that is later this week.

The problem with add-on sdk tests like this is the add-ons were uninstalled before the test completed. So any leaks held alive while the add-on is active is not found by end-of-test checking. We need a leak check *before* the web extension is removed.

(In reply to Kris Maglione [:kmag] from comment #5) > It's definitely on my radar, but I don't have any particularly good ideas at > this point. Can you add tests similar to what I did for addon-sdk here: https://dxr.mozilla.org/mozilla-central/source/addon-sdk/source/test/leak/leak-utils.js#38

And again, the browser-chrome memory checks are inadequate. They only verify we don't leak after removing the extension. We want to make sure we don't leak while the extension is installed.

Copying the gist of the leak test stuff here since it has been removed from m-c as part of removing addon-sdk. // Execute the given test function and verify that we did not leak windows // in the process. The test function must return a promise or be a generator. // If the promise is resolved, or generator completes, with an sdk loader // object then it will be unloaded after the memory measurements. exports.asyncWindowLeakTest = function*(assert, asyncTestFunc) { // Wait for the browser to finish loading. yield Startup.onceInitialized; // Track windows that are opened in an array of weak references. let weakWindows = []; function windowObserver(subject, topic) { let supportsWeak = subject.QueryInterface(Ci.nsISupportsWeakReference); if (supportsWeak) { weakWindows.push(Cu.getWeakReference(supportsWeak)); } } Services.obs.addObserver(windowObserver, "domwindowopened"); // Execute the body of the test. let testLoader = yield asyncTestFunc(assert); // Stop tracking new windows and attempt to GC any resources allocated // by the test body. Services.obs.removeObserver(windowObserver, "domwindowopened"); yield gc(); // Check to see if any of the windows we saw survived the GC. We consider // these leaks. assert.ok(weakWindows.length > 0, "should see at least one new window"); for (let i = 0; i < weakWindows.length; ++i) { assert.equal(weakWindows[i].get(), null, "window " + i + " should be GC'd"); } // Finally, unload the test body's loader if it provided one. We do this // after our leak detection to avoid free'ing things on unload. Users // don't tend to unload their addons very often, so we want to find leaks // that happen while addons are in use. if (testLoader) { testLoader.unload(); } }

Bulk move of bugs per https://bugzilla.mozilla.org/show_bug.cgi?id=1483958