Change lock method to turn screen off/on

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: zcampbell, Assigned: viorela)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
The lock method in GaiaDevice locks using some ugly atom code (judging by the comments made by the dev in the code).

I think we can avoid this by changing the lock method to turn the screen off and then on again which just uses ScreenManager. This will avoid the atom completely.

It will also stop any cases where we lock the screen without prior setting of the lockscreen to enabled (which would be an impossible real user scenario).
I think this makes sense as it's how a user will lock a device (or wait for the lock timeout to elapse). It may even be worth deprecating the lock method and encouraging use of sleep/wake methods to turn the screen off/on to more closely emulate a user. We should also update gcli if we change the behaviour of locking programmatically.
(Assignee)

Comment 2

4 years ago
Created attachment 8489895 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24083
Attachment #8489895 - Flags: review?(zcampbell)
Attachment #8489895 - Flags: review?(florin.strugariu)
Attachment #8489895 - Flags: review?(dave.hunt)
(Assignee)

Updated

4 years ago
Assignee: nobody → viorela.ioia
Comment on attachment 8489895 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24083

The change looks okay to me. I'm not 100% comfortable because we're assuming the lockscreen is enabled, but the assertion will protect us if it's not. Note that I haven't run the tests, it would be good to see an adhoc run of all affected tests (including the unit tests) before this lands.
Attachment #8489895 - Flags: review?(dave.hunt) → review+
(Assignee)

Comment 4

4 years ago
I ran all affected tests locally and they passed. I also started and adhoc run, just to make sure: http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame.ui.adhoc/142/console
(Reporter)

Comment 5

4 years ago
I have a feeling we might need ot keep the Wait, I'm holding back from r? until the Gaia-Try results are in.
(Reporter)

Comment 6

4 years ago
Comment on attachment 8489895 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24083

Thanks, I feel more comfortable with the wait back in there!

Thx for doing the adhoc run too, very useful.
Attachment #8489895 - Flags: review?(zcampbell)
Attachment #8489895 - Flags: review?(florin.strugariu)
Attachment #8489895 - Flags: review+
(Reporter)

Comment 7

4 years ago
https://github.com/mozilla-b2g/gaia/commit/556fa57f744baa550291f953a0a31e1e694de2d6
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

4 years ago
Let's uplift this to v2.1. 
All tests that use lockscreen are failing in latest v2.1 build: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-b2g34_v2_1.ui.functional.smoke/78/HTML_Report/
http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-b2g34_v2_1.ui.functional.non-smoke/80/HTML_Report/

I suppose that is because of the changes in https://github.com/mozilla-b2g/gaia/commit/f8d3bf44029e0afc0124600a4bb34dba8fc1ad21

The tests are passing if we update the lock method to turn screen off/on. I'll open a PR with the uplift.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 9

4 years ago
Created attachment 8526009 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26333

uplift to v2.1
Attachment #8526009 - Flags: review?(robert.chira)
Attachment #8526009 - Flags: review?(florin.strugariu)
Attachment #8526009 - Flags: review?(robert.chira)
Attachment #8526009 - Flags: review?(florin.strugariu)
Attachment #8526009 - Flags: review+
https://github.com/mozilla-b2g/gaia/commit/66e6a55892d2c5843d32ebbb63795d2d56892613
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.