Is it safe to commit a FragmentTransaction inside onActivityResult()?
Asked Answered
P

1

13

Years ago, I ran into a problem in one of my apps when I tried to commit a FragmentTransaction inside my onActivityResult() callback. Googling around, I found this question and answer, which say

At the time that onActivityResult() is called, the activity/fragment's state may not yet have been restored, and therefore any transactions that happen during this time will be lost as a result.

I wound up adapting the solution recommended in the same answer for my app, and life was good. However, recent experimentation shows that perhaps things have changed, and it may now be safe to commit a FragmentTransaction from inside onActivityResult().

The documentation for (support v4) FragmentManager.beginTransaction() defines the safe window for transactions as:

Note: A fragment transaction can only be created/committed prior to an activity saving its state. If you try to commit a transaction after FragmentActivity.onSaveInstanceState() (and prior to a following FragmentActivity.onStart or FragmentActivity.onResume(), you will get an error.

Reading the documentation for onActivityResult(), I see

You will receive this call immediately before onResume() when your activity is re-starting.

This leads me to believe that it should be safe to execute these transactions in onActivityResult(), as onStart() will have already been called, putting me inside the safe window.

I made an app to test this out, and I'm successfully seeing dialog fragments that I create and commit inside onActivityResult(). I had the same app also log the activity lifecycle callbacks so I could inspect their order, and I see onStart(), then onRestoreInstanceState(), and then onActivityResult() every time.

Am I missing something? Or has the framework changed and onActivityResult() is now guaranteed to be a safe place for fragment transactions? Does this behavior vary by API level?

I found another question and answer that seems to read the documentation the same way I have, but both are over a year old and neither specifically refers to onActivityResult() as a safe place for transactions.

Pris answered 20/11, 2017 at 22:51 Comment(2)
Take a look at androiddesignpatterns.com/2013/08/…Elwina
@Elwina That blog post is one of the materials I found back when I first had this issue. However, as illustrated by azizbekian's answer below, it seems that it is no longer an accurate explanation of the safe windows to commit FragmentTransactions.Pris
B
18

A little dive into sources

There is a boolean variable inside FragmentManager class, called mStatedSaved. This variable keeps track of the saved state depending on the activity's lifecycle callbacks. Here's the method, that throws well-known exception:


    private void checkStateLoss() {
        if (mStateSaved) {
            throw new IllegalStateException(
                    "Can not perform this action after onSaveInstanceState");
        }
        ...
    }

This means, that as soon as this variable is changed to false, then fragment transactions are free to be executed. This variable would become true, when the state of the activity is being saved, i.e. onSaveInstanceState().


Back to the question

You said, that previously you had problems commiting a transaction from onActivityResult(). That should mean, that previously mStateSaved was not being assigned false and currently it is. In deed it is so.

Here's onActivityResult() implementation from O release:


    @Override
    protected void onActivityResult(int requestCode, int resultCode, Intent data) {
        mFragments.noteStateNotSaved();
        ...
    }

Where, noteStateNotSaved() would do following:


    public void noteStateNotSaved() {
        ...
        mStateSaved = false;
        ...
    }

On the contrary, you can see the implementation of onActivityResult() of Jelly Bean release:


    @Override
    protected void onActivityResult(int requestCode, int resultCode, Intent data) {
        int index = requestCode>>16;
        if (index != 0) {
            index--;
            if (mFragments.mActive == null || index = mFragments.mActive.size()) {
                Log.w(TAG, "Activity result fragment index out of range: 0x"
                        + Integer.toHexString(requestCode));
                return;
            }
            Fragment frag = mFragments.mActive.get(index);
            if (frag == null) {
                Log.w(TAG, "Activity result no fragment exists for index: 0x"
                        + Integer.toHexString(requestCode));
            } else {
                frag.onActivityResult(requestCode&0xffff, resultCode, data);
            }
            return;
        }

        super.onActivityResult(requestCode, resultCode, data);
    }

Nothing would change the value of mStateSaved field, which would make an exception to be thrown if a transaction is committed.

In fact, the line mFragments.noteStateNotSaved() was introduced in Kit-Kat release. As you can see by the commit's comment made by Dianne Hackborn:

ActivityFragment should clear the flag that state is saved when it receives onNewIntent(). This can happen before the activity is resumed, so we may not have cleared it yet. Also need to do the same thing for onActivityResult().

Sum up

Is onActivityResult() now guaranteed to be a safe place for fragment transactions?

Assuming you are using sources that include commit 4ccc001, which was made in October 2012 - yes, it is a safe place for fragment transactions.

Brandt answered 30/11, 2017 at 10:10 Comment(6)
Reading answers like this one is a pure joy. Thank you.Noonan
Thanks, @Vasiliy, enjoying your answers/talks also.Brandt
@Brandt Of course I want to echo the other commenters that this was a great answer, but I also want to say that the only reason I didn't instantly accept + award is that I wanted to wait and see if anyone else chimed in. Thanks for being so detailed; it is greatly appreciated.Pris
@Brandt Thanks for the such deep explanation . By reading the code i assume the super.onActivityResult should be first line inside onActivityResult not the last cause this is call which set the mStateSaved to false .. Am i Right ?Transfix
@ADM, unless you have a specific reason for not putting super call in the top, I cannot see why you should move it to the bottom. Thanks.Brandt
Yeah .. I don't have any specific reason . But i have seen in lots of Code some of mine too (old time) .. The code above also has it at bottom .. Thanks that's Clear enough.. Super call should be first ..Transfix

© 2022 - 2024 — McMap. All rights reserved.