1189641 - [LockScreen][pre-Gleipnir] Move listeners and observers from lockscreen.js to other controller (mid-term plan)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)

Description

[Greg Weng [:snowmantw][:gweng][:λ]]

Bug 1173284 reveals the potential flaw of handling inputs in the legacy lockscreen.js. It also confuses people since it's unclear which file should be patched. So I propose to move the listeners out of LockScreen, therefore LockScreen will be a simple controllee, not the controller. In that way, it will keep all the function like unlocking and changing the wallpapers, and components rely on these functions can still work well.

The controller is not a whole new component. Since we now have the LockScreenStateManager, we can move these logic to the state manger and states. After that, we can start to migrate to Gleipnir, the library for state machine.

Comment 1

[Greg Weng [:snowmantw][:gweng][:λ]]

Take it and NI Tim for feedback. We may also have some offline discussions.

Comment 2

[Greg Weng [:snowmantw][:gweng][:λ]]

The conclusion of Offline discussion: we should start to solve this on master since it's the correct way to handle racing.

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] snowmantw:bug1189641-rev2 > mozilla-b2g:master

Comment 4

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8645525 [details] [review]
[gaia] snowmantw:bug1189641-rev2 > mozilla-b2g:master

Fixes:

1. Move all security value readings to state manager: when user trigger new inputs, they blocks the following transferring if they're still pending.
2. Don't touch other parts yet. I need to crack down the file patch by patch.
3. Remove legacy code.
4. Add new "integration" test in unit test of state manager.

Test: 

1. Manually tested these:

Respect passcode timeout after rebooting: Yes
Respect timeout after setting it and before rebooting: Yes
With passcode, no bypassing while booting: Yes
Without passcode, no bypassing while booting: Yes
Secure app works without bypassing passcode: Yes
Unlocking to ordinary app works after passcode expired: Yes

2. CI result: green:

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=3610f82d9497d958a01a7af87db15c0b98c7074a

After reviewing and landing I think we need QA and security team to verify this solved the racing bug on master.

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8645525 [details] [review]
[gaia] snowmantw:bug1189641-rev2 > mozilla-b2g:master

Looks like you nail it! What more can we do to prevent regressions than writing integration tests in GU suite? Is it feasible to do so in Gij suite?

Comment 6

[Greg Weng [:snowmantw][:gweng][:λ]]

If you mean to prevent the further racing, yes, we should add Gij test. But the difficult part is it only happen (or easily happen) after a device reset, not b2g-restart. So I'm not sure if we can really catch the regression by Gij.

I may give a try after I solve other urgent bugs. I'll fire a follow-up bug first.

Comment 7

[Greg Weng [:snowmantw][:gweng][:λ]]

Let's land it.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/6b6bc7bae0b18fa0bce70e0dfab56eb01409b5cc