Merge pull request #1585 from famedly/krille/fix-upload-keys

refactor: Trigger upload keys on sync and not in background job and u…

lib/encryption/encryption.dart

@@ -77,9 +77,8 @@ class Encryption {
         olmAccount: olmAccount,
         deviceId: isDehydratedDevice ? deviceId : ourDeviceId,
         pickleKey: pickleKey);
-    _backgroundTasksRunning = ourDeviceId ==
-        client.deviceID; // Don't run tasks for dehydrated devices
-    _backgroundTasks(); // start the background tasks
+
+    if (!isDehydratedDevice) keyManager.startAutoUploadKeys();
   }
 
   bool isMinOlmVersion(int major, int minor, int patch) {
@@ -420,24 +419,7 @@ class Encryption {
     }
   }
 
-  // this method is responsible for all background tasks, such as uploading online key backups
-  bool _backgroundTasksRunning = true;
-  void _backgroundTasks() {
-    if (!_backgroundTasksRunning || !client.isLogged()) {
-      return;
-    }
-
-    keyManager.backgroundTasks();
-
-//    autovalidateMasterOwnKey();
-
-    if (_backgroundTasksRunning) {
-      Timer(Duration(seconds: 10), _backgroundTasks);
-    }
-  }
-
   Future<void> dispose() async {
-    _backgroundTasksRunning = false;
     keyManager.dispose();
     await olmManager.dispose();
     keyVerificationManager.dispose();

lib/encryption/key_manager.dart

@@ -747,15 +747,34 @@ class KeyManager {
     }
   }
 
-  bool _isUploadingKeys = false;
+  Future<void>? _uploadingFuture;
 
-  Future<void> backgroundTasks() async {
+  void startAutoUploadKeys() {
+    _uploadKeysOnSync = encryption.client.onSync.stream
+        .listen((_) => uploadInboundGroupSessions(skipIfInProgress: true));
+  }
+
+  /// This task should be performed after sync processing but should not block
+  /// the sync. To make sure that it never gets executed multiple times, it is
+  /// skipped when an upload task is already in progress. Set `skipIfInProgress`
+  /// to `false` to await the pending upload task instead.
+  Future<void> uploadInboundGroupSessions(
+      {bool skipIfInProgress = false}) async {
     final database = client.database;
     final userID = client.userID;
-    if (_isUploadingKeys || database == null || userID == null) {
+    if (database == null || userID == null) {
       return;
     }
-    _isUploadingKeys = true;
+
+    // Make sure to not run in parallel
+    if (_uploadingFuture != null) {
+      if (skipIfInProgress) return;
+      await _uploadingFuture;
+    }
+    final completer = Completer<void>();
+    _uploadingFuture = completer.future;
+
+    await client.userDeviceKeysLoading;
     try {
       if (!(await isCached())) {
         return; // we can't backup anyways
@@ -817,7 +836,7 @@ class KeyManager {
     } catch (e, s) {
       Logs().e('[Key Manager] Error uploading room keys', e, s);
     } finally {
-      _isUploadingKeys = false;
+      completer.complete();
     }
   }
 
@@ -1017,7 +1036,10 @@ class KeyManager {
     }
   }
 
+  StreamSubscription<SyncUpdate>? _uploadKeysOnSync;
+
   void dispose() {
+    _uploadKeysOnSync?.cancel();
     for (final sess in _outboundGroupSessions.values) {
       sess.dispose();
     }

lib/encryption/utils/bootstrap.dart

@@ -594,7 +594,7 @@ class Bootstrap {
           'And finally set all megolm keys as needing to be uploaded again...');
       await client.database?.markInboundGroupSessionsAsNeedingUpload();
       Logs().v('And uploading keys...');
-      await client.encryption?.keyManager.backgroundTasks();
+      await client.encryption?.keyManager.uploadInboundGroupSessions();
     } catch (e, s) {
       Logs().e('[Bootstrapping] Error setting up online key backup', e, s);
       state = BootstrapState.error;

lib/src/client.dart

@@ -553,6 +553,9 @@ class Client extends MatrixApi {
   /// including all persistent data from the store.
   @override
   Future<void> logout() async {
+    // Upload keys to make sure all are cached on the next login.
+    await encryption?.keyManager.uploadInboundGroupSessions();
+
     try {
       await super.logout();
     } catch (e, s) {
@@ -567,6 +570,9 @@ class Client extends MatrixApi {
   /// including all persistent data from the store.
   @override
   Future<void> logoutAll() async {
+    // Upload keys to make sure all are cached on the next login.
+    await encryption?.keyManager.uploadInboundGroupSessions();
+
     final futures = <Future>[];
     futures.add(super.logoutAll());
     futures.add(clear());

test/encryption/online_key_backup_test.dart

@@ -97,7 +97,7 @@ void main() {
           forwarded: true);
       var dbSessions = await client.database!.getInboundGroupSessionsToUpload();
       expect(dbSessions.isNotEmpty, true);
-      await client.encryption!.keyManager.backgroundTasks();
+      await client.encryption!.keyManager.uploadInboundGroupSessions();
       await FakeMatrixApi.firstWhereValue(
           '/client/v3/room_keys/keys?version=5');
       final payload = FakeMatrixApi