1813416 - Fenix prevents screen/display lock after timeout if media played previously

Status: VERIFIED FIXED

(GeckoView :: Media, defect)

Description

[Chris Peterson [:cpeterson]]

From github: https://github.com/mozilla-mobile/fenix/issues/27897.

Steps to reproduce

1. Open any website. Observe that the screen gets locked after the screen timeout threshold set in OS.
2. Open any website and play any short video.
3. After this, close the tab and open another.
4. Open a completely different website.
5. Wait for the screen to time out.
6. Screen permanently on.
7. Switch away from Fenix.
8. Timeout works.
9. Switch back to Fenix.
10. No more screen timeout.

Expected behaviour

The screen should turn off after the timeout.

Actual behaviour

The screen remains on permanently, as long as you're on Fenix.

Device name

multiple

Android version

Android 12

Firefox release type

Firefox

Firefox version

105.1.0

Device logs

No response

Additional information

No response

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[Chris Peterson [:cpeterson]]

QA, can you reproduce this bug on Android 12? Makoto wasn't able to reproduce on Android 13. The bug reporter was using Android 12 (but Makoto didn't have an Android 12 device).

Updated STR from the GitHub issue:

Steps to reproduce

1. Open any website. Observe that the screen gets locked after the screen timeout threshold set in OS.
2. Open https://libreddit.kavin.rocks/r/contagiouslaughter and play any short video in full screen.
3. After this, close the tab and open another.
4. Open a completely different website.
5. Wait for the screen to time out.
6. Screen permanently on.
7. Switch away from Fenix.
8. Timeout works.
9. Switch back to Fenix.
10. No more screen timeout.

Comment 2

[Makoto Kato [:m_kato]]

I suspect that this occurs after we upgrade newer exoplayer2. If this occurs on HLS only, I guess that root cause is that. Newer version uses screen wake lock. If I can reproduce this, I can verify it.

Comment 3

[zesanup]

I've ruled out HLS as being the cause. In both HLS and non-HLS cases, it is going full screen that causes the bug to occur.

Also, I've now reproduced this on both Android 12 and 13.

Comment 4

[Makoto Kato [:m_kato]]

I can procedure this on my OPPO A55s. I am looking...

Comment 5

[Makoto Kato [:m_kato]]

I guess https://searchfox.org/mozilla-mobile/rev/c4186545f9f5534b456e7e285c2725deeee959d6/fenix/app/src/main/java/org/mozilla/fenix/ext/Activity.kt#38

Comment 6

[Mira Lobontiu (Android QA)]

Hello all!
I was able to reproduce this bug on an Oppo Reno 6 (Android 12) device, and on an Oppo Find X3 Lite (Android 11), on Fenix Nightly 111.0a1 from 2/6, Beta 110.0b6, and RC 109.2.0, following the steps provided by Chris.
Not able to reproduce it on a Google Pixel 6 (Android 13).

Comment 7

[Makoto Kato [:m_kato]]

Or https://github.com/mozilla-mobile/android-components/issues/12544?

Comment 8

[Makoto Kato [:m_kato]]

Do you know whether this is regression? This may not occur on Firefox 103, 104 or 105.

Comment 9

[BMO Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1813416 - Clear FLAG_KEEP_SCREEN_ON when playing media is finished. (#716)

Comment 10

[Mira Lobontiu (Android QA)]

I've installed several older builds, and it turns out everything worked as expected until RC 104.2.0 build 2 from here https://firefox-ci-tc.services.mozilla.com/tasks/index/mobile.v2.fenix.release.2022.09.08.latest/arm64-v8a.
The first "bad" build was RC 105.1.0 - https://firefox-ci-tc.services.mozilla.com/tasks/index/mobile.v2.fenix.release.2022.09.12.latest/arm64-v8a.

Comment 12

[Lorand Janos]

Managed to reproduce the issue on RC 110 Build 1 from 2023-02-07 using an Oppo Find X5 (Android 13).
Verified as fixed on the latest Nightly 111.0a1 (2023-02-13) using the same device.
Marking the ticket as verified fixed.

Comment 13

[Lorand Janos]

Verified on the latest Beta 111.0b2 build.
Devices used:

• Oppo Find X3 Lite (Android 11).
• Oppo Reno 6 (Android 12).
• Oppo Find X5 (Android 13).

Comment 14

[Chris Peterson [:cpeterson]]

Makoto, do you think this bug fix would be safe to uplift to a 110 dot release?

Comment 15

[Makoto Kato [:m_kato]]

(In reply to Chris Peterson [:cpeterson] from comment #14)

Makoto, do you think this bug fix would be safe to uplift to a 110 dot release?

Yes, I think risk is low.

Comment 16

[Chris Peterson [:cpeterson]]

(In reply to Makoto Kato [:m_kato] from comment #15)

(In reply to Chris Peterson [:cpeterson] from comment #14)

Makoto, do you think this bug fix would be safe to uplift to a 110 dot release?

Yes, I think risk is low.

Cool. Can you please request uplift to firefox110 next week? We can include it in a 110 dot release to be built on March 6.

Comment 17

[Makoto Kato [:m_kato]]

Comment on attachment 9316106 [details] [review]
[mozilla-mobile/firefox-android] Bug 1813416 - Clear FLAG_KEEP_SCREEN_ON when playing media is finished. (#716)

Beta/Release Uplift Approval Request

• User impact if declined: When user plays video content by fullscreen mode, Firefox doesn't turn off screen automatically until Firefox is foreground. This issue will be continued until Firefox is stop.

This is bad thing for power consumption.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We forget to clear screen wake lock when exiting fullscreen or video is finished.
• String changes made/needed:
• Is Android affected?: Yes

Comment 18

[Pascal Chevrel:pascalc]

(In reply to Chris Peterson [:cpeterson] from comment #16)

(In reply to Makoto Kato [:m_kato] from comment #15)

(In reply to Chris Peterson [:cpeterson] from comment #14)

Makoto, do you think this bug fix would be safe to uplift to a 110 dot release?

Yes, I think risk is low.

Cool. Can you please request uplift to firefox110 next week? We can include it in a 110 dot release to be built on March 6.

Note that our 110 planned dot release builds today.

Comment 19

[Pascal Chevrel:pascalc]

Comment on attachment 9316106 [details] [review]
[mozilla-mobile/firefox-android] Bug 1813416 - Clear FLAG_KEEP_SCREEN_ON when playing media is finished. (#716)

Approved for our next dot release, thanks. 
We will need a rebased PR to the 110 branch.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

(In reply to Pascal Chevrel:pascalc from comment #19)

We will need a rebased PR to the 110 branch.

Comment 21

[Makoto Kato [:m_kato]]

Attachment: for releases_v110

Comment 22

[Ryan VanderMeulen [:RyanVM]]

We need a PR that is for landing on the v111 branch, not a patch attached to this bug. You should be able to create that PR automatically by adding a @mergifyio backport releases_v111 comment to the original PR.

Comment 23

[Makoto Kato [:m_kato]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #22)

We need a PR that is for landing on the v111 branch, not a patch attached to this bug. You should be able to create that PR automatically by adding a @mergifyio backport releases_v111 comment to the original PR.

I have no permission for it. (https://github.com/mozilla-mobile/firefox-android/pull/716#issuecomment-1449347887)

Comment 24

[BMO Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1813416 - Clear FLAG_KEEP_SCREEN_ON when playing media is finished. (backport #716) (#1046)

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9316106 [details] [review]
[mozilla-mobile/firefox-android] Bug 1813416 - Clear FLAG_KEEP_SCREEN_ON when playing media is finished. (#716)

Moving Pascal's approval over to the new attachment for 110.1.1.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Johan, can we please look into why Makoto didn't have permission to initiate the backport?

Comment 27

[Johan Lorenzo [:jlorenzo]]

Attachment: mergify restriction.png

I see :m_kato doesn't have write access to the repo (see screenshot). I see he's not part of the mozilla-mobile Github organization[1] which explains the lack for write access.

I see 2 ways of fixing this:

1. either we loosen the restriction on the backport command to grant anyone the ability to create backports.
2. or we assing :m_kato a team on the mozilla-mobile GH org.

:csadilek, what's your take on this?

[1] https://github.com/orgs/mozilla-mobile/people?query=kato
[2] https://docs.mergify.com/configuration/#commands-restrictions

Comment 28

[Christian Sadilek [:csadilek]]

or we assing :m_kato a team on the mozilla-mobile GH org.

Absolutely, not sure why that hasn't happened yet.

:m_kato I've sent you an invite to join a Github team. You will then have write access to the repo.

Comment 29

[Pascal Chevrel:pascalc]

Chris, how important is it to ship a fenix dot release for this single patch on Monday one week before 111 ships? We usually do not ship dot releases unless forced by a chemspill during RC week and this bug is an S3.

Comment 30

[Chris Peterson [:cpeterson]]

Pascal and I talked offline. There's no need to ship a new 110 dot release next week.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3af10a4c6ae2
Clear FLAG_KEEP_SCREEN_ON when playing media is finished.