1700755 - waitForWindow() in "WebDriver:NewSession" hangs forever if no window is found

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

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

This is a regression from bug 1696878 in Firefox 88. Due to the usage of a plain Promise the call to waitForWindow() never causes the Promise to resolve when there is no browser window. This is a problem in some GeckoView scenarios:

(In reply to Andrew Creskey [:acreskey] [he/him] from bug 1700290 comment #7)

Maybe this is not possible, but could we still launch and not block on waitForWindow()?

So that we could use geckodriver for other scenarios that don't involve a browser window.

Currently the Fenix MAIN tests do this -- they launch Fenix via geckodriver and then ignore the Marionette exception because Fenix is launched and that's all we need.

As such we should make use of a TimedPromise and probably fail the WebDriver:NewSession command. I would have to check that this doesn't cause a force stop of the application.

Comment 1

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

Attachment: Bug 1700755 - [marionette] Use TimedPromise when waiting for initial application windows.

Comment 2

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

Andrew, would the patch as attached work for you? If not you cannot use geckodriver to just start Fenix in a mode that is not supported. Code in the new session command requires a valid application window.

Comment 3

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

So without the patch on bug 1696878 we failed with an error like:

1616662638227	Marionette	DEBUG	1 -> [0,1,"WebDriver:NewSession",{"strictFileInteractability":true}]
1616662638228	Marionette	DEBUG	1 <- [1,1,{"error":"unknown error","message":"TypeError: can't access property \"register\", this.curBrowser is null","stacktrace" ... t@chrome://marionette/content/server.js:238:9\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20\n"},null]

This is actually problematic because per WebDriver spec the command should only throw a SessionNotCreated error. As such the changes in my proposed patch make sense.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ad523c9e6e0
[marionette] Use TimedPromise when waiting for initial application windows. r=marionette-reviewers,jdescottes

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5ad523c9e6e0

Comment 6

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

Comment on attachment 9211360 [details]
Bug 1700755 - [marionette] Use TimedPromise when waiting for initial application windows.

Beta/Release Uplift Approval Request

• User impact if declined: Tests for WebDriver / Marionette will hang during startup of Firefox for Android if a wrong activity has been chosen and no window of a supported window type is open.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Uses a timed promise to break out of searching for a supported window type after 1.5s (4.5s debug build). Reverts to the pre-bug 1696878 behavior including a spec conformant error message.
• String changes made/needed:

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9211360 [details]
Bug 1700755 - [marionette] Use TimedPromise when waiting for initial application windows.

Approved for 88.0b5.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d925b48864b0