Fix: Prevent crash when pausing/resuming uploads after screen rotation

[Kota-Jagadeesh]

Description (required)

Fixes #6433

What changes did you make and why?

This PR resolves a critical IllegalStateException crash that occurred when a user triggered a configuration change (e.g., screen rotation) while on the upload progress screen, and then quickly tapped the "Pause uploads" or "Resume uploads" menu item.

The crash was due to a timing mismatch between the Activity, the Fragment lifecycle, and Dagger dependency injection: the host UploadProgressActivity was calling methods on the PendingUploadsFragment before the Fragment's dependencies, specifically the pendingUploadsPresenter, were re-injected by Dagger after rotation.

The following changes implement robust safety mechanisms for tha xternal Fragment interaction:

1. PendingUploadsFragment.kt:

   • Changed the injected property pendingUploadsPresenter to be nullable (@Inject @JvmField var presenter: Presenter? = null). This prevents the initial UninitializedPropertyAccessException and the subsequent custom IllegalStateException we added.
   • Updated all public action methods (pauseUploads(), restartUploads(), deleteUploads()) to use safe calls (presenter?.action()). This ensures the app gracefully aborts the action if the presenter is not yet ready, making the functionality smooth instead of crashy.

2. UploadProgressActivity.kt:

   • Removed the reliance on potentially stale private fields (pendingUploadsFragment).
   • Introduced a helper method (getPendingUploadsFragment()) that uses supportFragmentManager.findFragmentByTag() right before any fragment method is called. This guarantees the Activity is always interacting with the current, fully attached, and ready Fragment instance, regardless of rotation timing.

Tests performed (required)

Tested ProdDebug on Redmi note 13 5g with API level 35.

Steps to verify the fix:

1. Open the app and start 2-3 pending uploads.
2. Rotate the device (Portrait / Landscape).
3. Quickly tap the "Pause uploads" or "Resume uploads" menu icon multiple times.
4. Expected: The upload state changes and the app does not crash.

[github-actions]

✅ Generated APK variants!

• 🤖 Download betaDebugAPK
• 🤖 Download prodDebugAPK

[RitikaPahwa4444]

Haven't reviewed yet but just thinking if we could get a similar crash in FailedUploadsPresenter due to the same reason 🤔

[RitikaPahwa4444]

Verified the crash in FailedUploadsPresenter (not on this branch). Open question: why can't dagger help instantiate injected properties?

PS: this is for me to dig in, just brushing up as it's been a while since I worked on Android :)

[Kota-Jagadeesh]

Haven't reviewed yet but just thinking if we could get a similar crash in FailedUploadsPresenter due to the same reason 🤔

Yes, you are absolutely correct. The FailedUploadsFragment is structured similarly, using a Dagger-injected presenter (FailedUploadsPresenter), and the host UploadProgressActivity interacts with it via menu clicks in the same way. This means the same rotation-timing issue exists there. The fix implemented here (making the fragment methods safe and retrieving the fragment by tag in the Activity) should be applied to FailedUploadsFragment.kt and its methods in UploadProgressActivity.kt for a complete, robust solution.

[Kota-Jagadeesh]

Verified the crash in FailedUploadsPresenter (not on this branch). Open question: why can't dagger help instantiate injected properties?

PS: this is for me to dig in, just brushing up as it's been a while since I worked on Android :)

That's the heart of the issue with configuration changes in Android! Dagger uses the onAttach() or onActivityCreated() lifecycle phases of the fragment (usually handled by the base class) to perform injection. When the screen rotates:

• The Activity re-creates and calls methods on the Fragment manager.
• The Fragment re-creates.
• The menu click happens extremely fast.
• The external call from the Activity (e.g., restartUploads()) often fires after the Fragment re-creates but before Dagger's internal injection mechanism has successfully wired up the @Inject field.

[RitikaPahwa4444]

I was referring to the presenter variable with @Inject annotation that caused the crash in this issue and not "FailedUploadsPresenter". There's no FailedUploadsPresenter variable injected in the code base 😅 Also, while I understand making a field nullable helps fix the crash, the code base probably handles configuration changes differently. That's what I meant in my second comment but I've not tested those things out yet in this scenario, so I would refrain from suggesting that as an approach at the moment.

[Kota-Jagadeesh]

I was referring to the presenter variable with @Inject annotation that caused the crash in this issue and not "FailedUploadsPresenter". There's no FailedUploadsPresenter variable injected in the code base 😅 Also, while I understand making a field nullable helps fix the crash, the code base probably handles configuration changes differently. That's what I meant in my second comment but I've not tested those things out yet in this scenario, so I would refrain from suggesting that as an approach at the moment.

My mistake for assuming that name; I meant the presenter for the Failed Uploads tab, whatever its name is (since the fragments are handled similarly). Since you verified the crash on the failed uploads side, the approach we used to make the PendingUploadsFragment safe-retrieving the Fragment by tag in the Activity and using safe calls (?.) in the Fragment-will fix that side too.

---

I agree. The core issue is that the Activity can get a hold of the Fragment instance before Dagger's injection process is guaranteed to complete after a rotation. The codebase's standard way of handling configuration changes might be robust for UI/state, but this specific sequence (Rotation -> Fast Menu Click -> External call to a Fragment method) creates a tiny window where the @Inject field is still null.

[RitikaPahwa4444]

Would you mind checking how the app handles configuration changes in other activities?

[Kota-Jagadeesh]

Would you mind checking how the app handles configuration changes in other activities?

I’ll be a bit busy until the end of the month as my mid-sem exams are going on. I’ll need some time before I can get back to this issue.
Thank you

[RitikaPahwa4444]

No worries, @Kota-Jagadeesh. All the best for your exams :)

[RitikaPahwa4444]

Just wanted to update before you end up resuming your work on this... I raised an issue with a broader scope to standardise how we handle configuration changes: #6538. Please feel free to pick that up if you dig into the nuances to answer my question above but since I brought that up later, you need not handle all the scenarios as part of this PR as long as we're following the best practices even with limited scope 🙌🏻

In case you decide to continue with this one, I'll simplify it: I waited for quite some time before hitting that pause button but still ended up with a crash. So this might not be a timing issue or a race condition. We are possibly not persisting the state at all. I'll narrow down my question as the other one had a much broader scope: could you confirm if making properties nullable is a recommended way while injecting properties like a presenter and share a link to the official docs/stack overflow/blog, if possible? When is this property expected to be null?