Promise for unlock()?

[jjmax75]

I understand that this might not be possible from the note -

unlock() does not return a Promise because it is equivalent to locking to the default orientation which might or might not be known by the user agent. Hence, the user agent can not predict what the new orientation is going to be and even if it is going to change at all.

I am working on building an app that uses WebVR. Entering VR mode requires going fullscreen and then locking orientation to landscape. This works perfectly.

However exiting this mode when phone is in portrait orientation, on tested Android devices, results in the browser bar and menu disappearing and a large white space appearing under the app. Dragging down on the screen reveals the browser bar and covers the white space, you can not then drag back up.

I've overcome this issue using the following code

screen.orientation.unlock();
setTimeout( () => {
    fullscreen.exit();
}, 0 );

A cleaner solution would be something like -

screen.orientation.unlock().then( () => {
    fullscreen.exit();
});

[marcoscaceres]

@mounirlamouri I’ve been going over the spec with @Johanna-hub and, given unlocking is “lock to the default” (even if there is no actual orientation change) I am starting to be convinced that returning a promise is actually the right thing to do here: if there is no actual change, we just return a resolved promise. Otherwise, wait for orientation change and we resolve the default promise.

WDTY?

[mounirlamouri]

It's been a few years but I would look into whether an implementation of this would be reliable on Android and iOS. On Android, unlock will get the app to ask the system to go back to its default orientation which the app doesn't know off hand. We should check whether this is easy to check but either way, there would be some reverse engineering of the Android system code to do in the UA which doesn't seem great.

[jjmax75]

Hi guys, I opened this a while back when I was working at another company.

As I recall the promise approach would have been nice as it matched up with the lock() method but I'm not sure anymore about that.

My case was definitely an edge case at the time but would have been nice not to use the setTimeout hack :D

[marcoscaceres]

Thanks @jjmax75 for following up. It's come up again in a different issue - hi @kenchris! could use your input here too.

On Android, unlock will get the app to ask the system to go back to its default orientation which the app doesn't know off hand. We should check whether this is easy to check but either way, there would be some reverse engineering of the Android system code to do in the UA which doesn't seem great.

Agree - we definitely don't want to go around poking at system code. However, we should check if since we first spec'ed this if the APIs have changed to provide a call back?

@Johanna-hub, could you please research "Handling View Rotations" in iOS:
https://developer.apple.com/documentation/uikit/uiviewcontroller

And, on Android, setRequestedOrientation:
https://developer.android.com/reference/android/app/Activity#setRequestedOrientation(int)

@mounirlamouri, I wonder if for unlock we could still just call:

setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED);

And just resolve the promise? The call at the Android level is synchronous anyway, by the looks of the documentation.

[kenchris]

I would really like these to return promises if possible - it is really brittle integrating this with fullscreen today.

cc @dominickng

[marcoscaceres]

I wonder if we have any visibility into the implementation of this in Chrome on Android? I'm guessing not. I'd be really interested to see how setRequestedOrientation (or something else?) is being called specially for locking and unlocking.

[dominickng]

The Chrome on Android implementation looks like it's here

It looks to me like there's a separate call that happens when restoring from VR mode here

[marcoscaceres]

Gecko's one is here:
https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoScreenOrientation.java#258

Looking at unlock(), it's basically identical to lock() - which includes a confirmation of actual unlocking. So seem totally feasible to return the promise. I've emailed there person who implemented it at Mozilla to get confirmation.

[marcoscaceres]

@Johanna-hub and I went poking around the DOM implementation... basically unlock() calls into our HAL (hardware abstraction layer):
https://searchfox.org/mozilla-central/source/hal/Hal.h#244

And for whatever reason (probably the reason @mounirlamouri gave :)), the Hal definition of unlock() is fire and forget (no return value):

void UnlockScreenOrientation();

This doesn't seem to align with the Java side tho... which does return a boolean for unlock. Trying to find out more.

[marcoscaceres]

Gecko bug... seeing if it's doable as we suspect this causing intermittent failures because of the interaction with exit fullscreen: https://bugzilla.mozilla.org/show_bug.cgi?id=1567805

[mounirlamouri]

How would you get confirmation from unlocking?

[marcoscaceres]

I think it might be sufficient to just IPC over to Android and do the call to setRequestedOrientation() and then resolve the promise in the task that’s doing the request. Even if it doesn’t change, it should prevent the race condition with .exitFullScreen().

So, we would just return the promise here, instead of dropping it on the floor:
https://github.com/mozilla/gecko-dev/blob/master/dom/base/ScreenOrientation.cpp#L377

[marcoscaceres]

Actually, reflecting on this, I think we need to tie the two APIs together.

You can't use this API without full screen, and exiting full screen should automatically unlock.

I'll spin up a new issue.

[marcoscaceres]

filed #209

[marcoscaceres]

Reopening, as this is still an issue.

@saschanaz, @foolip, @michaelwasserman, we've now hit this issue also in WebKit (this was also an issue in Firefox, IIRC).

The problem is that we end up with a race condition:

1. unlock() makes IPC call.
2. exitFullscreen() makes IPC call.

The IPC calls can arrive and be processed out of order. That makes a mess for FS API and this one.

I'd like to make .unlock() return a promise. Would you have any objections to us doing that?

[annevk]

Could you maybe explain the mess and how returning a promise helps resolve it?

At least to me one seems like an implementation concern we'd have to resolve one way or another and the other is a nicety of the API, but doesn't really change the implementation. Except it requires it to do something with the promise, but that seems strictly additional to having to deal with the race.

(One way out might be to make unlock() no-op and require exiting fullscreen instead. Not sure how compatible that is though.)

[marcoscaceres]

Could you maybe explain the mess and how returning a promise helps resolve it?

Sure!

Problem

The over problem with the design of the API is that it's designed to be "fire and forget". I think this was done originally (9 years ago) because some OSs didn't provide any signal about the success of the orientation change.

That led to the current model:

(promise) `.lock()` -> IPC -> OS maybe does something -> IPC (success/fail) -> settle JS promise. 

And the assumption with unlock() was the it was "fire and forget" (hence no promise):

`.unlock()` -> IPC -> OS maybe does something but doesn't notify success. 

The above assumption is wrong, because .unlock() IPC SHOULD notify of success/failure.

Thus, if one calls:

p1 = lock();
unlock();

They race, because the IPC messages could end up out of order. Similarly:

.unlock();
.exitFullscreen();

Has the race condition, as noted by the OP. This leads to horrible hacks, like using setTimeout() and requestAnimationFrame() to provide enough time for the screen to actually unlock.

The work around in WebKit's test framework is now to always basically do:

const initialOrientation = screen.orientation.type;
// ... test ....
// manually reset it
await .lock(initialOrientation);
.unlock(); // nothing happens at device because of call above
await .exitFullscreen();

Addressing the issue

What should have actually happened in the API is that it should have been impossible to make call to .unlock() (or another .lock()) before the first call finishes (and the promise settles).

const p1 = orientation.lock("any");
// InvalidStateError, p1 hasn't settled.
const p2 = orientation.lock("landscape");
// Unlock, InvalidStateError, p1 hasn't settled
const p3 = orientation.unlock();

Unfortunately, doing the above might no longer be web compatible...

What we should do...

Unlocking should be treated exactly the same as .lock(). That is:

(promise) `.unlock()` -> IPC -> OS maybe does something -> IPC (success/fail) -> settle JS promise. 

That means that .unlock() will abort any pending .lock() promises (same as lock already does), and will have exactly the same behavior as .lock(). When used correctly by developers, the race conditions go away:

await .lock();
await .unlock();
await .anyOtherAPI();

(One way out might be to make unlock() no-op and require exiting fullscreen instead. Not sure how compatible that is though.)

The problem is that the API can be used outside the context of the FS API (e.g., a web application that was installed that launched full screen).

However, we should nonetheless integrate the two APIs as I proposed here:
whatwg/fullscreen#202 (comment)

[annevk]

Okay, so the promise gives web developers a chance to do things better, but we still have to define what happens when they don't. I suppose there's also calling unlock() followed by lock(). And are these IPC calls all truly parallel or is it more a parallel queue (see HTML)? It seems weird for IPC to go out-of-order but I guess it depends on what it ends up talking to.

[marcoscaceres]

Okay, so the promise gives web developers a chance to do things better,

Not only that, but predictability and a degree of assurance that unlocking actually happened (or, in an exceptional case, not).

but we still have to define what happens when they don't.

Yes, absolutely: that's mostly what all the machinery that aborts the promises of previous calls to lock() across all associated documents does. The current model is far from perfect (how it works with events is still a bit busted as per #184), but it's workable enough.

It seems weird for IPC to go out-of-order but I guess it depends on what it ends up talking to.

Yeah, this is why the integration with FS API should really help: it will prevent much of the weirdness that can happen when trying to full screen and then change orientation by allowing the user agent to do it all smoothly in one action.

[marcoscaceres]

Sent #231 ... folks interested in this topic, would really appreciate your review and feedback. CC @michaelwasserman

[michaelwasserman]

This seems like it would be a clear win for screen orientation API ergonomics, but I'm curious about the premise:
The above assumption is wrong, because .unlock() IPC SHOULD notify of success/failure.

Can you reference source material regarding how underlying OS APIs report the success or failure of these requests? I'm a novice in this space and I don't see anything quite so obvious.

It doesn't help that Chromium's current implementation seems incautious at first glance. After issuing OS requests, promises seem to be resolved immediately for locks matching the current orientation, or when a matching orientation change event occurs.

Should the spec incorporate potentially undetectable OS API failures, beyond leaving promises indefinitely unresolved?

[marcoscaceres]

Can you reference source material regarding how underlying OS APIs report the success or failure of these requests? I'm a novice in this space and I don't see anything quite so obvious.

I'm not sure about how it works on Android... but quick search says one simply sets requestedOrientation on an Activity and then you can check if onConfigurationChanged was called. That doesn't exactly tell you if it failed though. 🤔 We might need to ask someone who knows Android well.

On Apple's platform, one can monitor for changes also in a similar manner via UIDeviceOrientationDidChangeNotification.

There is also requestGeometryUpdate(_:errorHandler:), which, as you can see from the method signature, literally has the error handler as the callback. For the success case, one would rely on the change notification, should dispatch after a successful change.

[michaelwasserman]

Returning a promise seems nice, but the spec might need to note they are best-effort? AFAICT:

1. OSes may lack success/failure signals, or delay honoring requests (e.g. activity backgrounded)
2. OS notifications for user-triggered orientation changes may happen to match ignored/pending API lock requests

[dmurph]

TPAC 2024: We can acknowledge that this is best case. Returning Promise.resolve() would at least spin the event loop to help folks get out of trouble. Change seems fine.