From 0f9c20c986d8a58f48990fff57c03323b6e9ca70 Mon Sep 17 00:00:00 2001 From: devlearner Date: Fri, 2 Dec 2022 09:25:06 +0000 Subject: [PATCH] Improve (un)registering FragmentLifecycleCallbacks to avoid adding it multiple times and ensure proper cleanup --- .../org/schabi/newpipe/RouterActivity.java | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index afc4321c1..35bb8612f 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -32,6 +32,7 @@ import androidx.appcompat.content.res.AppCompatResources; import androidx.core.app.NotificationCompat; import androidx.core.app.ServiceCompat; import androidx.core.math.MathUtils; +import androidx.fragment.app.DialogFragment; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; import androidx.preference.PreferenceManager; @@ -113,6 +114,7 @@ public class RouterActivity extends AppCompatActivity { private boolean selectionIsDownload = false; private boolean selectionIsAddToPlaylist = false; private AlertDialog alertDialogChoice = null; + private FragmentManager.FragmentLifecycleCallbacks dismissListener = null; @Override protected void onCreate(final Bundle savedInstanceState) { @@ -142,6 +144,27 @@ public class RouterActivity extends AppCompatActivity { super.onCreate(savedInstanceState); Icepick.restoreInstanceState(this, savedInstanceState); + // FragmentManager will take care to recreate (Playlist|Download)Dialog when screen rotates + // We used to .setOnDismissListener(dialog -> finish()); when creating these DialogFragments + // but those callbacks won't survive a config change + // Try an alternate approach to hook into FragmentManager instead, to that effect + // (ref: https://stackoverflow.com/a/44028453) + final FragmentManager fm = getSupportFragmentManager(); + if (dismissListener == null) { + dismissListener = new FragmentManager.FragmentLifecycleCallbacks() { + @Override + public void onFragmentDestroyed(@NonNull final FragmentManager fm, + @NonNull final Fragment f) { + super.onFragmentDestroyed(fm, f); + if (f instanceof DialogFragment && fm.getFragments().isEmpty()) { + // No more DialogFragments, we're done + finish(); + } + } + }; + } + fm.registerFragmentLifecycleCallbacks(dismissListener, false); + if (TextUtils.isEmpty(currentUrl)) { currentUrl = getUrl(getIntent()); @@ -171,29 +194,10 @@ public class RouterActivity extends AppCompatActivity { protected void onStart() { super.onStart(); - // FragmentManager will take care to recreate DialogFragments when screen rotates - // currently that's namely PlaylistDialog or DownloadDialog - // We used to .setOnDismissListener(dialog ->finish()); when creating those Dialogs - // but those callbacks won't survive a config change - // Try an alternate approach to hook into FragmentManager instead, to that effect - // (courtesy of https://stackoverflow.com/a/44028453) - final FragmentManager fm = getSupportFragmentManager(); - fm.registerFragmentLifecycleCallbacks(new FragmentManager.FragmentLifecycleCallbacks() { - @Override - public void onFragmentViewDestroyed(@NonNull final FragmentManager fm, - @NonNull final Fragment f) { - super.onFragmentViewDestroyed(fm, f); - if (fm.getFragments().isEmpty()) { - // No more Dialog, we're done - finish(); - } - } - }, false); - // Don't overlap the DialogFragment after rotating the screen // If there's no DialogFragment, we're either starting afresh // or we didn't make it to PlaylistDialog or DownloadDialog before the orientation change - if (fm.getFragments().isEmpty()) { + if (getSupportFragmentManager().getFragments().isEmpty()) { // Start over from scratch handleUrl(currentUrl); } @@ -203,6 +207,9 @@ public class RouterActivity extends AppCompatActivity { protected void onDestroy() { super.onDestroy(); + if (dismissListener != null) { + getSupportFragmentManager().unregisterFragmentLifecycleCallbacks(dismissListener); + } disposables.clear(); }