I've spun out bug 1367797 to handle the overflowing tab cases, since that's blocked on some tab strip work. So the full list now is: * Window closing * Tab opening (existing tabs must shrink) * Tab closing (existing tabs must grow) * Tab switching (both tabs in view)

Comment on attachment 8871877 [details] Bug 1363505 - Add tab switch reflow test. https://reviewboard.mozilla.org/r/143392/#review148016 ::: browser/base/content/test/performance/browser_tabswitch_reflows.js:48 (Diff revision 1) > + > + let firstSwitchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone"); > + let otherTab = await BrowserTestUtils.openNewForegroundTab(gBrowser); > + await firstSwitchDone; > + > + // Add a reflow observer and open a new tab. the "open a new tab" part of the comment is incorrect.

Comment on attachment 8871875 [details] Bug 1363505 - Add tab squeeze reflow test. https://reviewboard.mozilla.org/r/143388/#review148018 Looks reasonable overall, but there are enough comments that I would like to have another quick look. ::: browser/base/content/test/performance/browser_tabopen_squeeze_reflows.js:59 (Diff revision 1) > + > + await withReflowObserver(async function() { > + let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone"); > + BrowserOpenTab(); > + await BrowserTestUtils.waitForEvent(gBrowser.selectedTab, "transitionend", > + false, e => e.propertyName === "max-width"); nit: the indent feels wrong here. Either use 2 spaces, or align with the first parameter of the waitForEvent call. ::: browser/base/content/test/performance/head.js:187 (Diff revision 1) > +let tabMinWidth = > + parseInt(getComputedStyle(gBrowser.selectedTab, null).minWidth, 10); > + > +const TAB_COUNT_MAX = Math.floor(tabStripWidth / tabMinWidth); > +const TAB_COUNT_FOR_SQUEEZE = TAB_COUNT_MAX - 1; > +const TAB_COUNT_FOR_OVERFLOW = TAB_COUNT_MAX + 1; I don't feel good about running this tab size computation code for all tests when only 2 of them actually need it. Could you change this code to a function called by the 2 tests? TAB_COUNT_FOR_OVERFLOW isn't used (yet, I assume), so it's tempting to say the function should return TAB_COUNT_FOR_SQUEEZE, but otherwise I would just make a function returning the max tab count, and let the individual tests deal with the +/- 1. ::: browser/base/content/test/performance/head.js:201 (Diff revision 1) > + * @param howMany (int) > + * How many about:blank tabs to open. > + */ > +async function createTabs(howMany) { > + let uris = []; > + while (--howMany) { Did you mean howMany-- rather than --howMany? If 'howMany' is 1, you'll currently never enter the loop.

Comment on attachment 8871876 [details] Bug 1363505 - Add tab growth reflow test. https://reviewboard.mozilla.org/r/143390/#review148020 ::: browser/base/content/test/performance/browser_tabclose_grow_reflows.js:56 (Diff revision 1) > + await withReflowObserver(async function() { > + let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone"); > + let tab = gBrowser.tabs[gBrowser.tabs.length - 1]; > + gBrowser.removeTab(tab, { animate: true }); > + await BrowserTestUtils.waitForEvent(tab, "transitionend", > + false, e => e.propertyName === "max-width"); same indent nit here.

Comment on attachment 8871875 [details] Bug 1363505 - Add tab squeeze reflow test. https://reviewboard.mozilla.org/r/143388/#review148386 ::: browser/base/content/test/performance/head.js:194 (Diff revision 2) > + let tabStripRect = gBrowser.tabContainer.mTabstrip.getBoundingClientRect(); > + let tabStripWidth = tabStripRect.width; > + let tabMinWidth = > + parseInt(getComputedStyle(gBrowser.selectedTab, null).minWidth, 10); > + > + let maxTabCount = Math.floor(tabStripWidth / tabMinWidth) - 1; What's this magic - 1 constant here? Should it instead be the current tab count? (if not, I think we need a comment explaining why we take one off).

Comment on attachment 8871875 [details] Bug 1363505 - Add tab squeeze reflow test. https://reviewboard.mozilla.org/r/143388/#review148386 > What's this magic - 1 constant here? Should it instead be the current tab count? (if not, I think we need a comment explaining why we take one off). Yeah, it's the current tab count - I'll just use that instead. Thanks!

Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/afc25136f9ae Add tab squeeze reflow test. r=florian https://hg.mozilla.org/integration/autoland/rev/b6447762b05a Add tab growth reflow test. r=florian https://hg.mozilla.org/integration/autoland/rev/b0c79c15c6ff Add tab switch reflow test. r=florian

Backed out for frequently failing own test browser_tabclose_grow_reflows.js and browser_roundedWindow_newWindow.js: https://hg.mozilla.org/integration/autoland/rev/5e373ab3dab1e6e775ad3776da2d677260783b92 https://hg.mozilla.org/integration/autoland/rev/f7561f92cf88b59b2397eb5d6600fd3f9eb04699 https://hg.mozilla.org/integration/autoland/rev/c8f21748527b42bdb0bb2d84ca25b0410730072a Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b0c79c15c6ffdadedc4d64fa90a409a0316c8a31&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log browser_roundedWindow_newWindow.js | The screen.height has a correct rounded value - 900 == 800: https://treeherder.mozilla.org/logviewer.html#?job_id=103504138&repo=autoland Failure log browser_tabclose_grow_reflows.js: https://treeherder.mozilla.org/logviewer.html#?job_id=103488746&repo=autoland

The browser_roundedWindow_newWindow.js failures were caused by bug 1355764, not this one.

Turns out I also forgot to post a test that I'd written in this series! I also think I've fixed the breakage that was caused by bug 1355764; I have to account for the width of the newtab button when calculating how many tabs can fit into the tab strip without overflowing.

Comment on attachment 8873278 [details] Bug 1363505 - Add reflow test for window closing. https://reviewboard.mozilla.org/r/144732/#review148634 ::: browser/base/content/test/performance/browser_windowclose_reflows.js:14 (Diff revision 1) > + * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers > + * for tips on how to do that. > + */ > +const EXPECTED_REFLOWS = [ > + /** > + * Nothing here! Please don't anything new! Nit: don't add.

Comment on attachment 8873278 [details] Bug 1363505 - Add reflow test for window closing. https://reviewboard.mozilla.org/r/144732/#review148730 ::: browser/base/content/test/performance/browser_windowclose_reflows.js:25 (Diff revision 1) > + * uninterruptible reflows when closing windows. When the > + * window is closed, the test waits until the original window > + * has activated. > + */ > +add_task(function*() { > + // Ensure that this browser window starts focused. Doesn't hurt to check I guess... but isn't this always the case when a bc test starts?

(In reply to Florian Quèze [:florian] [:flo] from comment #26) > Doesn't hurt to check I guess... but isn't this always the case when a bc > test starts? Perhaps, but not necessarily when the test itself repeats. I've been attempting to pre-emptively stamp out intermittents on these tests by running them (and the whole directory) on repeat a bunch, and making sure they pass. This was one of the things I needed to do to stamp out an intermittent.

(In reply to Mike Conley (:mconley) from comment #27) > (In reply to Florian Quèze [:florian] [:flo] from comment #26) > > Doesn't hurt to check I guess... but isn't this always the case when a bc > > test starts? > > Perhaps, but not necessarily when the test itself repeats. I've been > attempting to pre-emptively stamp out intermittents on these tests by > running them (and the whole directory) on repeat a bunch, and making sure > they pass. This was one of the things I needed to do to stamp out an > intermittent. You may want to explain this in a comment to stop the next reader of the file from 'cleaning it up' ;).

Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0c50bc93a3dc Add reflow test for window closing. r=florian https://hg.mozilla.org/integration/autoland/rev/6f6b73f4f305 Add tab squeeze reflow test. r=florian https://hg.mozilla.org/integration/autoland/rev/494590d4307a Add tab growth reflow test. r=florian https://hg.mozilla.org/integration/autoland/rev/c6b6bc96e99c Add tab switch reflow test. r=florian

Backout by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e603b83ca395 Backed out 4 changesets to avoid conflicts with bug 1369140 on inbound.

Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/798f637191ec Add reflow test for window closing. r=florian https://hg.mozilla.org/integration/mozilla-inbound/rev/5e037ab2ee52 Add tab squeeze reflow test. r=florian https://hg.mozilla.org/integration/mozilla-inbound/rev/1ae492c2da06 Add tab growth reflow test. r=florian https://hg.mozilla.org/integration/mozilla-inbound/rev/8bd3380db77f Add tab switch reflow test. r=florian on a CLOSED TREE

https://hg.mozilla.org/mozilla-central/rev/798f637191ec https://hg.mozilla.org/mozilla-central/rev/5e037ab2ee52 https://hg.mozilla.org/mozilla-central/rev/1ae492c2da06 https://hg.mozilla.org/mozilla-central/rev/8bd3380db77f