Merge lp:~mterry/ubuntu-system-settings/wizard-double-finish into lp:ubuntu-system-settings/rtm-14.09

[Michael Terry]

Don't accept multiple "Finish" clicks during the last step of the wizard; doing so can cause password-setting to be left in an inconsistent state.

Don't accept multiple "Finish" clicks during the last step of the wizard; doing so can cause password-setting to be left in an inconsistent state.

Here's how that happens:

A) User clicks Finish multiple times, queuing up multiple input events (queued because Finish makes some blocking calls).
B) The first Finish starts, let's say it's called with new password "1111" and type "passcode". The current password is "" with a type of "swipe".
C) The first Finish completes, having set the password correctly to 1111/passcode.
D) The second Finish starts. Also called with "1111/passcode". But it still thinks the current type is "swipe" because it takes a moment for AccountsService to update itself.
E) The second Finish first changes the display mode hint to "passcode"
F) Then the second Finish can't change the password because it's trying to authenticate using "" when the password is actually "1111" now.
G) So the second Finish enters the "error mode cleanup" stage and tries to revert its display mode hint change from step E. But it's reverting to what it thought the previous type was -- "swipe" -- which uses the "passphrase" hint behind the scenes (instead of "passcode")

Note that this doesn't *always* happen. In between the first and second Finish operations, wizard cleanup starts, and the wizard may be killed during step F, which means we end up with the right state. But it's a race.

Some possible solutions to this:
- Cache the password type locally instead of waiting for AccountsService to update (not a bad idea, I'm not sure whether that's desirable or not -- I originally did it this way thinking local state might get out of sync, but not apparently it's remote state that's out of sync)
- Don't let the user click the Finish button twice. This seems like a no-brainer that should be done anyway. This would also avoid the whole problem.

I think for RTM, just the latter fix is enough. (I have separately filed a unity8 branch to do the latter fix in vivid's welcome wizard.) For trunk, I'd entertain also locally caching the password type if you want me to.

To reproduce this bug / test this fix, enable the wizard, enter adb, run "sudo passwd -d phablet", reboot, then go through the wizard but just keep spamming that final Finish button. You'll also notice some slow down because it's trying to do all those Finish operations.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:975
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-rtm-14.09-ci/48/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-14.09-touch/27
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-rtm-14.09-14.09-armhf-ci/49
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-rtm-14.09-14.09-armhf-ci/49/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-14.09-mako/28
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-14.09-armhf/27
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-14.09-armhf/27/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17580

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-rtm-14.09-ci/48/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

Looks good!

review: Approve

=== modified file 'wizard/qml/main.qml'
--- wizard/qml/main.qml	2014-10-31 14:20:14 +0000
+++ wizard/qml/main.qml	2015-01-28 22:19:27 +0000
@@ -48,6 +48,8 @@
     }
 
     function quitWizard() {
+        pageStack.currentPage.enabled = false
+
         // Immediately go to black to give quick feedback
         blackCover.visible = true