1335085 - "WebDriver:SwitchToWindow" has to wait for the "activate" and "focus" events before returning

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

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

While writing the patch for 1124604 I noticed a case where Marionette fails to correctly focus a window/tab. While the current handles are updated correctly, the focus doesn't change. Which means we operate with the tab in the background.

The steps for the test are:
* Open a new tab
* Switch to this tab
* Open a new window
* Switch to the tab in the new window
* Switch back to the second tab in the first window

With the last step the focus doesn't change.

For now I will exclude this specific test from my patch on bug 1124604.

Comment 1

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

This is actually a race because the command doesn't wait until the target window has focus and is activated. My patch on bug 1504756 will add a couple of helpers which will make those checks easier to perform.

Comment 2

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

To remove the race condition which can result in intermittent failures we should get this bug fixed. Also because it has visibility through geckodriver.

Comment 3

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

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

Comment 4

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

Attachment: Bug 1335085 - [marionette] Make use of "WebDriver:NewWindow" in firefox-puppeteer. r=#webdriver

Comment 5

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

Attachment: Bug 1335085 - [marionette] Fix "WebDriver:SwitchToWindow" for tab and window focus switching. r=#webdriver

Selecting a different tab is asynchronous, and as such Marionette has
to wait for the "TabSelect" event to be emitted. Further the chrome
window also needs to be focused.

Depends on D33689

Comment 6

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

Attachment: Bug 1335085 - [marionette] Simplify focus handling for window classes in firefox-puppeteer. r=#webdriver

Depends on D33690

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1387e00703d
[marionette] Make use of "WebDriver:NewWindow" in firefox-puppeteer. r=webdriver-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/252fc1ac18cc
[marionette] Fix "WebDriver:SwitchToWindow" for tab and window focus switching. r=webdriver-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/5dc349bb1690
[marionette] Simplify focus handling for window classes in firefox-puppeteer. r=webdriver-reviewers,ato

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/e1387e00703d
https://hg.mozilla.org/mozilla-central/rev/252fc1ac18cc
https://hg.mozilla.org/mozilla-central/rev/5dc349bb1690

Comment 9

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

I would like to keep it on the radar for the Firefox 68 release, given that it massively improves the stability and reduces the risk of race conditions. If there are no regressions in the next couple of days, lets get it uplifted to beta.

Comment 10

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

I haven't seen any regressions from landing this patch on mozilla-central. To improve the reliability of Marionette and external tools like Selenium using geckodriver I would like to see this test-only patch uplifted to beta for the ESR release. Thanks.

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dde54edc86e1
https://hg.mozilla.org/releases/mozilla-beta/rev/1e1c5ad53cfc
https://hg.mozilla.org/releases/mozilla-beta/rev/992cbc55ecf7