Closed Bug 1376822 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Toolbars and Customization, enhancement)

55 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: Gijs, Assigned: mconley)

References

Details

Attachments

(1 file)

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 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
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+
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!
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c2d1e7ed4349
Re-enable appmenu reflow tests for sync subview. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/c2d1e7ed4349
Status: NEW → RESOLVED
Closed: 3 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.