Re-enable reflow testing of the remote tabs (sync) subview in the hamburger panel in browser_appmenu_reflows.js

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: Gijs, Assigned: mconley)

Tracking

55 Branch
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Because of issues with how we dirty the frame tree after a sync reflow, it isn't currently easily possible to deterministically test the number of reflows incurred when the remote tabs subview is being shown.

Hopefully bug 1363361 will resolve these issues. This bug tracks re-enabling the testing of the subview within the automated test.
Comment hidden (mozreview-request)
(Assignee)

Comment 4

2 years ago
mozreview-review
Comment on attachment 8886229 [details]
Bug 1376822 - Re-enable appmenu reflow tests for sync subview.

https://reviewboard.mozilla.org/r/157014/#review162070

::: browser/base/content/test/performance/browser_appmenu_reflows.js:163
(Diff revision 1)
> +        await BrowserTestUtils.waitForCondition(() => {
> +          return !PanelUI.multiView.instance._viewContainer.hasAttribute("width");
> +        });
>          info("Shown " + PanelUI.multiView.instance._currentSubView.id);
>          // Unfortunately, I can't find a better accessor to the current
>          // subview, so I have to reach the PanelMultiView instance
>          // here.
>          await openSubViewsRecursively(PanelUI.multiView.instance._currentSubView);
>          PanelUI.multiView.goBack();
>          await BrowserTestUtils.waitForEvent(PanelUI.panel, "ViewShown");
> +        await BrowserTestUtils.waitForCondition(() => {
> +          return !PanelUI.multiView.instance._viewContainer.hasAttribute("width");
> +        });

Note that these `waitForCondition`'s are to account for this `setTimeout`:

http://searchfox.org/mozilla-central/rev/cef8389c687203085dc6b52de2fbd0260d7495bf/browser/components/customizableui/PanelMultiView.jsm#632-635
(Reporter)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8886229 [details]
Bug 1376822 - Re-enable appmenu reflow tests for sync subview.

https://reviewboard.mozilla.org/r/157016/#review162802

::: browser/base/content/test/performance/browser_appmenu_reflows.js:163
(Diff revision 1)
> +        await BrowserTestUtils.waitForCondition(() => {
> +          return !PanelUI.multiView.instance._viewContainer.hasAttribute("width");
> +        });

Could you add a comment pointing to bug 1363756 here? AIUI, if/when we fix that bug, we can remove this code again.
Attachment #8886229 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 6

2 years ago
mozreview-review-reply
Comment on attachment 8886229 [details]
Bug 1376822 - Re-enable appmenu reflow tests for sync subview.

https://reviewboard.mozilla.org/r/157016/#review162802

> Could you add a comment pointing to bug 1363756 here? AIUI, if/when we fix that bug, we can remove this code again.

Done, for both cases. Thanks!
Comment hidden (mozreview-request)

Comment 8

2 years ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c2d1e7ed4349
Re-enable appmenu reflow tests for sync subview. r=Gijs

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c2d1e7ed4349
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Assignee: nobody → mconley
You need to log in before you can comment on or make changes to this bug.