Fixes #1555

[VishnuSanal]

Description

Issue tracker

Fixes #1555

Manual tests

• Done

Build tasks success

Successfully running following tasks on local:

• ./gradlew assembledebug
• ./gradlew spotlessCheck

[EmmanuelMess]

This needs to be fixed, it would get a missing tab:

   try {
        if (fragmentManager == null) {
          fragmentManager = getActivity().getSupportFragmentManager();
        }
        fragments.add(0, fragmentManager.getFragment(savedInstanceState, "tab" + 0));
        fragments.add(1, fragmentManager.getFragment(savedInstanceState, "tab" + 1));
   } catch (Exception e) {
        e.printStackTrace();
   }
 

And remove the generic Exception.

[VishnuSanal]

it would get a missing tab

How?

I tried doing this:

if (!fragment.isAdded())
     fragmentManager.beginTransaction().add(fragment, "tab" + i).commit();

     fragmentManager.putFragment(outState, "tab" + i, fragment);

But it gave me the following memory leak:

┬───
│ GC Root: System class
│
├─ android.view.inputmethod.InputMethodManager class
│    Leaking: NO (InputMethodManager↓ is not leaking and a class is never
│    leaking)
│    ↓ static InputMethodManager.sInstance
├─ android.view.inputmethod.InputMethodManager instance
│    Leaking: NO (DecorView↓ is not leaking and InputMethodManager is a
│    singleton)
│    ↓ InputMethodManager.mCurRootView
├─ com.android.internal.policy.DecorView instance
│    Leaking: NO (LinearLayout↓ is not leaking and View attached)
│    View is part of a window view hierarchy
│    View.mAttachInfo is not null (view attached)
│    View.mWindowAttachCount = 1
│    mContext instance of com.android.internal.policy.DecorContext, wrapping
│    activity com.amaze.filemanager.ui.activities.MainActivity with mDestroyed
│    = false
│    ↓ DecorView.mContentRoot
├─ android.widget.LinearLayout instance
│    Leaking: NO (MainActivity↓ is not leaking and View attached)
│    View is part of a window view hierarchy
│    View.mAttachInfo is not null (view attached)
│    View.mWindowAttachCount = 1
│    mContext instance of com.amaze.filemanager.ui.activities.MainActivity with
│    mDestroyed = false
│    ↓ View.mContext
├─ com.amaze.filemanager.ui.activities.MainActivity instance
│    Leaking: NO (LinearLayout↓ is not leaking and Activity#mDestroyed is false)
│    mainActivity instance of com.amaze.filemanager.ui.activities.MainActivity
│    with mDestroyed = false
│    mApplication instance of com.amaze.filemanager.application.AppConfig
│    mBase instance of androidx.appcompat.view.ContextThemeWrapper
│    ↓ MainActivity.indicator_layout
├─ android.widget.LinearLayout instance
│    Leaking: NO (Indicator↓ is not leaking and View attached)
│    View is part of a window view hierarchy
│    View.mAttachInfo is not null (view attached)
│    View.mID = R.id.indicator_layout
│    View.mWindowAttachCount = 1
│    mContext instance of com.amaze.filemanager.ui.activities.MainActivity with
│    mDestroyed = false
│    ↓ ViewGroup.mChildren
├─ android.view.View[] array
│    Leaking: NO (Indicator↓ is not leaking)
│    ↓ View[].[0]
├─ com.amaze.filemanager.ui.views.Indicator instance
│    Leaking: NO (View attached)
│    View is part of a window view hierarchy
│    View.mAttachInfo is not null (view attached)
│    View.mID = R.id.indicator
│    View.mWindowAttachCount = 1
│    mContext instance of com.amaze.filemanager.ui.activities.MainActivity with
│    mDestroyed = false
│    ↓ Indicator.viewPager
│                ~~~~~~~~~
├─ com.amaze.filemanager.ui.views.DisablableViewPager instance
│    Leaking: UNKNOWN
│    Retaining 87.2 kB in 1390 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mID = R.id.pager
│    View.mWindowAttachCount = 1
│    mContext instance of com.amaze.filemanager.ui.activities.MainActivity with
│    mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
╰→ androidx.constraintlayout.widget.ConstraintLayout instance
      Leaking: YES (ObjectWatcher was watching this because com.amaze.
      filemanager.ui.fragments.TabFragment received Fragment#onDestroyView()
      callback (references to its views should be cleared to prevent leaks))
      Retaining 2.5 kB in 59 objects
      key = e3f65561-803e-4f17-87c5-2e79d6be6037
      watchDurationMillis = 14584
      retainedDurationMillis = 9581
      View not part of a window view hierarchy
      View.mAttachInfo is null (view detached)
      View.mWindowAttachCount = 1
      mContext instance of com.amaze.filemanager.ui.activities.MainActivity
      with mDestroyed = false

METADATA

Build.VERSION.SDK_INT: 29
Build.MANUFACTURER: realme
LeakCanary version: 2.6
App process name: com.amaze.filemanager.debug
Stats: LruCache[maxSize=3000,hits=7565,misses=94493,hitRate=7%]
RandomAccess[bytes=5536529,reads=94493,travel=31931450462,range=23362279,size=28
557628]
Heap dump reason: user request
Analysis duration: 8640 ms

What do you think 🤔

Also, from Telegram group, ICYMI

Ref: Your comment on 2772

And if the fragment is not added? Doesn't this just skip putting it?

Yes, we're saving that. Right? So isn't that okay to skip if it is not present in the FragmentManager? (Reference (https://stackoverflow.com/a/26285801/9652621))

[EmmanuelMess]

it would get a missing tab

How?

It would get a missing tab if you call fragmentManager.getFragment(savedInstanceState, "tab" + 0) on a missing key

[EmmanuelMess]

if (!fragment.isAdded())
     fragmentManager.beginTransaction().add(fragment, "tab" + i).commit();

     fragmentManager.putFragment(outState, "tab" + i, fragment);

I don't understand why you try to add the fragment twice here. Also, the leak seems not related.

[VishnuSanal]

if (!fragment.isAdded())
     fragmentManager.beginTransaction().add(fragment, "tab" + i).commit();

     fragmentManager.putFragment(outState, "tab" + i, fragment);

I don't understand why you try to add the fragment twice here. Also, the leak seems not related.

Twice? I was adding the fragment if that was not added since you asked it here

And if the fragment is not added? Doesn't this just skip putting it?

& here

it would get a missing tab

How?

It would get a missing tab if you call fragmentManager.getFragment(savedInstanceState, "tab" + 0) on a missing key

[EmmanuelMess]

if (!fragment.isAdded())
     fragmentManager.beginTransaction().add(fragment, "tab" + i).commit();

     fragmentManager.putFragment(outState, "tab" + i, fragment);

I don't understand why you try to add the fragment twice here. Also, the leak seems not related.

Twice? I was adding the fragment if that was not added since you asked it here

Sorry my mistake, why are you adding the fragment here though? I meant for you to add it if its not in the bundle when the fragment is recreated.

[EmmanuelMess]

it would get a missing tab

How?

It would get a missing tab if you call fragmentManager.getFragment(savedInstanceState, "tab" + 0) on a missing key

this still happens AFAIK in this solution:

if (!fragment.isAdded())
     fragmentManager.beginTransaction().add(fragment, "tab" + i).commit();

     fragmentManager.putFragment(outState, "tab" + i, fragment);

[VishalNehra]

This will conflict with #2780 (edit by EmmanuelMess) #2789

[EmmanuelMess]

How does this fix the issue?

[VishalNehra]

This will force execute any pending transactions for the fragment manager. So by the time we reach to put fragments in instance state we have both of them.

[EmmanuelMess]

This should work