Merge pull request TeamNewPipe#8248 from dtcxzyw/fix-readd-to-playlist

Fix inconsistency between user interaction and database commit order when re-adding videos to the playlist

Author: Stypox

app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java

@@ -38,6 +38,7 @@
 import org.schabi.newpipe.databinding.FragmentMainBinding;
 import org.schabi.newpipe.error.ErrorUtil;
 import org.schabi.newpipe.extractor.exceptions.ExtractionException;
+import org.schabi.newpipe.local.playlist.LocalPlaylistFragment;
 import org.schabi.newpipe.settings.tabs.Tab;
 import org.schabi.newpipe.settings.tabs.TabsManager;
 import org.schabi.newpipe.util.NavigationHelper;
@@ -217,6 +218,12 @@ private void updateTitleForTab(final int tabPosition) {
         setTitle(tabsList.get(tabPosition).getTabName(requireContext()));
     }
 
+    public void commitPlaylistTabs() {
+        pagerAdapter.getLocalPlaylistFragments()
+                .stream()
+                .forEach(LocalPlaylistFragment::commitChanges);
+    }
+
     private void updateTabLayoutPosition() {
         final ScrollableTabLayout tabLayout = binding.mainTabLayout;
         final ViewPager viewPager = binding.pager;
@@ -268,10 +275,18 @@ public void onTabReselected(final TabLayout.Tab tab) {
         updateTitleForTab(tab.getPosition());
     }
 
-    private static final class SelectedTabsPagerAdapter
+    public static final class SelectedTabsPagerAdapter
             extends FragmentStatePagerAdapterMenuWorkaround {
         private final Context context;
         private final List<Tab> internalTabsList;
+        /**
+         * Keep reference to LocalPlaylistFragments, because their data can be modified by the user
+         * during runtime and changes are not committed immediately. However, in some cases,
+         * the changes need to be committed immediately by calling
+         * {@link LocalPlaylistFragment#commitChanges()}.
+         * The fragments are removed when {@link LocalPlaylistFragment#onDestroy()} is called.
+         */
+        private final List<LocalPlaylistFragment> localPlaylistFragments = new ArrayList<>();
 
         private SelectedTabsPagerAdapter(final Context context,
                                          final FragmentManager fragmentManager,
@@ -298,9 +313,17 @@ public Fragment getItem(final int position) {
                 ((BaseFragment) fragment).useAsFrontPage(true);
             }
 
+            if (fragment instanceof LocalPlaylistFragment) {
+                localPlaylistFragments.add((LocalPlaylistFragment) fragment);
+            }
+
             return fragment;
         }
 
+        public List<LocalPlaylistFragment> getLocalPlaylistFragments() {
+            return localPlaylistFragments;
+        }
+
         @Override
         public int getItemPosition(@NonNull final Object object) {
             // Causes adapter to reload all Fragments when

app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java

@@ -54,6 +54,7 @@
 import androidx.appcompat.widget.Toolbar;
 import androidx.coordinatorlayout.widget.CoordinatorLayout;
 import androidx.core.content.ContextCompat;
+import androidx.fragment.app.Fragment;
 import androidx.preference.PreferenceManager;
 
 import com.google.android.exoplayer2.PlaybackException;
@@ -84,11 +85,13 @@
 import org.schabi.newpipe.fragments.BackPressable;
 import org.schabi.newpipe.fragments.BaseStateFragment;
 import org.schabi.newpipe.fragments.EmptyFragment;
+import org.schabi.newpipe.fragments.MainFragment;
 import org.schabi.newpipe.fragments.list.comments.CommentsFragment;
 import org.schabi.newpipe.fragments.list.videos.RelatedItemsFragment;
 import org.schabi.newpipe.ktx.AnimationType;
 import org.schabi.newpipe.local.dialog.PlaylistDialog;
 import org.schabi.newpipe.local.history.HistoryRecordManager;
+import org.schabi.newpipe.local.playlist.LocalPlaylistFragment;
 import org.schabi.newpipe.player.Player;
 import org.schabi.newpipe.player.PlayerService;
 import org.schabi.newpipe.player.PlayerType;
@@ -472,10 +475,23 @@ private void setOnClickListeners() {
 
         binding.detailControlsBackground.setOnClickListener(v -> openBackgroundPlayer(false));
         binding.detailControlsPopup.setOnClickListener(v -> openPopupPlayer(false));
-        binding.detailControlsPlaylistAppend.setOnClickListener(makeOnClickListener(info ->
+        binding.detailControlsPlaylistAppend.setOnClickListener(makeOnClickListener(info -> {
+            if (getFM() != null && currentInfo != null) {
+                final Fragment fragment = getParentFragmentManager().
+                        findFragmentById(R.id.fragment_holder);
+
+                // commit previous pending changes to database
+                if (fragment instanceof LocalPlaylistFragment) {
+                    ((LocalPlaylistFragment) fragment).commitChanges();
+                } else if (fragment instanceof MainFragment) {
+                    ((MainFragment) fragment).commitPlaylistTabs();
+                }
+
                 disposables.add(PlaylistDialog.createCorrespondingDialog(requireContext(),
                         List.of(new StreamEntity(info)),
-                        dialog -> dialog.show(getParentFragmentManager(), TAG)))));
+                        dialog -> dialog.show(getParentFragmentManager(), TAG)));
+            }
+        }));
         binding.detailControlsDownload.setOnClickListener(v -> {
             if (PermissionHelper.checkStoragePermissions(activity,
                     PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE)) {

app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java

@@ -41,6 +41,7 @@
 import org.schabi.newpipe.error.ErrorInfo;
 import org.schabi.newpipe.error.UserAction;
 import org.schabi.newpipe.extractor.stream.StreamInfoItem;
+import org.schabi.newpipe.fragments.MainFragment;
 import org.schabi.newpipe.fragments.list.playlist.PlaylistControlViewHolder;
 import org.schabi.newpipe.info_list.dialog.InfoItemDialog;
 import org.schabi.newpipe.info_list.dialog.StreamDialogDefaultEntry;
@@ -71,7 +72,7 @@
 
 public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistStreamEntry>, Void>
         implements PlaylistControlViewHolder {
-    // Save the list 10 seconds after the last change occurred
+    /** Save the list 10 seconds after the last change occurred. */
     private static final long SAVE_DEBOUNCE_MILLIS = 10000;
     private static final int MINIMUM_INITIAL_DRAG_VELOCITY = 12;
     @State
@@ -92,13 +93,20 @@ public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistSt
     private PublishSubject<Long> debouncedSaveSignal;
     private CompositeDisposable disposables;
 
-    /* Has the playlist been fully loaded from db */
+    /** Whether the playlist has been fully loaded from db. */
     private AtomicBoolean isLoadingComplete;
-    /* Has the playlist been modified (e.g. items reordered or deleted) */
+    /** Whether the playlist has been modified (e.g. items reordered or deleted) */
     private AtomicBoolean isModified;
-    /* Flag to prevent simultaneous rewrites of the playlist */
+    /** Flag to prevent simultaneous rewrites of the playlist. */
     private boolean isRewritingPlaylist = false;
 
+    /**
+     * The pager adapter that the fragment is created from when it is used as frontpage, i.e.
+     * {@link #useAsFrontPage} is {@link true}.
+     */
+    @Nullable
+    private MainFragment.SelectedTabsPagerAdapter tabsPagerAdapter = null;
+
     public static LocalPlaylistFragment getInstance(final long playlistId, final String name) {
         final LocalPlaylistFragment instance = new LocalPlaylistFragment();
         instance.setInitialData(playlistId, name);
@@ -158,6 +166,17 @@ protected ViewBinding getListHeader() {
         return headerBinding;
     }
 
+    /**
+     * <p>Commit changes immediately if the playlist has been modified.</p>
+     *  Delete operations and other modifications will be committed to ensure that the database
+     *  is up to date, e.g. when the user adds the just deleted stream from another fragment.
+     */
+    public void commitChanges() {
+        if (isModified != null && isModified.get()) {
+            saveImmediate();
+        }
+    }
+
     @Override
     protected void initListeners() {
         super.initListeners();
@@ -291,6 +310,9 @@ public void onDestroy() {
         if (disposables != null) {
             disposables.dispose();
         }
+        if (tabsPagerAdapter != null) {
+            tabsPagerAdapter.getLocalPlaylistFragments().remove(this);
+        }
 
         debouncedSaveSignal = null;
         playlistManager = null;
@@ -877,5 +899,10 @@ private void createShareConfirmationDialog() {
                 )
                 .show();
     }
+
+    public void setTabsPagerAdapter(
+            @Nullable final MainFragment.SelectedTabsPagerAdapter tabsPagerAdapter) {
+        this.tabsPagerAdapter = tabsPagerAdapter;
+    }
 }