fix(cloud_firestore): Fix Android Firestore transaction crash when running in background caused by null Activity.

[TimurDyushaliev]

Description

Hey! We are facing an error from Crashlytics (Android only) on many of our users. I could reproduce this by my own with running a transaction on background mode. Logs look like this:

E/AndroidRuntime( 6305): java.lang.NullPointerException: Attempt to invoke virtual method 'void android.app.Activity.runOnUiThread(java.lang.Runnable)' on a null object reference
E/AndroidRuntime( 6305): at io.flutter.plugins.firebase.firestore.streamhandler.TransactionStreamHandler.lambda$onListen$3$TransactionStreamHandler(TransactionStreamHandler.java:152)
E/AndroidRuntime( 6305): at io.flutter.plugins.firebase.firestore.streamhandler.-$$Lambda$TransactionStreamHandler$LIvgodBqRTFvU0xL0wKf2TWFG_k.onComplete(Unknown Source:6)
E/AndroidRuntime( 6305): at com.google.android.gms.tasks.zzj.run(Unknown Source:4)
E/AndroidRuntime( 6305): at android.os.Handler.handleCallback(Handler.java:938)
E/AndroidRuntime( 6305): at android.os.Handler.dispatchMessage(Handler.java:99)
E/AndroidRuntime( 6305): at android.os.Looper.loopOnce(Looper.java:201)
E/AndroidRuntime( 6305): at android.os.Looper.loop(Looper.java:288)
E/AndroidRuntime( 6305): at android.app.ActivityThread.main(ActivityThread.java:7829)
E/AndroidRuntime( 6305): at java.lang.reflect.Method.invoke(Native Method)
E/AndroidRuntime( 6305): at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
E/AndroidRuntime( 6305): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:982)

So this PR fixes this with simply adding a null-check operator:

if (activityRef.get() != null)

before using the runOnUiThread method:

if (activityRef.get() != null) {
activityRef.get().runOnUiThread(() -> events.success(map));
activityRef.get().runOnUiThread(events::endOfStream);
}

To ensure the runOnUiThread method will not be called from a null reference. This changes helped for me and the bug is gone.

I've also read the similar issues linked below (they are all closed, but I didn't find any solution):

5914
6374

Hopefully you can review this PR and if something is wrong I would be glad to hear this:)

I hope this will help for many as well as for me. Thank you!

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (melos run analyze) does not report any problems on my PR.
• I read and followed the Flutter Style Guide.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change.
• No, this is not a breaking change.

[russellwheatley]

There's also another reference to runOnUiThread that should probably be removed as well here

[russellwheatley]

Suggested change

	if (activityRef.get() != null) {
	activityRef.get().runOnUiThread(() -> events.success(map));
	activityRef.get().runOnUiThread(events::endOfStream);
	}
	events.success(map);
	events.endOfStream();

[russellwheatley]

Doesn't seem necessary to runOnUiThread given it will run on the UI thread anyway according to Flutter docs

[TimurDyushaliev]

removing the activityRef.get().runOnUiThread gives an error every time, when i run a transaction:

in our project inside the catchError method:
[cloud_firestore/null] null

in a demo project:

[ERROR:flutter/lib/ui/ui_dart_state.cc(209)] Unhandled Exception: type 'Null' is not a subtype of type 'String'
E/flutter ( 5826): #0 MethodChannelFirebaseFirestore.runTransaction.
package:cloud_firestore_platform_interface/…/method_channel/method_channel_firestore.dart:248
E/flutter ( 5826): #1 MethodChannelFirebaseFirestore.runTransaction.
package:cloud_firestore_platform_interface/…/method_channel/method_channel_firestore.dart:243
E/flutter ( 5826): #2 _rootRunUnary (dart:async/zone.dart:1436:47)
E/flutter ( 5826): #3 _CustomZone.runUnary (dart:async/zone.dart:1335:19)
I/DynamiteModule( 5826): Considering local module providerinstaller:0 and remote module providerinstaller:0
W/ProviderInstaller( 5826): Failed to load providerinstaller module: No acceptable module found. Local version is 0 and remote version is 0.
D/nativeloader( 5826): classloader namespace configured for unbundled product apk. library_path=/product/priv-app/PrebuiltGmsCore/lib/arm64:/product/priv-app/PrebuiltGmsCore/PrebuiltGmsCore.apk!/lib/arm64-v8a:/product/lib64:/system/product/lib64
V/NativeCrypto( 5826): Registering com/google/android/gms/org/conscrypt/NativeCrypto's 294 native methods...
W/ransaction_dem( 5826): Accessing hidden method Ljava/security/spec/ECParameterSpec;->getCurveName()Ljava/lang/String; (unsupported, reflection, allowed)
I/ProviderInstaller( 5826): Installed default security provider GmsCore_OpenSSL
W/ransaction_dem( 5826): Accessing hidden field Ljava/net/Socket;->impl:Ljava/net/SocketImpl; (unsupported, reflection, allowed)
W/ransaction_dem( 5826): Accessing hidden field Ljava/io/FileDescriptor;->descriptor:I (unsupported, JNI, allowed)
E/flutter ( 5826): #4 _CustomZone.runUnaryGuarded (dart:async/zone.dart:1244:7)
E/flutter ( 5826): #5 _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:341:11)
E/flutter ( 5826): #6 _DelayedData.perform (dart:async/stream_impl.dart:591:14)
E/flutter ( 5826): #7 _StreamImplEvents.handleNext (dart:async/stream_impl.dart:706:11)
E/flutter ( 5826): #8 _PendingEvents.schedule. (dart:async/stream_impl.dart:663:7)
E/flutter ( 5826): #9 _rootRun (dart:async/zone.dart:1420:47)
E/flutter ( 5826): #10 _CustomZone.run (dart:async/zone.dart:1328:19)
E/flutter ( 5826): #11 _CustomZone.runGuarded (dart:async/zone.dart:1236:7)
E/flutter ( 5826): #12 _CustomZone.bindCallbackGuarded. (dart:async/zone.dart:1276:23)
E/flutter ( 5826): #13 _rootRun (dart:async/zone.dart:1428:13)
E/flutter ( 5826): #14 _CustomZone.run (dart:async/zone.dart:1328:19)
E/flutter ( 5826): #15 _CustomZone.runGuarded (dart:async/zone.dart:1236:7)
E/flutter ( 5826): #16 _CustomZone.bindCallbackGuarded. (dart:async/zone.dart:1276:23)
E/flutter ( 5826): #17 _microtaskLoop (dart:async/schedule_microtask.dart:40:21)
E/flutter ( 5826): #18 _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)

(I didn't fully understand why the demo project could not catch this inside the catchError method, but anyway this shows us, that removing this method is very dangerous)

[Salakar]

Doesn't seem necessary to runOnUiThread given it will run on the UI thread anyway according to Flutter docs

This is the Dart/Flutter UI code, afaik this does not apply to plugin native code and that is ran off the UI thread.

I think the best solution here is to remove a requirement to hold the Activity entirely and just use the main looper directly like we do everywhere else, e.g. on RTDB transaction: https://github.com/FirebaseExtended/flutterfire/blob/eaf0af6c4b0d418aa36d7dd2cd8cd60e40b06ebf/packages/firebase_database/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/TransactionExecutor.java#L24-L26

[russellwheatley]

Hey @TimurDyushaliev, as suggested above, please remove activityRef & use the Handler implementation:

new Handler(Looper.getMainLooper())

This will always provide the context of the main activity instead of checking if the activity exists and the possibility of missing an event.

[TimurDyushaliev]

Yeah, this solution is great and works well. Thank you guys for helping me:) Can you please check my last updates?

I hope I did as you mean and if not I'm open to refactor. And because it's my first ever contributing, I've also added my name to AUTHORS file as written in Contributor Guide.

[russellwheatley]

Thanks for the update and the contribution, @TimurDyushaliev!

[russellwheatley]

LGTM