Fix remaining async build context risks; enable lint

kee-org · Aug 24, 2023 · e548695 · e548695
1 parent 527de3a
commit e548695
Show file tree

Hide file tree

Showing 11 changed files with 157 additions and 87 deletions.
diff --git a/analysis_options.yaml b/analysis_options.yaml
@@ -27,7 +27,7 @@ linter:
     # avoid_print: false  # Uncomment to disable the `avoid_print` rule
     prefer_single_quotes: true  # Uncomment to enable the `prefer_single_quotes` rule
     # prefer_double_quotes: true
-    use_build_context_synchronously: false # Too hard to create dialogs and do route navigation without a context across async boundary. Just have to hope it's OK until a better approach is possible.
+    use_build_context_synchronously: true
 
 # Additional information about this file can be found at
 # https://dart.dev/guides/language/analysis-options
@@ -40,3 +40,4 @@ analyzer:
     - lib/generated/**
     - lib/phonetic.dart
     - lib/generated_plugin_registrant.dart
+    - integration_test/** # until Patrol is no longer bundled with release code
diff --git a/lib/permissions.dart b/lib/permissions.dart
@@ -19,6 +19,9 @@ Future<PermissionResult> tryToGetPermission(
     PermissionStatus permissionStatus = await permission.status;
     if (permissionStatus == PermissionStatus.denied) {
       permissionStatus = await permission.request();
+      if (!context.mounted) {
+        return PermissionResult.rejected;
+      }
       if (permissionStatus == PermissionStatus.permanentlyDenied) {
         l.w('$permissionName permission permanently denied');
         if (await DialogUtils.showConfirmDialog(
@@ -31,12 +34,14 @@ Future<PermissionResult> tryToGetPermission(
             ))) {
           WidgetsBinding.instance.addPostFrameCallback((_) async {
             if (!await openAppSettings()) {
-              await DialogUtils.showSimpleAlertDialog(
-                context,
-                str.vaultStatusError,
-                str.permissionSettingsOpenError,
-                routeAppend: 'couldNotOpenPermissionSettings',
-              );
+              if (context.mounted) {
+                await DialogUtils.showSimpleAlertDialog(
+                  context,
+                  str.vaultStatusError,
+                  str.permissionSettingsOpenError,
+                  routeAppend: 'couldNotOpenPermissionSettings',
+                );
+              }
             }
           });
           // User should return later after changing settings so we can try again

diff --git a/lib/widgets/account_create.dart b/lib/widgets/account_create.dart
@@ -709,12 +709,19 @@ class _AccountCreateWidgetState extends State<AccountCreateWidget> {
       setState(() {
         saving = false;
       });
-      BlockingOverlay.of(context).hide();
+      try {
+        blockingOverlay.hide();
+      } on Exception {
+        if (context.mounted) {
+          BlockingOverlay.of(context).hide();
+        }
+      }
     }
   }
 
   registerAccount(String email, String password, bool marketingEmails) async {
-    BlockingOverlay.of(context).show(null, Duration(seconds: 1));
+    final blockingOverlay = BlockingOverlay.of(context);
+    blockingOverlay.show(null, Duration(seconds: 1));
     final accountCubit = BlocProvider.of<AccountCubit>(context);
     User user;
     try {
@@ -725,7 +732,13 @@ class _AccountCreateWidgetState extends State<AccountCreateWidget> {
         saveError = true;
         saving = false;
       });
-      BlockingOverlay.of(context).hide();
+      try {
+        blockingOverlay.hide();
+      } on Exception {
+        if (context.mounted) {
+          BlockingOverlay.of(context).hide();
+        }
+      }
       return;
     }
     await subscribeUser(user);
@@ -794,6 +807,8 @@ class AccountCreateWrapperWidget extends StatelessWidget {
             ),
           ),
           onWillPop: () async {
+            final vc = BlocProvider.of<VaultCubit>(context);
+            final ac = BlocProvider.of<AccountCubit>(context);
             final result = skipBackCheck ||
                 await showDialog(
                     routeSettings: RouteSettings(),
@@ -812,8 +827,7 @@ class AccountCreateWrapperWidget extends StatelessWidget {
                             ]));
             if (result) {
               // sign out so user can see initial signin/register page again.
-              final vc = BlocProvider.of<VaultCubit>(context);
-              await BlocProvider.of<AccountCubit>(context).forgetUser(vc.signout);
+              await ac.forgetUser(vc.signout);
               return true;
             }
             return false;

diff --git a/lib/widgets/binaries.dart b/lib/widgets/binaries.dart
@@ -234,8 +234,8 @@ class BinaryCardWidget extends StatelessWidget {
                       await file.create(recursive: true);
                       await file.writeAsBytes(bytes, flush: true);
                       try {
-                        final xfile = XFile(file.path, mimeType: mimeType);
-                        await Share.shareXFiles([xfile], subject: 'Kee Vault attachment');
+                        final xFile = XFile(file.path, mimeType: mimeType);
+                        await Share.shareXFiles([xFile], subject: 'Kee Vault attachment');
                       } finally {
                         await file.delete();
                       }
@@ -308,7 +308,11 @@ class BinaryCardWidget extends StatelessWidget {
                               ));
                             } on Exception catch (e, st) {
                               l.e('Export failed: $e', st);
-                              await DialogUtils.showErrorDialog(context, str.exportError, str.exportErrorDetails);
+                              if (context.mounted) {
+                                await DialogUtils.showErrorDialog(context, str.exportError, str.exportErrorDetails);
+                              } else {
+                                l.w('context was destroyed so could not notify user of previous error');
+                              }
                             }
                           }
                         });
@@ -325,7 +329,7 @@ class BinaryCardWidget extends StatelessWidget {
                             final proceed = await DialogUtils.showConfirmDialog(
                                 context: context,
                                 params: ConfirmDialogParams(content: str.attachmentConfirmDelete(attachment.key.key)));
-                            if (proceed) {
+                            if (proceed && context.mounted) {
                               _deleteFile(context, attachment.key);
                             }
                           });

diff --git a/lib/widgets/dialog_utils.dart b/lib/widgets/dialog_utils.dart
@@ -92,7 +92,7 @@ class DialogUtils {
     }
     if (!result) {
       final context = AppConfig.navigatorKey.currentContext;
-      if (context != null) {
+      if (context != null && context.mounted) {
         ScaffoldMessenger.of(context).showSnackBar(SnackBar(
           content: Text(S.of(context).urlOpenFailed),
           duration: Duration(seconds: 4),

diff --git a/lib/widgets/entry.dart b/lib/widgets/entry.dart
@@ -90,17 +90,26 @@ class EntryWidget extends StatelessWidget {
           type: FileType.any,
           withData: true,
         );
-        await FilePicker.platform.clearTemporaryFiles(); //TODO:f concurrently with below await operation
+        final cleanupFuture = FilePicker.platform.clearTemporaryFiles();
 
         final bytes = result?.files.firstOrNull?.bytes;
         if (bytes == null) {
           // User canceled the picker
+          await cleanupFuture;
           return;
         }
         final fileName = result?.files.firstOrNull?.name ?? UuidUtil.createNonCryptoUuid();
-        await _attachFileContent(context, fileName, bytes);
+        if (context.mounted) {
+          await _attachFileContent(context, fileName, bytes);
+        }
+        await cleanupFuture;
       } on Exception {
-        await DialogUtils.showErrorDialog(context, str.attachmentError, str.attachmentErrorDetails);
+        l.e('${str.attachmentError} ${str.attachmentErrorDetails}');
+        if (context.mounted) {
+          await DialogUtils.showErrorDialog(context, str.attachmentError, str.attachmentErrorDetails);
+        } else {
+          l.w('context was destroyed so could not notify user of previous error');
+        }
       }
     }
   }
@@ -192,6 +201,7 @@ class EntryWidget extends StatelessWidget {
           final barcodeResult = await barcode.BarcodeScanner.scan();
           if (barcodeResult.type == barcode.ResultType.Barcode) {
             final result = await cleanOtpCodeCode(barcodeResult.rawContent);
+            if (!context.mounted) break;
             if (result == null) {
               final tryAgain = await DialogUtils.showConfirmDialog(
                   context: context,
@@ -209,6 +219,7 @@ class EntryWidget extends StatelessWidget {
             }
           }
           if (barcodeResult.type == barcode.ResultType.Error) {
+            if (!context.mounted) break;
             final tryAgain = await DialogUtils.showConfirmDialog(
                 context: context,
                 params: ConfirmDialogParams(
@@ -239,6 +250,7 @@ class EntryWidget extends StatelessWidget {
     }
 
     while (true) {
+      if (!context.mounted) return null;
       final totpCode = await SimplePromptDialog(
               title: str.otpManualTitle,
               bodyText: str.otpExplainer1,
@@ -249,6 +261,7 @@ class EntryWidget extends StatelessWidget {
         return null;
       }
       final result = await cleanOtpCodeCode(totpCode);
+      if (!context.mounted) return null;
       if (result == null) {
         await DialogUtils.showSimpleAlertDialog(
           context,

diff --git a/lib/widgets/entry_field.dart b/lib/widgets/entry_field.dart
@@ -260,63 +260,71 @@ class _EntryTextFieldState extends _EntryFieldState implements FieldDelegate {
         // only used by [_OtpEntryFieldState]
         throw UnsupportedError('Field does not support this action.');
       case EntryAction.rename:
-        final cubit = BlocProvider.of<EntryCubit>(context);
-        final newName = await SimplePromptDialog(
-          title: str.renamingField,
-          labelText: str.renameFieldEnterNewName,
-          initialValue: widget.field.name,
-        ).show(context);
-        if (newName != null) {
-          cubit.renameField(widget.field.key, widget.field.browserModel?.displayName, newName);
+        // Think this is a use_build_context_synchronously false positive at least
+        // as of Dart 3.1 but a quick sanity check here does minimal harm
+        if (context.mounted) {
+          final cubit = BlocProvider.of<EntryCubit>(context);
+          final newName = await SimplePromptDialog(
+            title: str.renamingField,
+            labelText: str.renameFieldEnterNewName,
+            initialValue: widget.field.name,
+          ).show(context);
+          if (newName != null) {
+            cubit.renameField(widget.field.key, widget.field.browserModel?.displayName, newName);
+          }
         }
         break;
       case EntryAction.protect:
-        if (_isProtected) {
-          final cubit = BlocProvider.of<EntryCubit>(context);
-          if (widget.field.fieldStorage == FieldStorage.JSON) {
-            cubit.updateField(
-              null,
-              widget.field.browserModel!.displayName,
-              value: PlainValue(widget.field.textValue),
-              browserModel:
-                  widget.field.browserModel!.copyWith(type: FormFieldType.TEXT, value: widget.field.textValue),
-              protect: false,
-            );
-          } else {
-            cubit.updateField(
-              widget.field.key,
-              null,
-              value: PlainValue(widget.field.textValue),
-              protect: false,
-            );
-          }
-        } else {
-          final cubit = BlocProvider.of<EntryCubit>(context);
-          if (widget.field.fieldStorage == FieldStorage.JSON) {
-            cubit.updateField(
-              null,
-              widget.field.browserModel!.displayName,
-              value: ProtectedValue.fromString(widget.field.textValue),
-              browserModel:
-                  widget.field.browserModel!.copyWith(type: FormFieldType.PASSWORD, value: widget.field.textValue),
-              protect: true,
-            );
+        // Think this is a use_build_context_synchronously false positive at least
+        // as of Dart 3.1 but a quick sanity check here does minimal harm
+        if (context.mounted) {
+          if (_isProtected) {
+            final cubit = BlocProvider.of<EntryCubit>(context);
+            if (widget.field.fieldStorage == FieldStorage.JSON) {
+              cubit.updateField(
+                null,
+                widget.field.browserModel!.displayName,
+                value: PlainValue(widget.field.textValue),
+                browserModel:
+                    widget.field.browserModel!.copyWith(type: FormFieldType.TEXT, value: widget.field.textValue),
+                protect: false,
+              );
+            } else {
+              cubit.updateField(
+                widget.field.key,
+                null,
+                value: PlainValue(widget.field.textValue),
+                protect: false,
+              );
+            }
           } else {
-            cubit.updateField(
-              widget.field.key,
-              null,
-              value: ProtectedValue.fromString(widget.field.textValue),
-              protect: true,
-            );
+            final cubit = BlocProvider.of<EntryCubit>(context);
+            if (widget.field.fieldStorage == FieldStorage.JSON) {
+              cubit.updateField(
+                null,
+                widget.field.browserModel!.displayName,
+                value: ProtectedValue.fromString(widget.field.textValue),
+                browserModel:
+                    widget.field.browserModel!.copyWith(type: FormFieldType.PASSWORD, value: widget.field.textValue),
+                protect: true,
+              );
+            } else {
+              cubit.updateField(
+                widget.field.key,
+                null,
+                value: ProtectedValue.fromString(widget.field.textValue),
+                protect: true,
+              );
+            }
           }
+          setState(() {
+            if (_isProtected) {
+              _isValueObscured = false;
+            } else if (widget.field.textValue.isNotEmpty) {
+              _isValueObscured = true;
+            }
+          });
         }
-        setState(() {
-          if (_isProtected) {
-            _isValueObscured = false;
-          } else if (widget.field.textValue.isNotEmpty) {
-            _isValueObscured = true;
-          }
-        });
         break;
       case EntryAction.delete:
         widget.onDelete();

diff --git a/lib/widgets/entry_history.dart b/lib/widgets/entry_history.dart
@@ -68,6 +68,9 @@ class EntryHistoryWidget extends StatelessWidget {
                                     return;
                                   }
                                 }
+                                if (!context.mounted) {
+                                  return;
+                                }
                                 final proceed = await DialogUtils.showConfirmDialog(
                                     context: context,
                                     params: ConfirmDialogParams(