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.
Take it and NI Tim for feedback. We may also have some offline discussions.
Assignee: nobody → gweng
The conclusion of Offline discussion: we should start to solve this on master since it's the correct way to handle racing.
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.
Attachment #8645525 - Flags: review?(timdream)
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?
Attachment #8645525 - Flags: review?(timdream) → review+
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.
Let's land it.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-master: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S5 (21Aug)
You need to log in before you can comment on or make changes to this bug.