Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for vault timeout not firing on resume #1775

Merged
merged 2 commits into from
Feb 14, 2022
Merged

Conversation

mpbw2
Copy link
Contributor

@mpbw2 mpbw2 commented Feb 13, 2022

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Fix for vault timeout not firing on app resume in most recent release. Closes #1772

Code changes

  • App.xaml.cs: The recent change to make UpdateTheme async resulted in a race condition which allowed BaseContentPage.SaveActivity() to execute prior to VaultTimeoutService.CheckVaultTimeoutAsync(). This resulted in the LastActiveTime value being reset before it could be compared to the current time, making the app think only a few milliseconds had passed regardless of how long it was in the background. By checking vault timeout before executing the improved theme update process, we're able to establish the correct elapsed time again. @fedemkr Let me know if you see any issues with this interfering with the theme updater.

Testing requirements

Vault timeout should function as expected again.

Before you submit

  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

@mpbw2 mpbw2 requested review from fedemkr, vvolkgang and a team February 13, 2022 23:04
@@ -217,10 +217,9 @@ private async Task SleptAsync()

private async void ResumedAsync()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's take the opportunity to change this to async Task and use the FireAndForget method @fedemkr added in a previous PR, logging exceptions.

@mpbw2 mpbw2 requested a review from vvolkgang February 14, 2022 14:19
Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't interfere with the theme update, but just in case let QA know

@mpbw2 mpbw2 merged commit 3127295 into master Feb 14, 2022
@mpbw2 mpbw2 deleted the bugfix-vault-timeout branch February 14, 2022 16:29
mpbw2 added a commit that referenced this pull request Feb 14, 2022
* fix for vault timeout not firing on resume

* call ResumedAsync with FireAndForget
fedemkr added a commit that referenced this pull request Feb 14, 2022
* master:
  Build: Upload dSYMs to AppCenter (#1776)
  Fix for vault timeout not firing on resume (#1775)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android app no longer locks on vault timeout in version 2.16.1
3 participants