1400226 - Wait for visibilitychange event in WebDriver:MinimizeWindow

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

[Andreas Tolfsen ❲:ato❳]

The sizemodechange event is not strongly connected to the
visibilitychange event that the WPT minimize_window.py test is
now using to ascertain whether the window has been successfully
iconified.

Because Marionette uses the sizemodechange event it is
causing intermittents such as https://bugzil.la/1397007.  You
can also read a lengthy summary I did on the problem in
https://bugzil.la/1397007#c11.

The fix for the problem is to wait for the visibilitychange DOM
event content.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Wait for visibilitychange event on window minimize/restore. r?whimboo

The sizemodechange event is not strongly connected to the
visibilitychange event that the WPT minimize_window.py test is now
using to ascertain whether the window has been successfully iconified.

Because Marionette uses the sizemodechange event it is causing
intermittents such as https://bugzil.la/1397007.  You can also read a
lengthy summary I did on the problem in https://bugzil.la/1397007#c11.

The fix for the problem is to wait for the visibilitychange DOM
event content.

MozReview-Commit-ID: B6i33Ee5iMC

Comment 2

[Andreas Tolfsen ❲:ato❳]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f83a6d597a90fd32006e3d0370b675944c27ee3f

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8910282 [details] [diff] [review]
Wait for visibilitychange event on window minimize/restore. r?whimboo

Review of attachment 8910282 [details] [diff] [review]:
-----------------------------------------------------------------

A mozreview request would be nicer, maybe you plan one now that the dependency has been fixed. Until then a question I would like see answered first. Thanks.

::: testing/marionette/driver.js
@@ +3720,2 @@
>   */
> +async function restoreWindow(chromeWindow, contentWindow) {

Why can't we add this method to the BrowserContext class, which has both the information for chrome and content window available, and IMO would be a better fit for this feature.

Comment 4

[Andreas Tolfsen ❲:ato❳]

(In reply to Henrik Skupin (:whimboo) from comment #3)
> ::: testing/marionette/driver.js
> @@ +3720,2 @@
> >   */
> > +async function restoreWindow(chromeWindow, contentWindow) {
> 
> Why can't we add this method to the BrowserContext class, which
> has both the information for chrome and content window available,
> and IMO would be a better fit for this feature.

This will get moved to a class similar in nature to browser.Context
with http://bugzil.la/marionette-window-tracking, but considering
none of the other window manipulation helpers are there, it makes
sense to leave it in its current place for now.

Comment 5

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8910282 [details] [diff] [review]
Wait for visibilitychange event on window minimize/restore. r?whimboo

Can we get this patch landed soon so that we can resolve
https://bugzil.la/1397007?

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6470a12114de
Wait for visibilitychange event on window minimize/restore. r=whimboo

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6470a12114de