florent37 / InlineActivityResult

Receive the activity result directly after the startActivityForResult with InlineActivityResult
MIT License
273 stars 28 forks source link

No countermeasure for Activity recreation when the screen rotates #17

Open BoxResin opened 3 years ago

BoxResin commented 3 years ago

I found a trivial bug. rx and coroutine packages are affected by this.

Scenario

  1. A Activity starts B Activity.
  2. Rotate the screen in B Activity.
  3. Tap the back button in B Activity.
  4. A Activity will lost the activity result from B Activity.

Activitys are recreated when the screen rotates as we know. Starting another Activity is also the case. If B Activity has been rotated, then A Activity get recreated.(It's deferred until B Activity is finished) In this case, A Activity calling startActivityForResult() never get callback for onActivityResult(). (It would be called in another instance of A Activity).

I attach a video that actually reproduces the bug.

https://user-images.githubusercontent.com/13031505/104625606-42e28c00-56d8-11eb-9213-39f8401cbd84.mp4

BoxResin commented 3 years ago

This feature may fix the issue.

BoxResin commented 3 years ago

This documentation also maybe helpful.

victor-denisenko commented 3 years ago

Hello.

This documentation also maybe helpful.

If you want to use the new Activity Result API, then you don't need this library, IMO.

This feature may fix the issue.

Yes, shared ViewModel can store com.github.florent37.inlineactivityresult.Result value. But we need to store com.github.florent37.inlineactivityresult.InlineActivityResult callbacks also and in this case it will cause memory leak. We can use LiveData observer to get a result in activity/fragment, but it breaks RX chain logic (RX subscriber will be unused).

In simple words, the library needs to store some internal callbacks and with screen rotation it must be to resubscribed to a new observer (activity/fragment). ViewModel won't help (without huge rewriting of implementation). The new Activity Result API solve this problem in the other way using some private methods in activity. I don't found RX wrapper for Activity Result API lib (but its necessary? probably no).

BoxResin commented 3 years ago

@victor-denisenko Thanks for your opinions.