** This was introduced by bug 1123325 ** My patch to the settings migrator did not take into account that the default settings are a passcode of "0000", even if it is not enabled. That means that when you set a different passcode (using the new settings attribute) the migrator will not notice that *both* exist and overwrite the new-style with the old-style since it thinks a migration is necessary. STR: 1) Create a passcode (e.g., 4444). Locking and Unlocking works fine 2) Reboot 3) Migrator will overwrite new-style value with "0000", because old-style value with "0000" exists. 4) Unlock only possible with "4444". I am fixing this by modifying the existing check to not only see if an old-style passcode is existant but also to bail out if a new one has been already set.

Fred, can you review?

Comment on attachment 8631018 [details] github pull request please add unit test since the migrator is a high risk area

Comment on attachment 8631017 [details] [gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master It's hard to test something that shoots something off asynchronously but does not return the promise. So I had to rewrite a few bits, so that it's returns the promises for things that it kicks off. :-) We could also rewrite the other unit tests to not use a fake clock but look into the promises that are now returned. I'd prefer if this happens in a follow up. For now, I left them untouched as they are passing fine. Fred, can you take another look please?

Comment on attachment 8631017 [details] [gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master please try mockLazyLoader which turns lazy loader to synchronized and make inner code testable

I don't think this would make a big difference, since neither start() nor keyMigration() return anything. We still need to rewrite those functions. Can we please do this in a follow-up? I do not want passcodes broken on master for so long.

I understand that its hard to test inside of lazyloader, so you don't have to. The expect tests are: 1. when certain settings value already exists, make sure it not trigger the migration 2. when certain settings value does not exist, the migration (lazyloader part) is called

Comment on attachment 8631017 [details] [gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master Can you review again, please?

Comment on attachment 8631017 [details] [gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master Thanks for hard work! r+ if comment addressed in github. Please add suite('when old passcode is not present') test case to make sure its not run to LazyLoader.

Master: https://github.com/mozilla-b2g/gaia/commit/506a55fcb5b5992c64f3a776f29294b8b15af6b3

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0. Actual results: Lockscreen passcode is correct and not reset to "0000" after reboot. See attachment: verified_FlameKK_v2.5.3gp Reproduce rate: 0/10 Device: Flame KK 2.5 (Pass) Build ID 20150818150207 Gaia Revision 507ba38fb64b27f87d11f4104dfcc58448e12b1a Gaia Date 2015-08-18 10:50:12 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/2c272af993c23e803f6ea7798a812b0c8abfad4d Gecko Version 43.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150818.184156 Firmware Date Tue Aug 18 18:42:07 EDT 2015 Firmware Version v18D v4 Bootloader L1TC000118D0 Device: Aries KK 2.5(Pass) Build ID 20150818233027 Gaia Revision 1e1197e0e8e64307aa382ffba4711d1c661de7ca Gaia Date 2015-08-18 16:54:35 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/88e1d293ffa9faf065b2a944c94a2705aabf1fb9 Gecko Version 43.0a1 Device Name aries Firmware(Release) 4.4.2 Firmware(Incremental) eng.worker.20150818.225517 Firmware Date Tue Aug 18 22:55:24 UTC 2015 Bootloader s1