1937188 - many /geolocation/ tests fail on win11 24H2 update.

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

I am not able to find a component to file a bug in for /geolocation/ tests in the moz.build file.

Many of these tests fail on the win11 Oct 2024 update that we plan to migrate to in a few weeks (around the new year). I have disabled the failing tests in https://phabricator.services.mozilla.com/D231801.

To run the tests on try, one can:
./mach try fuzzy --no-artifact -q 'test-windows11-64 web-platform-tests !reftest !wdspec' --worker-override='win11-64-2009=gecko-t/win11-64-24h2' --worker-override='win11-64-2009-gpu=gecko-t/win11-64-24h2-gpu' --worker-override='win11-64-2009-source=gecko-t/win11-64-24h2-source' --worker-override='win11-64-2009-ssd=gecko-t/win11-64-24h2-ssd' --worker-override='win11-64-2009-ssd-gpu=gecko-t/win11-64-24h2-ssd-gpu'

I am not sure if on the command line you can reduce by giving a path like we do for other test harnesses.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

for reference the other geolocation tests in wpt and mochitest all appear to work.

here is a link to try. and here is a snippet from a log file:

[task 2024-12-06T21:12:13.400Z] 21:12:13     INFO - TEST-START | /geolocation/PositionOptions.https.html
[task 2024-12-06T21:12:13.681Z] 21:12:13     INFO - PID 6684 | [Child 3624, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, NS_ERROR_INVALID_ARG) failed with result 0x80520012 (NS_ERROR_FILE_NOT_FOUND): file /builds/worker/checkouts/gecko/intl/l10n/L10nRegistry.cpp:385
[task 2024-12-06T21:12:14.011Z] 21:12:14     INFO - PID 6684 | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to C:\Users\task_173351856241718\AppData\Local\Temp\tmp4vmy_iar\runtests_leaks_1112_tab_pid10716.log
[task 2024-12-06T21:12:14.205Z] 21:12:14     INFO - PID 6684 | [Child 10716, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, NS_ERROR_INVALID_ARG) failed with result 0x80520012 (NS_ERROR_FILE_NOT_FOUND): file /builds/worker/checkouts/gecko/intl/l10n/L10nRegistry.cpp:385
[task 2024-12-06T21:12:14.826Z] 21:12:14     INFO - PID 6684 | [Parent 10864, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /builds/worker/checkouts/gecko/chrome/nsChromeRegistry.cpp:182
[task 2024-12-06T21:12:14.827Z] 21:12:14     INFO - PID 6684 | [Parent 10864, Main Thread] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/chrome/nsChromeProtocolHandler.cpp:73
[task 2024-12-06T21:12:14.988Z] 21:12:14     INFO - PID 6684 | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to C:\Users\task_173351856241718\AppData\Local\Temp\tmp4vmy_iar\runtests_leaks_1112_tab_pid9792.log
[task 2024-12-06T21:12:15.010Z] 21:12:15     INFO - PID 6684 | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to C:\Users\task_173351856241718\AppData\Local\Temp\tmp4vmy_iar\runtests_leaks_1112_tab_pid8768.log
[task 2024-12-06T21:12:15.051Z] 21:12:15     INFO - PID 6684 | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2024-12-06T21:12:15.052Z] 21:12:15     INFO - PID 6684 | [Child 9792, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/checkouts/gecko/dom/media/CubebUtils.cpp:467
[task 2024-12-06T21:12:15.056Z] 21:12:15     INFO - PID 6684 | [Parent 10864, Main Thread] WARNING: Not resolving response 'PContent::Reply_GetModulesTrust': actor is dead: file /builds/worker/checkouts/gecko/ipc/glue/ProtocolUtils.cpp:861
[task 2024-12-06T21:12:15.128Z] 21:12:15     INFO - TEST-UNEXPECTED-ERROR | /geolocation/PositionOptions.https.html | expected OK
[task 2024-12-06T21:12:15.128Z] 21:12:15     INFO - TEST-INFO took 1728ms
[task 2024-12-06T21:12:15.132Z] 21:12:15     INFO - PID 6684 | 1733519535131	Marionette	INFO	Stopped listening on port 51994
[task 2024-12-06T21:12:15.903Z] 21:12:15     INFO - PID 6684 | [Child 7148, Main Thread] WARNING: DispatchEvent called on non-current inner window, dropping. Please check the window in the caller instead.: file /builds/worker/checkouts/gecko/dom/base/nsGlobalWindowInner.cpp:4152
[task 2024-12-06T21:12:16.013Z] 21:12:16     INFO - PID 6684 | [Child 10716, Main Thread] WARNING: NS_ENSURE_TRUE(child) failed: file /builds/worker/checkouts/gecko/dom/base/nsContentPermissionHelper.cpp:268
[task 2024-12-06T21:12:16.044Z] 21:12:16     INFO - PID 6684 | [Parent 10864, Socket Thread] WARNING: cannot post event if not initialized: file /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpConnectionMgr.cpp:240
[task 2024-12-06T21:12:16.046Z] 21:12:16     INFO - PID 6684 | [Parent 10864, Socket Thread] WARNING: cannot post event if not initialized: file /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpConnectionMgr.cpp:240
[task 2024-12-06T21:12:16.063Z] 21:12:16     INFO - PID 6684 | [Child 3624, IPC I/O Child] WARNING: [44DB673D7FA0DA33.FB4B8CF07B51D576]: GetUserData call for port 'A0F54579D296DC2D.F6DBB148A6A920B7' failed: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:424
[task 2024-12-06T21:12:16.095Z] 21:12:16     INFO - PID 6684 | [Parent 10864, Main Thread] WARNING: Not resolving response 'PContent::Reply_GetModulesTrust': actor is dead: file /builds/worker/checkouts/gecko/ipc/glue/ProtocolUtils.cpp:861
[task 2024-12-06T21:12:16.095Z] 21:12:16     INFO - PID 6684 | [Parent 10864, Main Thread] WARNING: Not resolving response 'PContent::Reply_GetModulesTrust': actor is dead: file /builds/worker/checkouts/gecko/ipc/glue/ProtocolUtils.cpp:861
[task 2024-12-06T21:12:16.201Z] 21:12:16     INFO - Browser exited with return code 572

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

Per https://treeherder.mozilla.org/logviewer?job_id=485789470&repo=try&lineNumber=6653 there's a lot of logs where the window just closes immediately after start:

[task 2024-12-06T22:52:54.434Z] 22:52:54     INFO - TEST-START | /geolocation/non-fully-active.https.html
[task 2024-12-06T22:52:54.434Z] 22:52:54     INFO - Closing window 3975e8f5-0cec-4d1d-a9cd-0b29c8297fa2
[task 2024-12-06T22:52:55.054Z] 22:52:55     INFO - TEST-UNEXPECTED-ERROR | /geolocation/non-fully-active.https.html | expected OK
[task 2024-12-06T22:52:55.054Z] 22:52:55     INFO - TEST-INFO took 627ms

Given this is 24H2 specific, perhaps this is related to what's described in bug 1900225? I don't see such error on local 24H2 though. David, any idea how this could happen?

Comment 3

[David Parks [:handyman]]

My first guess is that the tests are using system geolocation, and the permission isn't granted so bug 1900225 is opening system prefs and waiting for the user to grant permission there -- none of which is expected by the test harness. I haven't totally confirmed that this is the cause of this failure but I do see the test opening system prefs locally when I have geolocation turned off. That setting may be why the failure didn't show up in comment 2. System geolocation behavior is turned off by prefs in mochitests (this has always been the case). I'm guessing those prefs need to be set for wpt tests as well. Before bug 1900225 and without system geolocation permission on try, this would have previously been trying to use the MLS fallback (IDK what that would look like at present given MLS retirement). The test prefs are probably a better solution anyway.

FYI, with system geolocation permission granted, the test runs all cases (unlike before) but I see two failures. There may be more to this. I'll keep poking at it.

Comment 4

[David Parks [:handyman]]

Setting geo.prompt.open_system_prefs to false -- which skips opening system prefs if geolocation isn't on, and which is not set to false in other tests -- seems to work for geolocation/PositionOptions.https.html. I assume it will work for the others as well. It makes sense to set for tests. I'll see if I can track down the wpt tests that need it, although it might be better if it were set for all of them. The dialog is not skipped when using the other geolocation test prefs (at least not that I noticed). I'm not sure why the dialog is not breaking the other geolocation tests that request position.

Additionally, so far, the two failures that I mentioned in comment 3 also happen after setting the pref to false. They seem to be legitimate failures that aren't properly reported by the wpt harness. A trimmed down part of the log:

 1:02.29 TEST_START: /geolocation/PositionOptions.https.html
 1:02.52 INFO Setting pref geo.prompt.open_system_prefs to false
[...]
 1:17.84 TEST_END: Test OK. Subtests passed 4/6. Unexpected 0
 1:17.84 INFO Pausing until the browser exits
[...]
web-platform-test
~~~~~~~~~~~~~~~~~
Ran 7 checks (6 subtests, 1 tests)
Expected results: 7
Unexpected results: 0
OK

Somehow 4/6 subtest passes became 6/6 passes. The log doesn't contain anything about the individual checks but the browser window does:

Pass	Set timeout and maximumAge to 0, check that timeout error raised (getCurrentPosition)	
Asserts run | Pass	| assert_equals(3, 3) | /geolocation/PositionOptions.https.html:27:18

Fail	Set timeout and maximumAge to 0, check that timeout error raised (watchPosition)	promise_test: Unhandled rejection with value: object "[object GeolocationPosition]"
Asserts run | No asserts ran

Pass	Check that a negative timeout and maxAge values are clamped to 0 (getCurrentPosition)	
Asserts run | Pass	 | assert_equals(3, 3) |  /geolocation/PositionOptions.https.html:50:18

Fail	Check that a negative timeout and maxAge values are clamped to 0 (watchPosition)	promise_test: Unhandled rejection with value: object "[object GeolocationPosition]"
Asserts run | No asserts ran

Pass	Call getCurrentPosition with wrong type for enableHighAccuracy. No exception expected.	
Asserts run | No asserts ran

Pass	Call watchPosition with wrong type for enableHighAccuracy. No exception expected.	
Asserts run | No asserts ran

This suggests an additional issue with watchPosition.

Comment 5

[David Parks [:handyman]]

(In reply to David Parks [:handyman] from comment #4)

Fail	Set timeout and maximumAge to 0, check that timeout error raised (watchPosition)	promise_test: Unhandled rejection with value: object "[object GeolocationPosition]"
Asserts run | No asserts ran
[...]
Fail	Check that a negative timeout and maxAge values are clamped to 0 (watchPosition)	promise_test: Unhandled rejection with value: object "[object GeolocationPosition]"
Asserts run | No asserts ran

Derp -- these failures are known: https://searchfox.org/mozilla-central/rev/66aaf96f955d9bdc4bdb087a5075d5fbff2b9c8d/testing/web-platform/meta/geolocation/PositionOptions.https.html.ini#6-10

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

so we just need a pref and we are back to the existing known failures?

Comment 7

[David Parks [:handyman]]

Nope. This is a bit stranger than that.

Adding geo.prompt.open_system_prefs=false corrects most of the tests but runs into failures in the enabled_by_permission_policy* tests. The problem seems to be that those tests post the normal "grant geolocation permission to this page" dialog, which then hangs the test similarly to how it was hanging w/o geo.prompt.open_system_prefs=false (but it is a different dialog). geo.prompt.testing=true blocks that dialog, and adding that seems to work for this second hang. However, we weren't setting it before, so I'm guessing that the permission policy setting in these tests was preventing Firefox from showing that prompt before bug 1900225, but I have not confirmed that yet. If that's the case then we have unexpected behavior.

wpt tests hang for me when run from m-c locally -- I've been using m-c from about 2 weeks ago. Unfortunately, I get the same hang when I try to run them from m-c from before bug 1900225. So I'll need another way to find out what the test was doing before.

Comment 8

[David Parks [:handyman]]

I was able to run the wpt test enabled-by-permission-policy-attribute-redirect-on-load.https.sub.html locally, using m-c from before bug 1900225 landed, by forcing wpt to rebuild its manifest once. (Command line args didn't work for this but inserting _kwargs["rebuild"]=True here did. Additionally, the system clock has to be set before Sept 11 2024 to avoid an invalid cert.) With that, I see the same behavior as for the recent m-c with geo.prompt.open_system_prefs=false -- it presents the permissions dialog and then times-out waiting for a response. The test results are a "timeout" and a "didn't run".

@saschanaz, bug 1902720 removed geo.prompt.testing=true when you added the mock location server. It also fixed a bunch of expected timeouts -- those timeouts resemble the ones I'm seeing now (even before the Windows UI work in bug 1900225). So, before bug 1902720, we had those prefs and were timing out anyway -- the exact opposite of what I would have expected. Then, you removed those prefs and apparently the timeouts went away -- again, the exact opposite of what I'd have expected given my results here. Do you have any insight into this? I would just re-add the two prefs from comment 7 and be done with it but the history has me doubting my conclusions. The real question I have is whether the policy stuff is supposed to test that the permission dialog isn't shown (I'm thinking no?) or if it is really just intended to be used to deny geolocation access (so you still get the dialog even with an explicit allow permission)?

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

geo.prompt.testing is gone because there's now set_permission() webdriver function that can grant the permission. With that the prompt should not open unless there's no permission. Perhaps there are some tests that intentionally don't grant permission which opens the system popup and breaks things?

Comment 10

[David Parks [:handyman]]

Thanks, that's what I needed. So the enable tests are supposed to be checking that setting the policy overrides the dialog. Now this makes even less sense :-Z

Comment 11

[David Parks [:handyman]]

Of the enabled-by-* geolocation tests, in comment 8 I had managed to choose the only one that isn't fixed by setting geo.prompt.open_system_prefs=false -- the others already properly obey the policy and don't show the dialog. I think that test may not be setting the policy right -- that is also the one that failed for pre-bug 1900225 m-c.

Comment 12

[David Parks [:handyman]]

Of the enabled-by-* geolocation tests, in comment 8 I had managed to choose the only one that isn't fixed by setting
geo.prompt.open_system_prefs=false
That was wrong. They still had geo.prompt.testing.

I may have found the main issue and I should be able to draft a patch for it. There are few problems and some outstanding mysteries. First, geo.prompt.open_system_prefs=false should of course be set for all tests -- that was done in bug 1900225 for mochitests but not for wpt. That's the easy part. After that, the enabled-by-permission-policy-attribute* tests still timeout due to a dialog, but this time its the usual Firefox geolocation permission one. The issue here seems to be that a commit went into wpt in June that changed the test -- the checkin comment suggests it's going to move the place where the top-level frame is granted permission (these are iframe inherited permission tests -- the "attribute" from the test filename is the "allow" attribute on the iframe). Instead, it just deletes it. So the top level didn't have permission so the iframes had nothing to inherit. Adding the set_permission call to the test files as suggested does work... although there is ALSO an intermittent issue, so it seems to only pass about half the time for me locally.

I am utterly baffled at how we are passing these in CI, especially since wpt.fyi seems to believe that no browsers have been passing these tests for many months. (Note that that link is for results from last August.) Worse, I saw the same pattern for a bunch of other wpt permissions tests, such as the battery API. All of those tests also fail for me locally.

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

wpt.fyi fails because it doesn't have the same prefs that we do in our CI, it only has the default prefs. It's unfortunate but not surprising.

Comment 14

[David Parks [:handyman]]

I see what's happening now -- it's (a bug in) the test framework, keeping life interesting. comment 12 explains the failure and the fix (patch coming). @saschanaz clarified the wpt.fyi difference (thanks!) but that wasn't related to the local failures here, or the apparently missing permission setting. But those are explained by this try run, which is just m-c but is only running the tests that are missing the permission. They fail. The reason they pass in CI is that those tests are run after disabled-by-permissions-policy.https.sub.html, which grants the permission. So the permission granting lingers past the test. That's probably also related to the other wpt permission tests that fail locally but not "in CI" (like the battery API mentioned in comment 12).

Comment 15

[David Parks [:handyman]]

There is also a test-verify failure on these tests, after the fix, even in CI.

Comment 16

[David Parks [:handyman]]

Attachment: Bug 1937188: Set geo.prompt.open_system_prefs=false in wpt tests r=saschanaz!,jgraham!

geo.prompt.open_system_prefs keeps the tests from opening system preferences
for the user and waiting for them to turn on location services (which obviously
will never happen). This is currently only relevant on Windows.

Comment 17

[David Parks [:handyman]]

Attachment: Bug 1937188: Restore geolocation permission for wpt geolocation policy tests and make attribute tests more similar r=saschanaz!,jgraham!

Permission was removed from permissions-policy-geolocation.html upstream but not
re-added to the individual tests. These two tests need it. They were
incorrectly passing in CI due to the tests that run before them
(disabled-by-permissions-policy.https.sub.html) granting the permission --
they would fail when run individually.

This also does some cosmetic cleanup to make it clear that the tests are
very similar. This includes renaming the tests from "Geolocation API"
to "Geolocation", as the name was changed in Bug 1902628 [wpt PR 46750].

Additionally, enabled-by-permission-policy-attribute fails in test-verify mode
as an async_test -- the async setup function is not compatible with
that. enabled-by-permission-policy-attribute-redirect-on-load
is the same test but with a redirect -- it is a promise_test. This
switches enabled-by-permission-policy-attribute to work as a promise_test,
which fixes the test-verify issue.

Depends on D232771

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jgraham, could you have a look please?

For more information, please visit BugBot documentation.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68958ce94869
Set geo.prompt.open_system_prefs=false in wpt tests r=saschanaz,jgraham
https://hg.mozilla.org/integration/autoland/rev/2ad450615824
Restore geolocation permission for wpt geolocation policy tests and make attribute tests more similar r=saschanaz,jgraham

Comment 20

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

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

Comment 21

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/68958ce94869
https://hg.mozilla.org/mozilla-central/rev/2ad450615824

Comment 22

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

Upstream PR merged by moz-wptsync-bot