An addon iframes' extension context is closed correctly when an extension is unloaded, but it fails if the iframe is removed with the exception: TypeError: this.extensionWindows is undefined because of a typo in the DocumentManager.observe method. I'm going to attach to this issue a patch with an additional test case which raise the exception, and the related fix.

Comment on attachment 8711883 [details] [diff] [review] 001-fix-exception-on-iframe-destroy.patch Review of attachment 8711883 [details] [diff] [review]: ----------------------------------------------------------------- We should request uplift to aurora for this. ::: toolkit/components/extensions/test/mochitest/test_ext_contentscript_create_iframe.html @@ +210,5 @@ > + // Not necessary in browser-chrome tests, but monitorConsole gripes > + // if we don't call it. > + SimpleTest.waitForExplicitFinish(); > + > + SimpleTest.monitorConsole(resolve, [{ message: /extensionWindows is undefined/, forbid: true }]); I'd rather just assert that there are no console messages. `SimpleTest.monitorConsole(resolve, [], true);`

Unfortunately there are two unrelated errors which make the test to fail if I do so: - JavaScript Error: "TelemetryStopwatch: requesting elapsed time for nonexisting stopwatch. ...", related to resource://gre/modules/TelemetryStopwatch.jsm - JavaScript Error: "[Exception... "Component returned failure code: ...", related to chrome://browser/content/browser.js (TabsProgressListener.onStateChange) How about forbid errors based on the "sourceName" as a compromise? let forbiddenMessages = [{ forbid: true, isScriptError: true, sourceName: /ExtensionContent\.jsm/, }]; ... SimpleTest.monitorConsole(resolve, forbiddenMessages);

Updates on the previous patch version: - fixed eslint warnings and errors - applied the described tweak (forbid console errors based on the sourceName instead of the error message) Re-applied the previous r+ flag.

Minor tweak on the previous version of the attached patch: - removed the test case used to demonstrate the issue (a new test case to properly test that "the extension context is completely cleaned up when the iframe is removed" will be added in a follow up of this issue)

Comment on attachment 8713106 [details] [diff] [review] 0001-fix-exception-webextension-iframe-destroy.patch Approval Request Comment [Feature/regressing bug #]: 1214658 [User impact if declined]: minor memory leak and errors appearing in the error console. [Describe test coverage new/current, TreeHerder]: the tear-down code isn't adequately tested, more comprehensive tests will be added as a follow-up. [Risks and why]: The patch is composed by a low-risk single-line typo fix. [String/UUID change made/needed]: None.

Comment on attachment 8713106 [details] [diff] [review] 0001-fix-exception-webextension-iframe-destroy.patch Fix for recent regression, ok to uplift to aurora.

Tracking for 46+