1715482 - Geolocation should gracefully handle Documents that are not fully active

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect, P3)

Description

[marcos]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1.1 Safari/605.1.15

Steps to reproduce:

When a document becomes no longer fully active (i.e., iframe is removed), a reference to Geolocation API shouldn't throw when its methods are called.

See http://marcoscaceres.github.io/playground/geo_test.html
And spec bug: https://github.com/w3c/geolocation-api/issues/78

Actual results:

It throws an ERROR_UNEXPECTED exception.

Expected results:

Should gracefully return -1 and shut down the geolocation service for document.

Comment 1

[marcos]

Attachment: Bug 1715482 - Geolocation should gracefully handle Documents that are not fully active

Comment 2

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

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 3

[Anne (:annevk)]

Kagami, would you be comfortable reviewing this?

Comment 4

[marcos]

I'm not super confident about the "shutting down the geolocation service" part... It's a bit of an edge case: but I can imagine someone removes an iframe - stuff happens + user moves around - and then re-attaches the iframe again.

Maybe safer if I don't call Shutdown()? 🤔

Comment 5

[Anne (:annevk)]

Once you remove an iframe its associated browsing context is destroyed. If you append it again a new one is created. I don't know what the service is associated with though. I would kinda expect that to be a global service the API calls into, but I'm not familiar with the details.

Comment 6

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

The example never throws anything so I'm not sure I see the issue? It only shows green messages here.

Comment 7

[marcos]

(In reply to Kagami :saschanaz from comment #6)

The example never throws anything so I'm not sure I see the issue? It only shows green messages here.

The example is just testing what happens. It should gracefully return -1 and never throws.

(In reply to Anne (:annevk) from comment #5)

Once you remove an iframe its associated browsing context is destroyed. If you append it again a new one is created. I don't know what the service is associated with though. I would kinda expect that to be a global service the API calls into, but I'm not familiar with the details.

So, Safari (iOS) stops the geo service, even if you re-attach and call from the fresh context.

Chrome on Android resumes the service after one re-attaches the iframe.

If we are creating a new context on re-attachment, Chrome's behavior might be more correct.

Comment 8

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

(In reply to marcos from comment #7)

(In reply to Kagami :saschanaz from comment #6)

The example never throws anything so I'm not sure I see the issue? It only shows green messages here.

The example is just testing what happens. It should gracefully return -1 and never throws.

Okay then, could you provide a test that Firefox fails? 👀

What I see from the current example is:

loading iframe
0
1
2
watchPosition, 3
getCurrentPosition,
reattach
done
Hello from re-attached frame
[object GeolocationPosition]

Comment 9

[marcos]

Yes, will do! Sorry, I thought I'd attached one to the patch! 😅

Comment 10

[marcos]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d26925586c26f8e9e90fba3b5542beee8a9043a

Comment 11

[marcos]

Try 2:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ec626f9ded5877fd1168299b21e47e4f33477bd

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa23b80472a7
Geolocation should gracefully handle Documents that are not fully active r=saschanaz

Comment 13

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/aa23b80472a7