Closed Bug 1367797 Opened 3 years ago Closed 2 years ago

Write reflow tests for tab opening and closing when the tab strip is overflowed

Categories

(Firefox :: General, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-performance])

Attachments

(1 file)

Similar to bug 1363505, but blocked until we can make the tab strip cause reflows more deterministically (instead of via rAF).

Specific cases to be tested:
* Tab opening (tab strip switches to overflow)
* Tab opening (in overflow state)
* Tab closing (in overflow state)
* Tab closing (tab strip underflows)
* Tab switching (selected tab is scrolled out of view)
Whiteboard: [photon-performance] → [photon-performance] [triage]
Depends on: 1368208
Flags: qe-verify-
Priority: P2 → P3
Whiteboard: [photon-performance] [triage] → [reserve-photon-performance]
Flags: needinfo?(mconley)
Finally coming back around to this.

What we've already got:

* Tab opening (tab strip switches to overflow)
* Tab closing (tab strip underflows)

What we still need:

* Tab opening (in overflow state)
* Tab closing (in overflow state)
* Tab switching (selected tab is scrolled out of view)
Flags: needinfo?(mconley)
Comment on attachment 8930620 [details]
Bug 1367797 - Add reflow tests for adding, removing and switching tabs while the tab strip is overflowed.

https://reviewboard.mozilla.org/r/201380/#review210210

Looks good, thanks.

I wonder if there would be any point in testing tab closing without animation.

Also, would it be useful to test closing several tabs at once? ie. should we put a reflow observer around the removeAllButFirstTab call? And if so should we do it both in an overflow and a non-overflow case?

Should we check that no reflow happens when closing a window? (a window with one tab / a window with multiple tabs)

(just giving food for thoughts here, none of these new cases is required for landing)

::: browser/base/content/test/performance/browser_tabstrip_overflow_underflow_reflows.js:57
(Diff revision 1)
>  
> -  await withReflowObserver(async function() {
> +  Assert.ok(gBrowser.tabContainer.hasAttribute("overflow"),
> +            "Tabs should now be overflowed.");
> +
> +  // Now test that opening and closing a tab while overflowed doesn't cause
> +  // us to overflow.

s/overflow/reflow/

::: browser/base/content/test/performance/browser_tabstrip_overflow_underflow_reflows.js:88
(Diff revision 1)
> +            "First tab should be scrolled out of view.");
> +
> +  // Now switch to the first tab. We shouldn't flush layout at all.
> +  await withReflowObserver(async function(dirtyFrame) {
> +    let switchDone = BrowserTestUtils.waitForEvent(window, "TabSwitchDone");
> +    await BrowserTestUtils.switchTab(gBrowser, firstTab);

BrowserTestUtils.switchTab returns a promise that waits for the TabSwitchDone event already.
Attachment #8930620 - Flags: review?(florian) → review+
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cfa04fd0f319
Add reflow tests for adding, removing and switching tabs while the tab strip is overflowed. r=florian
Comment on attachment 8930620 [details]
Bug 1367797 - Add reflow tests for adding, removing and switching tabs while the tab strip is overflowed.

https://reviewboard.mozilla.org/r/201380/#review210210

Good ideas! I've filed bug 1424377 for them.
Made some minor adjustments to make the test more resilient across different display sizes.
Flags: needinfo?(mconley)
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c1389acf296a
Add reflow tests for adding, removing and switching tabs while the tab strip is overflowed. r=florian
https://hg.mozilla.org/mozilla-central/rev/c1389acf296a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Assignee: nobody → mconley
You need to log in before you can comment on or make changes to this bug.