Browser action and page action popups, and inline options UI browsers, are automatically sized to fit their content. The current size checking and mutation observer code relies on having access to the content window, which means that it needs to be moved to a frame script, and notify the parent of size changes via the message manager or IPDL.

Comment on attachment 8802376 [details] Bug 1287209: Handle popup and options browser resizing using a frame script. https://reviewboard.mozilla.org/r/86788/#review85972 Mostly nits. Meta-comment: for a reasonbly complex patch like this (complex to somebody like me who doesn't already know the underlying code as well as you do anyway), unrelated refactorings and cleanups make reading and understanding the patch harder than it needs to be, please put those in separate commits. ::: browser/components/extensions/ext-utils.js:60 (Diff revision 2) > }); > } > }); > } > > XPCOMUtils.defineLazyGetter(this, "stylesheets", () => { nit: now that this isn't actually doing anything very expensive, the lazy getter seems like overkill ::: browser/components/extensions/ext-utils.js:146 (Diff revision 2) > }); > } > > destroyBrowser(browser) { > - browser.removeEventListener("DOMWindowCreated", this, true); > - browser.removeEventListener("load", this, true); > + let mm = browser.messageManager; > + if (mm) { under what circumstances do we not have an mm here? ::: browser/components/extensions/ext-utils.js:300 (Diff revision 2) > + } > + > + let event = new this.window.CustomEvent("WebExtPopupResized", {detail}); > + this.browser.dispatchEvent(event); > + > + this._resolveContentReady(); you're also doing this immediately when you get the resize event (line 197), can this one go? ::: toolkit/components/extensions/ExtensionUtils.jsm:1141 (Diff revision 2) > }); > }); > } > > /** > + * Returns a Promise which resolves the given event is dispatched to the nit: ...resolves *when* the given event... ::: toolkit/components/extensions/ExtensionUtils.jsm:1146 (Diff revision 2) > + * observer's subject and data, should return true if this is the > + * expected event, false otherwise. looks like stray lines left over from a bungled copy/paste maybe?

Comment on attachment 8802376 [details] Bug 1287209: Handle popup and options browser resizing using a frame script. https://reviewboard.mozilla.org/r/86788/#review85972 > under what circumstances do we not have an mm here? When the browser has already been removed from the document because the popup was closed externally. > you're also doing this immediately when you get the resize event (line 197), can this one go? Yeah. > looks like stray lines left over from a bungled copy/paste maybe? Yeah. No idea how that happened.

Comment on attachment 8802754 [details] Bug 1287209: Make popup tests compatible with remote browsers. https://reviewboard.mozilla.org/r/87052/#review86096 ::: browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js:120 (Diff revision 1) > > yield extension.startup(); > > + /* eslint-disable mozilla/no-cpows-in-tests */ > + > + function alter(browser, task) { you do a bunch of stuff that follows (roughly) this same pattern in the next test as well, maybe move this to head.js? ::: browser/components/extensions/test/browser/head.js:113 (Diff revision 1) > + // Debouncing code makes this a bit racy. > + // Try to skip the first, early resize, and catch the resize event we're > + // looking for, but don't wait longer than a few seconds. I don't understand the issue here? With the current debouncing implementation, I think you should always see the delayed event unless the window goes away but you wouldnt be using this on a window that can disappear... ?

Comment on attachment 8802754 [details] Bug 1287209: Make popup tests compatible with remote browsers. https://reviewboard.mozilla.org/r/87052/#review86096 > I don't understand the issue here? With the current debouncing implementation, I think you should always see the delayed event unless the window goes away but you wouldnt be using this on a window that can disappear... ? Oh, yeah, this comment isn't relevant anymore. It applied to the old implementation.

https://hg.mozilla.org/integration/mozilla-inbound/rev/68806639c03110bfb67dd6c705a5441a917afcd6 Bug 1287209: Handle popup and options browser resizing using a frame script. r=aswan https://hg.mozilla.org/integration/mozilla-inbound/rev/642ee7eb70bbe1d512a06cf6478910bd2d0db273 Bug 1287209: Make popup tests compatible with remote browsers. r=aswan

Backed out for failing browser-chrome test browser_ext_popup_corners.js: Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=642ee7eb70bbe1d512a06cf6478910bd2d0db273 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=37960294&repo=mozilla-inbound [task 2016-10-20T05:55:35.757362Z] 05:55:35 INFO - 425 INFO TEST-PASS | browser/components/extensions/test/browser/browser_ext_popup_corners.js | Panel and body borderBottomRightRadius should be the same - [task 2016-10-20T05:55:35.759704Z] 05:55:35 INFO - 426 INFO TEST-PASS | browser/components/extensions/test/browser/browser_ext_popup_corners.js | Panel and view borderBottomLeftRadius should be the same - [task 2016-10-20T05:55:35.766000Z] 05:55:35 INFO - 427 INFO TEST-PASS | browser/components/extensions/test/browser/browser_ext_popup_corners.js | Panel and body borderBottomLeftRadius should be the same - [task 2016-10-20T05:55:35.767867Z] 05:55:35 INFO - 428 INFO Test menu panel browserAction popup [task 2016-10-20T05:55:35.770642Z] 05:55:35 INFO - 429 INFO Console message: [JavaScript Error: "Error: Popup destroyed" {file: "chrome://browser/content/ext-utils.js" line: 124}] [task 2016-10-20T05:55:35.772532Z] 05:55:35 INFO - 430 INFO TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_popup_corners.js | Uncaught exception - TypeError: Argument 1 of Window.getComputedStyle is not an object. [task 2016-10-20T05:55:35.774303Z] 05:55:35 INFO - 431 INFO Leaving test bound testPopupBorderRadius [task 2016-10-20T05:55:35.776693Z] 05:55:35 INFO - Not taking screenshot here: see the one that was previously logged [task 2016-10-20T05:55:35.778614Z] 05:55:35 INFO - 432 INFO TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_popup_corners.js | Extension left running at test shutdown - [task 2016-10-20T05:55:35.783899Z] 05:55:35 INFO - Stack trace: [task 2016-10-20T05:55:35.785679Z] 05:55:35 INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:107 [task 2016-10-20T05:55:35.787667Z] 05:55:35 INFO - chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:435 [task 2016-10-20T05:55:35.789425Z] 05:55:35 INFO - testScope/test_finish/<@chrome://mochikit/content/browser-test.js:1004:11 [task 2016-10-20T05:55:35.791121Z] 05:55:35 INFO - testScope/test_executeSoon/<.run@chrome://mochikit/content/browser-test.js:935:9

Backout by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e964a086a0 Backed out changeset 642ee7eb70bb https://hg.mozilla.org/integration/mozilla-inbound/rev/787c5e10eae7 Backed out changeset 68806639c031 for failing browser-chrome test browser_ext_popup_corners.js. r=backout

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ea8213113d73fcbdd5160397db9ed4fbe295ee0 Bug 1287209: Handle popup and options browser resizing using a frame script. r=aswan https://hg.mozilla.org/integration/mozilla-inbound/rev/a6787db4d131dce2eabe0fab90c0e03e531190b5 Bug 1287209: Make popup tests compatible with remote browsers. r=aswan

Test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=38002804&repo=mozilla-inbound Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/db5272d59d98

I hate that test...

https://hg.mozilla.org/integration/mozilla-inbound/rev/cc7503f09572bc49cd2f86184d0d3abc810351c3 Bug 1287209: Handle popup and options browser resizing using a frame script. r=aswan https://hg.mozilla.org/integration/mozilla-inbound/rev/50dcca551b63ee6e9b07207208d6cb1c239467af Bug 1287209: Make popup tests compatible with remote browsers. r=aswan

Test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=38029041&repo=mozilla-inbound Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/1171f3ae792b91d7ca49ef0907b251e381f1df56

Also https://treeherder.mozilla.org/logviewer.html#?job_id=38025730&repo=mozilla-inbound

Wups, no, browser_ext_popup_corners.js is coming from somewhere below this.

Oh, no it's not, it's coming from the previous time this landed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/442e281215648e53bfc55ba6739dc529aefe7607 Bug 1287209: Handle popup and options browser resizing using a frame script. r=aswan https://hg.mozilla.org/integration/mozilla-inbound/rev/18a51ecab347bd7bfa89c63903b34732c24fcb1a Bug 1287209: Make popup tests compatible with remote browsers. r=aswan

Keeping fingers crossed... So far clean try runs seem to have no correlation with what happens on inbound...

https://hg.mozilla.org/mozilla-central/rev/442e28121564 https://hg.mozilla.org/mozilla-central/rev/18a51ecab347