Closed Bug 851642 Opened 11 years ago Closed 11 years ago

lockOrientation stops working, lets homescreen/apps freely rotate

Categories

(Core :: DOM: Core & HTML, defect)

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

()

RESOLVED FIXED
1.0.1 Cert2 (21may)
blocking-b2g tef+
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- wontfix
firefox24 --- fixed
b2g18 --- verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- verified

People

(Reporter: Harald, Assigned: mounir)

References

(Blocks 1 open bug)

Details

(Whiteboard: [games:p?][apps watch list])

Attachments

(4 files)

This happens occasionally when debugging apps, that homescreen suddenly freely rotates freely between landscape and portrait.

screen.lockOrientation seems to unlock the screen at one point, I had the same problem in my app.

What happens:
At one point (not sure when), lockOrientation stops working and the screen feely rotates. This can be seen on homescreen but also apps using lockOrientation.

Steps to reproduce:
Install https://marketplace.firefox.com/app/game-pack/ , it uses screen.lockOrientation to lock orientation for each game. Switch between games and at one point lock orientation will stop working, only working again after restarting the whole app.

I see this happen a lot and it really breaks homescreen and apps UX. Therefor marking critical, maybe even blocker.
Mounir - Any ideas why lock orientation eventually stops working?
Component: Gaia → General
Flags: needinfo?(mounir)
Harald, do you have the same issue if you install this game on Android?
Flags: needinfo?(mounir) → needinfo?(hkirschner)
This is not related to this app, but I will try and come back with results.

Screwing up homescreen orientation happens a lot when switching between apps that lock orientation (which is mostly games).
Flags: needinfo?(hkirschner)
I'm asking because screen lock feature isn't only a Firefox OS thing. There is a common layer (the DOM layer) and a backend layer (one for Gonk, aka Firefox OS and one for Android). If there are issues only on Firefox OS, that means there is a bug in Gonk and we should CC the correct persons.
The app is packaged and can't be tested on Android at this point (but packaged apps will work soon).

I'd recommend a test case that just switched between 2 pages, both asking for different orientation. If I have time this week, I can create on.
Flagging you for information to keep track of this.
Flags: needinfo?(hkirschner)
I am seeing the same issue on the camera app on ikura:

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/54285d67454b
Gaia   c47ef39de04e634d847cc86b6acc616fabce69eb
BuildID 20130426070207
Version 18.0

I can reproduce this by rotating the phone with videocamera preview in focus.   The camera icons at the bottom continue to rotate a bit after I'm in landscape orientation.
Blocks: 866478
No longer blocks: 866478
Impacts a preinstall - gamepack. Partner reproduced this as well, and TEF wasn't too happy about this bug either.
blocking-b2g: --- → tef?
Although note that a potential workaround here instead of fixing this in the short term is ask the app developer to use the app manifest to permanently lock the orientation to portrait-primary.
blocking-b2g: tef? → tef+
I don't see a workaround due to the nature of the bug. Once lockOrientation stops working once, calling it again doesn't work, even for homescreen. Only switching the screen off and on resets it.

I am open to ideas on how to work around it in the short term.

I will also work on test case, but until then I recommend to install the game pack; as it's the best reproduction we got.
Flags: needinfo?(hkirschner)
Attached image Homescreen in landscape
Showing the severity, even homescreen isn't locked anymore and freely rotates. This only stops when switch the screen off and on.
Alive, didn't we have an issue with orientation recently?
Assignee: nobody → alive
I couldn't reproduce by quickly switching the games in game-pack app. And I don't know what  does comment 7 mean, exactly.

But I notice that if the user is on a game in game pack app and then press home button and then launch game-pack again, the orientation is wrong.

We couldn't fix anything from this but the app dev should listen to 'visibilitychange' event to handle app switch if they want to restore the orientation by themselves.
(In reply to Alive Kuo [:alive] from comment #14)
> I couldn't reproduce by quickly switching the games in game-pack app. And I
> don't know what  does comment 7 mean, exactly.
> 
> But I notice that if the user is on a game in game pack app and then press
> home button and then launch game-pack again, the orientation is wrong.
> 
> We couldn't fix anything from this but the app dev should listen to
> 'visibilitychange' event to handle app switch if they want to restore the
> orientation by themselves.

What you are describing here that you've seen sounds like bug 865010.
Whiteboard: [games:p?]
QA Contact: jsmith
I was able to reproduce very easily on a 5/3 Unagi build.

See the STR Lisa gave in https://bugzilla.mozilla.org/show_bug.cgi?id=866478#c3 that worked for me to reproduce this.
Keywords: qawanted
Attached image screenshot
issue reproduces pretty easily following STR from Comment 0

1. install https://marketplace.firefox.com/app/game-pack/
2. play any game that requires landscape mode - "Farm Lines"
3. tap home button to go to the homescreen
    => homescreen appears in landscape mode as shown on the screenshot attachment

tested on Inari device running the following build:
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3f3489356bbc
Gaia   3e232bce289c9e156d92553e752616cba284bc8f
Build  20130503070205
Version 18.0
I use the lasted v1.0.1 build and it's easy for homescreen app to turn upside down after unlocking lockscreen. Lemme check if something wrong in WM.
My thought is this might be a platform bug because the execution flow in gaia doesn't seem wrong:

1. Launch game-pack(screen.mozUnlockOrientation in system app:Window Manager)
2. Click a landscape game(screen.mozLockOrientation in game-pack app)
3. Press home button(screen.mozLockOrientation in system app:Window Manager)

The 3 call to mozLockOrientation doesn't fail but the orientation of homescreen app isn't locked.

During this test I also find another even more terrible thing: we don't prevent a background app to call mozLockOrientation. With this, if an app is trying to call |mozLockOrientation('landscape-primary') when it's in background, the homescreen would be corrupted by this API. Weird....
Whiteboard: [games:p?] → [games:p?][apps watch list1]
(In reply to Alive Kuo [:alive] from comment #20)
> My thought is this might be a platform bug because the execution flow in
> gaia doesn't seem wrong:
> 
> 1. Launch game-pack(screen.mozUnlockOrientation in system app:Window Manager)
> 2. Click a landscape game(screen.mozLockOrientation in game-pack app)
> 3. Press home button(screen.mozLockOrientation in system app:Window Manager)
> 
> The 3 call to mozLockOrientation doesn't fail but the orientation of
> homescreen app isn't locked.

The above and below (and the dependency) is contradict with one another.
Will this bug be fixed by bug 868914 or not? If so we need to raise that bug to tef+ ...

> During this test I also find another even more terrible thing: we don't
> prevent a background app to call mozLockOrientation. With this, if an app is
> trying to call |mozLockOrientation('landscape-primary') when it's in
> background, the homescreen would be corrupted by this API. Weird....
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #21)
> (In reply to Alive Kuo [:alive] from comment #20)
> The above and below (and the dependency) is contradict with one another.
> Will this bug be fixed by bug 868914 or not? If so we need to raise that bug
> to tef+ ...

That bug have been already tef+'d.
Alive, see comment 20. Will this bug be fixed by bug 868914?
Flags: needinfo?(alive)
I am going to deassign myself per comment 20 because IMO this not a gaia bug and from gecko side I'm not qualified enough to figure out what's the root cause.

Bug 868914 is not relevant to this issue.
Assignee: alive → nobody
Flags: needinfo?(alive)
No longer blocks: 868914
Mounir, see comment 20.

The call to screen.mozLockOrientation('portrait-primary') returns true,
and after that I don't see any call to lock/unlock from gaia side.

The way homescreen behaves like as if the orientation is locked, and soonly unlocked. The only reason I could come up is somebody calls screen.unlockOrientation, maybe in gecko, because it's not homescreen or system app theirself. This case isn't cover in bug 868914 though.
set ni? for comment 25
Flags: needinfo?(mounir)
Today :bchen did some survey and he found that nsScreen object is destoryed after returning to homescreen.

My guess to this is:
1. When home button is pressed, the game-pack app is still visible.
1.5 We call lockOrientation: portrait-primary for homescreen in gaia here.
2. When the app zoom out animation ends, we set visibility of the app to be invisible.
3. The app is killed by OOM. Or it is crashed.
4. The screen object of the app is killed, and in the destructor it calls unlockOrientation in gecko.

4 overwrites what 1.5 does.

The problem is:
1. Why we need to call unlockOrientation in gecko? It's easy to introduce race condition.
2. Why the app seems to be OOM but only screen object is destroyed instead of the whole app? I don't see mozbrowsererror[fatal] event is passed to gaia.
backtrace for the |UnlockScreenOrientation|.

It looks like when we are pressing home key back to homescreen, the app
receive "memory-pressure" then trigger |nsScreen::Reset()|.
And we do |hal::UnlockScreenOrientation()| in |nsScreen::Reset()|.
Daniel - you tef+'d this, what test cycle/target milestone should be set on it?
Flags: needinfo?(dcoloma)
Assignee: nobody → mounir
(In reply to Alive Kuo [:alive] from comment #27)
> Today :bchen did some survey and he found that nsScreen object is destoryed
> after returning to homescreen.
> 
> My guess to this is:
> 1. When home button is pressed, the game-pack app is still visible.
> 1.5 We call lockOrientation: portrait-primary for homescreen in gaia here.
> 2. When the app zoom out animation ends, we set visibility of the app to be
> invisible.
> 3. The app is killed by OOM. Or it is crashed.
> 4. The screen object of the app is killed, and in the destructor it calls
> unlockOrientation in gecko.
> 
> 4 overwrites what 1.5 does.
> 
> The problem is:
> 1. Why we need to call unlockOrientation in gecko? It's easy to introduce
> race condition.
> 2. Why the app seems to be OOM but only screen object is destroyed instead
> of the whole app? I don't see mozbrowsererror[fatal] event is passed to gaia.

Yes, this is a gecko bug. We can't rely on the destructor of a reference counted object to do anything at a predictable time. Mounir, we need to do the unlocking here based on navigation time, not based on when the nsScreen destructor gets called. Navigator already gets a call on navigation (Navigator::OnNavigation()), we should call into the nsScreen::Reset() code at the same time as well. Or something like that :)
Attached patch PatchSplinter Review
This should fix the problem you have been experiencing on Firefox OS. I haven't tried it though.
On Firefox Android, this patch is fixing a lot of edge cases that were very flaky before.
Attachment #747533 - Flags: review?(bugs)
Flags: needinfo?(mounir)
Whiteboard: [games:p?][apps watch list1] → [status: needs review][games:p?][apps watch list1]
Attachment #747533 - Flags: review?(bugs) → review+
Flags: needinfo?(dcoloma)
Whiteboard: [status: needs review][games:p?][apps watch list1] → [status: needs landing/uplift][games:p?][apps watch list1]
Target Milestone: --- → 1.0.1 Cert2 (28may)
Whiteboard: [status: needs landing/uplift][games:p?][apps watch list1] → [status: needs landing/uplift][games:p?][apps watch list]
https://hg.mozilla.org/integration/mozilla-inbound/rev/7679ca720a18
Status: NEW → ASSIGNED
Component: General → DOM
Product: Boot2Gecko → Core
Version: unspecified → Trunk
(In reply to Mounir Lamouri (:mounir) from comment #32)
> Created attachment 747533 [details] [diff] [review]
> Patch
> 

This patch should be also applied to 1.1 as the same issue occurs in leo.
https://hg.mozilla.org/mozilla-central/rev/7679ca720a18
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Keywords: verifyme
(In reply to Leo from comment #35)
> This patch should be also applied to 1.1 as the same issue occurs in leo.

tef+ includes both 1.0.1 and 1.1 landing. That said, this needs a branch-specific patch for uplift.
Whiteboard: [status: needs landing/uplift][games:p?][apps watch list] → [games:p?][apps watch list]
ooo, fancy...
This patch doesn't work for me at all - I can still reproduce this bug with STR given https://bugzilla.mozilla.org/show_bug.cgi?id=851642#c18. Followup bug coming.
comment 42 was tested with a 5/16 unagi build on b2g18.

Build Id: 20130516230207
Also reproduced on an inari 1.01 5/17 build.
Depends on: 873668
No longer depends on: 873668
Per talking with Mounir, the issue I hit was a different bug.

As for this bug, this now works for me on b2g18 and 1.01.
Blocks: gecko-games
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: