Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does window.getScreenDetails() always resolve with the same object? #80

Closed
reillyeon opened this issue Jan 8, 2022 · 4 comments · Fixed by #87
Closed

Does window.getScreenDetails() always resolve with the same object? #80

reillyeon opened this issue Jan 8, 2022 · 4 comments · Fixed by #87

Comments

@reillyeon
Copy link
Member

As written this seems to be implied by the idea that the Promise is resolved "with this's associated ScreenDetails object" but I don't see any specification text which defines how this object is initialized. I recommend extending the Window interface with an internal slot which holds the Promise returned by this function. Future invocations of getScreenDetails() can return this same Promise which may have already resolved.

This raises a related question about what happens when the "window-placement" permission is revoked. This should probably clear this internal slot (so that the permission check has to be rerun) as well as preventing new events for advanced observable properties from firing.

@quisquous
Copy link
Contributor

Is returning the same promise just a question of ergonomics? We assumed that folks would call getScreenDetails() once and cache the value so that everything they wanted to do was synchronous from then on. We're currently resolving the promise with the same object, but it's a different promise every call.

I am not sure we can should do anything when the permission is revoked on the current page as the ScreenDetails object could be referenced by a JavaScript variable. Are you suggesting that we should be throwing exceptions on ScreenDetails property accesses if the permission is revoked?

@reillyeon
Copy link
Member Author

There isn't that much of a difference between returning a new Promise that resolves to the same value and returning the same Promise but the first option most clearly expresses the idea that calling the method multiple times does not repeat any of the previous work.

As for how to handle permissions being revoked, over time I've seen a shift in how this is handled (at least in Chromium). Originally many implementations made the assumption that permission updates wouldn't be reflected until the page was reloaded, acknowledging the issue that you raise about the ScreenDetails object still being potentially referenced by a Javascript variable. Chrome's UI reinforced this with a butter bar saying that changes would take effect when the page reloaded. Since then I've seen some effort being made to have permission changes take effect immediately. For example, revoking permission to access a USB device causes the device to appear to disconnect from the perspective of the page. For an API like this one I don't think it is worthwhile to actually cause any of the ScreenDetails accessors to throw exceptions since the page may have already cached that data anyways but we can shut down any of the listeners which would update those properties, freezing the object in the state it was in when permission was revoked.

@michaelwasserman
Copy link
Member

I attempted to define this a bit better in #87, but I'm unsure how reusing a promise interacts with getScreenDetails() attempting to resolve or reject its promise(s) based on a dynamic permission state. FWIW, it's creating a new promise each time and using an internal slot to store and reuse the ScreenDetails interface object for now.

I also have an issue to "Define behavior of cached objects and method steps when the permission is revoked. (See #80)".

I hope this might be sufficient for now, and that we can refine this further at a later date.

@reillyeon
Copy link
Member Author

I think this is sufficient. Feel free to close this issue when your PR is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants