1877469 - When creating and switching tabs wait for the former tab's "document.visibilityState" value set to "hidden"

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

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

There is currently bug 1876604 which prevents the document.visibilityState property (for the previous tab) set to hidden immediately when a tab gets switched, opened, or the window minimized.

We should add more tests to verify the correct behavior.

Comment 1

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

With the patch on bug 1876604 we will get a new preference that can be used to control the unload behavior for switched away tabs. We have to update our code to use this preference that we want to set to 0, but also have to wait returning from the command until the tab's document.visibilityState is set to hidden.

Webdriver tests for browsingContext.activate and browsingContext.create should be created.

For improving stability I would suggest to add this to M10.

Comment 2

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

Attachment: Bug 1877469 - [webdriver-bidi] Await visibilitystatus change for previously selected tab when creating or switching tabs.

Comment 3

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

Attachment: Bug 1877469 - [wdspec] Improve tests for "browsingContext.activate" and "browsingContext.create" for document status.

Depends on D201061

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4aa0d79f3e06
[webdriver-bidi] Await visibilitystatus change for previously selected tab when creating or switching tabs. r=webdriver-reviewers,Sasha
https://hg.mozilla.org/integration/autoland/rev/9bda3647e32c
[wdspec] Improve tests for "browsingContext.activate" and "browsingContext.create" for document status. r=webdriver-reviewers,Sasha

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44494 for changes under testing/web-platform/tests

Comment 6

[Cristina Horotan [:chorotan]]

Backed out 2 changesets (bug 1877469) for causing Wd failures at context_destroyed.py

Backout: https://hg.mozilla.org/integration/autoland/rev/050b73cefd4b86f13890663736a23e6ad8860df9

Failure push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=9bda3647e32cfcb956ea814e9c701a943ff0b390&selectedTaskRun=PU7bwGprSLuAs93P1tKRBQ.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=446659788&repo=autoland&lineNumber=2296
https://treeherder.mozilla.org/logviewer?job_id=446659784&repo=autoland&lineNumber=2075

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 8

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

When I was testing this feature on Android I was actually using fenix but missed to test with the testrunner even in CI. :/

The problem here is really with the testrunner only given that even you switch tabs the one in the background doesn't get the visibilityState="hidden" but stays visible. It works just fine for Fenix and geckoview_example. Sadly wpt uses testrunner only so we would need a solution for that package as well.

Olivia, do you think that this is something easy to add? Olli Petty mentioned to me that there was a similar issue last year with Fenix which got already fixed, but we cannot find this bug / PR where it was landed. Thanks.

Comment 9

[Olivia Hall [:olivia]]

Hi Henrik,

I mentioned this bug to the GeckoView team and it looks like the conversation is happening on the linked bug 1678103. Could you give more details about how you tested? Maybe a few specific tests or test suits you recommend for us to run against? I'm not sure if we need a separate GeckoView bug for the wpt tests or if it is a bigger issue and a part of bug 1678103. It might be worth filing to keep the different issues clear.

We are a bit surprised that the bug could have been solved on certain layers, but not others, because it is a Gecko call and we expect this behavior to be implemented there, so we would have the same behavior across apps. We are trying to narrow down where the difference in behavior is coming from.

Comment 10

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

Sure, just apply the patches from this bug and run as example any of the tests as added by D201062. But you can see on Treeherder that basically all the tests are failing that are creating a new tab or switching the tab.

Please let me know if you have issue with reproducing the underlying problem.

Comment 11

[Olivia Hall [:olivia]]

Thanks!

Quick refresh, and for others who might test, --package-name is the flag to change what device the test runs against?

I have ./mach wpt <path to test> --webdriver-binary <path to geckodriver binary> --package-name="org.mozilla.geckoview.test_runner" in my notes for test runner and package name can be swapped for Fenix or GVE too.

Comment 12

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

Yes, that's correct. And in case you don't have a geckodriver binary and running artifact builds switch into testing/geckodriver and run cargo build. Then the path is target/debug/geckodriver. To also get all the trace logs from Remote Agent use --webdriver-arg=-vv.

Comment 13

[Olivia Hall [:olivia]]

I filed bug 1880376 to make sure this issue doesn't get lost and also so it can be prioritized for investigation, especially where it is unclear where bug 1678103 stands.

Comment 14

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

(In reply to Olivia Hall [:olivia] from comment #13)

I filed bug 1880376 to make sure this issue doesn't get lost and also so it can be prioritized for investigation, especially where it is unclear where bug 1678103 stands.

Thanks and let me actually make it blocking this bug because I'm not able to land my changes without a fix or workaround for Android.

Comment 15

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

I filed the follow-up bug 1884142 for Android. As such we are no longer blocked on bug 1880376.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a8183cf7fab
[webdriver-bidi] Await visibilitystatus change for previously selected tab when creating or switching tabs. r=webdriver-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/e068a92055aa
[wdspec] Improve tests for "browsingContext.activate" and "browsingContext.create" for document status. r=webdriver-reviewers,Sasha

Comment 17

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/3a8183cf7fab
https://hg.mozilla.org/mozilla-central/rev/e068a92055aa

Comment 18

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

Hi Sasha, it looks like the sync bot didn't recognize the wdspec changes and no PR got created. Can you please have a look? Thanks.

Comment 19

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

I've filed an upstream PR to mark the Puppeteer test as expected pass for Firefox: https://github.com/puppeteer/puppeteer/pull/12072

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 21

[Alexandra Borovova [:Sasha]]

The bot missed posting a link for upstream, here it is: https://github.com/web-platform-tests/wpt/pull/44494