From 400ee808e08055fcbec4304a5488dbe38afa8a04 Mon Sep 17 00:00:00 2001 From: devlearner Date: Sat, 15 Oct 2022 23:34:39 +0800 Subject: [PATCH 01/14] Set up theme/locale before super.create() This seems to solve a bug where the Open action menu dialog does not appear the first time on cold start on older Android (8.0). This is also the order of things in MainActivity and probably good practice. --- .../main/java/org/schabi/newpipe/RouterActivity.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 936beecff..b012be109 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -114,6 +114,11 @@ public class RouterActivity extends AppCompatActivity { @Override protected void onCreate(final Bundle savedInstanceState) { + ThemeHelper.setDayNightMode(this); + setTheme(ThemeHelper.isLightThemeSelected(this) + ? R.style.RouterActivityThemeLight : R.style.RouterActivityThemeDark); + Localization.assureCorrectAppLanguage(this); + super.onCreate(savedInstanceState); Icepick.restoreInstanceState(this, savedInstanceState); @@ -125,11 +130,6 @@ public class RouterActivity extends AppCompatActivity { finish(); } } - - ThemeHelper.setDayNightMode(this); - setTheme(ThemeHelper.isLightThemeSelected(this) - ? R.style.RouterActivityThemeLight : R.style.RouterActivityThemeDark); - Localization.assureCorrectAppLanguage(this); } @Override From 73e32889b674d9344306b7494247529d0a3925d6 Mon Sep 17 00:00:00 2001 From: devlearner Date: Sun, 16 Oct 2022 01:01:19 +0800 Subject: [PATCH 02/14] Don't finish() to allow recreate when orientation change is on foot --- app/src/main/java/org/schabi/newpipe/RouterActivity.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index b012be109..5892d9766 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -161,6 +161,14 @@ public class RouterActivity extends AppCompatActivity { disposables.clear(); } + @Override + public void finish() { + // allow the activity to recreate in case orientation changes + if (!isChangingConfigurations()) { + super.finish(); + } + } + private void handleUrl(final String url) { disposables.add(Observable .fromCallable(() -> { From b175774ad882eaec1530d329ddcc104c5500aca8 Mon Sep 17 00:00:00 2001 From: devlearner Date: Sun, 16 Oct 2022 02:32:41 +0800 Subject: [PATCH 03/14] Try to amicably handle DialogFragment in FragmentManager when recreated from orientation change - Handle finish() call instead of passing around callbacks to setOnDismissListener() - Don't start over again if returning to DialogFragment before orientation change --- .../org/schabi/newpipe/RouterActivity.java | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 5892d9766..0c4cff8d1 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -31,6 +31,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.Fragment; import androidx.fragment.app.FragmentManager; import androidx.preference.PreferenceManager; @@ -151,7 +152,32 @@ public class RouterActivity extends AppCompatActivity { protected void onStart() { super.onStart(); - handleUrl(currentUrl); + // 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()) { + // Start over from scratch + handleUrl(currentUrl); + } } @Override @@ -659,12 +685,12 @@ public class RouterActivity extends AppCompatActivity { getThemeWrapperContext(), List.of(new StreamEntity(info)), playlistDialog -> { - playlistDialog.setOnDismissListener(dialog -> finish()); + // to be handled by FragmentManager + // playlistDialog.setOnDismissListener(dialog ->finish()); - playlistDialog.show( - this.getSupportFragmentManager(), - "addToPlaylistDialog" - ); + final FragmentManager fm = getSupportFragmentManager(); + playlistDialog.show(fm, "addToPlaylistDialog"); + fm.executePendingTransactions(); } ), throwable -> handleError(this, new ErrorInfo( @@ -684,7 +710,8 @@ public class RouterActivity extends AppCompatActivity { .observeOn(AndroidSchedulers.mainThread()) .subscribe(result -> { final DownloadDialog downloadDialog = new DownloadDialog(this, result); - downloadDialog.setOnDismissListener(dialog -> finish()); + // to be handled by FragmentManager since listener would be gone when recreated + // playlistDialog.setOnDismissListener(dialog ->finish()); final FragmentManager fm = getSupportFragmentManager(); downloadDialog.show(fm, "downloadDialog"); From c1f37d85919686ea5ee1eef6cba695f63146d3fd Mon Sep 17 00:00:00 2001 From: devlearner Date: Sun, 16 Oct 2022 02:47:53 +0800 Subject: [PATCH 04/14] Also show toast in openDownloadDialog() and lengthened a bit to inform user to wait... --- .../org/schabi/newpipe/RouterActivity.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 0c4cff8d1..4a26a7905 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -669,13 +669,7 @@ public class RouterActivity extends AppCompatActivity { } private void openAddToPlaylistDialog() { - // Getting the stream info usually takes a moment - // Notifying the user here to ensure that no confusion arises - Toast.makeText( - getApplicationContext(), - getString(R.string.processing_may_take_a_moment), - Toast.LENGTH_SHORT) - .show(); + pleaseWait(); disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, false) .subscribeOn(Schedulers.io()) @@ -705,6 +699,8 @@ public class RouterActivity extends AppCompatActivity { @SuppressLint("CheckResult") private void openDownloadDialog() { + pleaseWait(); + disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, true) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -719,6 +715,16 @@ public class RouterActivity extends AppCompatActivity { }, throwable -> showUnsupportedUrlDialog(currentUrl))); } + private void pleaseWait() { + // Getting the stream info usually takes a moment + // Notifying the user here to ensure that no confusion arises + Toast.makeText( + getApplicationContext(), + getString(R.string.processing_may_take_a_moment), + Toast.LENGTH_LONG) + .show(); + } + @Override public void onRequestPermissionsResult(final int requestCode, @NonNull final String[] permissions, From 391830558e4eb8261447d5e69e2fbe83afab90ea Mon Sep 17 00:00:00 2001 From: devlearner Date: Sun, 16 Oct 2022 03:10:37 +0800 Subject: [PATCH 05/14] Ensure our transparent activity doesn't block touch events to underlying windows so we won't hold up UI while fetching media info for Add to Playlist or Download actions lest user might think it freezes when in fact a network request is underway --- app/src/main/java/org/schabi/newpipe/RouterActivity.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 4a26a7905..3f3eb1670 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -16,6 +16,7 @@ import android.view.ContextThemeWrapper; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; +import android.view.WindowManager; import android.widget.Button; import android.widget.RadioButton; import android.widget.RadioGroup; @@ -120,6 +121,14 @@ public class RouterActivity extends AppCompatActivity { ? R.style.RouterActivityThemeLight : R.style.RouterActivityThemeDark); Localization.assureCorrectAppLanguage(this); + // Pass-through touch events to background activities + // so that our transparent window won't lock UI in the mean time + // network request is underway before showing PlaylistDialog or DownloadDialog + // (courtesy of https://stackoverflow.com/a/10606141) + getWindow().addFlags(WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE + | WindowManager.LayoutParams.FLAG_NOT_TOUCH_MODAL + | WindowManager.LayoutParams.FLAG_NOT_TOUCHABLE); + super.onCreate(savedInstanceState); Icepick.restoreInstanceState(this, savedInstanceState); From f860392ae9e67cf82eb82ee10dc9ed03c4ec2e33 Mon Sep 17 00:00:00 2001 From: devlearner Date: Fri, 2 Dec 2022 06:13:54 +0000 Subject: [PATCH 06/14] Address LayoutParams.FLAG_NOT_TOUCHABLE restriction on Andriod 12+ --- .../main/java/org/schabi/newpipe/RouterActivity.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 3f3eb1670..afc4321c1 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -124,11 +124,21 @@ public class RouterActivity extends AppCompatActivity { // Pass-through touch events to background activities // so that our transparent window won't lock UI in the mean time // network request is underway before showing PlaylistDialog or DownloadDialog - // (courtesy of https://stackoverflow.com/a/10606141) + // (ref: https://stackoverflow.com/a/10606141) getWindow().addFlags(WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE | WindowManager.LayoutParams.FLAG_NOT_TOUCH_MODAL | WindowManager.LayoutParams.FLAG_NOT_TOUCHABLE); + // Android never fails to impress us with a list of new restrictions per API. + // Starting with S (Android 12) one of the prerequisite conditions has to be met + // before the FLAG_NOT_TOUCHABLE flag is allowed to kick in: + // @see WindowManager.LayoutParams#FLAG_NOT_TOUCHABLE + // For our present purpose it seems we can just set LayoutParams.alpha to 0 + // on the strength of "4. Fully transparent windows" without affecting the scrim of dialogs + final WindowManager.LayoutParams params = getWindow().getAttributes(); + params.alpha = 0f; + getWindow().setAttributes(params); + super.onCreate(savedInstanceState); Icepick.restoreInstanceState(this, savedInstanceState); From 0f9c20c986d8a58f48990fff57c03323b6e9ca70 Mon Sep 17 00:00:00 2001 From: devlearner Date: Fri, 2 Dec 2022 09:25:06 +0000 Subject: [PATCH 07/14] 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(); } From 585bfff11da4edecc9b68ac2d2f408ef272d4c27 Mon Sep 17 00:00:00 2001 From: devlearner Date: Fri, 2 Dec 2022 14:22:00 +0000 Subject: [PATCH 08/14] Utilize a retained fragment to safekeep network requests in flight pending result for openAddToPlaylistDialog() and openDownloadDialog() Despite marked deprecated, setRetainInstance(true) is probably our best bet (since a ViewModel is probably too overkill for our present purpose) --- .../org/schabi/newpipe/RouterActivity.java | 203 ++++++++++++++---- 1 file changed, 163 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 35bb8612f..e61f1f738 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -83,9 +83,11 @@ import org.schabi.newpipe.util.urlfinder.UrlFinder; import org.schabi.newpipe.views.FocusOverlayView; import java.io.Serializable; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Vector; import icepick.Icepick; import icepick.State; @@ -300,7 +302,7 @@ public class RouterActivity extends AppCompatActivity { } } - private void showUnsupportedUrlDialog(final String url) { + protected void showUnsupportedUrlDialog(final String url) { final Context context = getThemeWrapperContext(); new AlertDialog.Builder(context) .setTitle(R.string.unsupported_url) @@ -587,7 +589,7 @@ public class RouterActivity extends AppCompatActivity { return returnedItems; } - private Context getThemeWrapperContext() { + protected Context getThemeWrapperContext() { return new ContextThemeWrapper(this, ThemeHelper.isLightThemeSelected(this) ? R.style.LightTheme : R.style.DarkTheme); } @@ -694,54 +696,175 @@ public class RouterActivity extends AppCompatActivity { return playerType == null || playerType == PlayerType.MAIN; } + public static class PersistentFragment extends Fragment { + private WeakReference context; + private boolean isPaused = true; + private final Vector buffer = new Vector<>(); + private final CompositeDisposable disposables = new CompositeDisposable(); + + public interface ResultRunnable { + void run(AppCompatActivity context); + } + + @Override + public void onAttach(@NonNull final Context activityContext) { + super.onAttach(activityContext); + context = new WeakReference<>((AppCompatActivity) activityContext); + } + + @SuppressWarnings("deprecation") + @Override + public void onCreate(final Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + setRetainInstance(true); + } + + @Override + public void onDestroy() { + super.onDestroy(); + disposables.clear(); + } + + @Override + public void onPause() { + isPaused = true; + super.onPause(); + } + + @Override + public void onResume() { + isPaused = false; + playback(); + super.onResume(); + } + + private AppCompatActivity getActivityContext() { + return context == null ? null : context.get(); + } + + // guard against IllegalStateException in calling DialogFragment.show() whilst in background + // (which could happen, say, when the user pressed the home button while waiting for + // the network request to return) when it internally calls FragmentTransaction.commit() + // after the FragmentManager has saved its states (isStateSaved() == true) + // (ref: https://stackoverflow.com/a/39813506) + private void playback() { + if (activityGone()) { + done(); + } + if (buffer.size() == 0 || isPaused) { + return; + } + while (buffer.size() > 0) { + final ResultRunnable runnable = buffer.elementAt(0); + buffer.removeElementAt(0); + getActivityContext().runOnUiThread(() -> { + // execute queued task with new context, in case activity has been recreated + runnable.run(getActivityContext()); + }); + } + done(); + } + private boolean activityGone() { + return getActivityContext() == null || getActivityContext().isFinishing(); + } + + // a DefaultLifecycleObserver is probably a good candidate here, but for now + // let's stick with a vanilla approach to avoid pulling in an extra artifact just for this + private void runOnVisible(final ResultRunnable runnable) { + if (activityGone()) { + done(); + } + if (isPaused) { + buffer.add(runnable); + if (!getActivityContext().isChangingConfigurations()) { + // try to bring the activity back to front if minimised + final Intent i = new Intent(getActivityContext(), RouterActivity.class); + i.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT); + startActivity(i); + } + } else { + getActivityContext().runOnUiThread(() -> { + runnable.run(getActivityContext()); + done(); + }); + } + } + + private void done() { + if (getActivityContext() != null) { + getActivityContext().getSupportFragmentManager() + .beginTransaction().remove(this).commit(); + } + } + + @SuppressLint("CheckResult") + protected void openDownloadDialog(final int currentServiceId, final String currentUrl) { + disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, true) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(result -> + runOnVisible(ctx -> { + final FragmentManager fm = ctx.getSupportFragmentManager(); + final DownloadDialog downloadDialog = new DownloadDialog(ctx, result); + // dismiss listener to be handled by FragmentManager + downloadDialog.show(fm, "downloadDialog"); + } + ), throwable -> runOnVisible(ctx -> + ((RouterActivity) ctx).showUnsupportedUrlDialog(currentUrl)))); + } + + private void openAddToPlaylistDialog(final int currentServiceId, final String currentUrl) { + disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, false) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe( + info -> runOnVisible(ctx -> + PlaylistDialog.createCorrespondingDialog( + ((RouterActivity) ctx).getThemeWrapperContext(), + List.of(new StreamEntity(info)), + playlistDialog -> { + // dismiss listener to be handled by FragmentManager + final FragmentManager fm = ctx.getSupportFragmentManager(); + playlistDialog.show(fm, "addToPlaylistDialog"); + } + )), + throwable -> runOnVisible(ctx -> handleError(ctx, new ErrorInfo( + throwable, + UserAction.REQUESTED_STREAM, + "Tried to add " + currentUrl + " to a playlist", + ((RouterActivity) ctx).currentService.getServiceId()) + )) + ) + ); + } + } + private void openAddToPlaylistDialog() { pleaseWait(); - disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, false) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe( - info -> PlaylistDialog.createCorrespondingDialog( - getThemeWrapperContext(), - List.of(new StreamEntity(info)), - playlistDialog -> { - // to be handled by FragmentManager - // playlistDialog.setOnDismissListener(dialog ->finish()); - - final FragmentManager fm = getSupportFragmentManager(); - playlistDialog.show(fm, "addToPlaylistDialog"); - fm.executePendingTransactions(); - } - ), - throwable -> handleError(this, new ErrorInfo( - throwable, - UserAction.REQUESTED_STREAM, - "Tried to add " + currentUrl + " to a playlist", - currentService.getServiceId()) - ) - ) - ); + getPersistFragment().openAddToPlaylistDialog(currentServiceId, currentUrl); } - @SuppressLint("CheckResult") private void openDownloadDialog() { pleaseWait(); - disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, true) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(result -> { - final DownloadDialog downloadDialog = new DownloadDialog(this, result); - // to be handled by FragmentManager since listener would be gone when recreated - // playlistDialog.setOnDismissListener(dialog ->finish()); - - final FragmentManager fm = getSupportFragmentManager(); - downloadDialog.show(fm, "downloadDialog"); - fm.executePendingTransactions(); - }, throwable -> showUnsupportedUrlDialog(currentUrl))); + getPersistFragment().openDownloadDialog(currentServiceId, currentUrl); } - private void pleaseWait() { + private PersistentFragment getPersistFragment() { + final FragmentManager fm = getSupportFragmentManager(); + PersistentFragment persistFragment = + (PersistentFragment) fm.findFragmentByTag("PERSIST_FRAGMENT"); + if (persistFragment == null) { + persistFragment = new PersistentFragment(); + fm.beginTransaction() + .add(persistFragment, "PERSIST_FRAGMENT") + .commitNow(); + } + return persistFragment; + } + + protected void pleaseWait() { // Getting the stream info usually takes a moment // Notifying the user here to ensure that no confusion arises Toast.makeText( From de7057ac3a6f188a9c54348432330b01b9d4b087 Mon Sep 17 00:00:00 2001 From: devlearner Date: Fri, 2 Dec 2022 17:31:01 +0000 Subject: [PATCH 09/14] Skip REORDER_TO_FRONT trick which doesn't seem to work on newer Androids probably due to background restrictions on Android 10+ --- app/src/main/java/org/schabi/newpipe/RouterActivity.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index e61f1f738..1a33c0ca4 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -10,6 +10,7 @@ import android.content.DialogInterface; import android.content.Intent; import android.content.SharedPreferences; import android.content.pm.PackageManager; +import android.os.Build; import android.os.Bundle; import android.text.TextUtils; import android.view.ContextThemeWrapper; @@ -776,7 +777,10 @@ public class RouterActivity extends AppCompatActivity { } if (isPaused) { buffer.add(runnable); - if (!getActivityContext().isChangingConfigurations()) { + // this trick doesn't seem to work on Android 10+ (API 29) + // which places restrictions on starting activities from the background + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q + && !getActivityContext().isChangingConfigurations()) { // try to bring the activity back to front if minimised final Intent i = new Intent(getActivityContext(), RouterActivity.class); i.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT); From c744f6756b75814243fae93924ea7fc62b0d885e Mon Sep 17 00:00:00 2001 From: devlearner Date: Fri, 2 Dec 2022 17:41:43 +0000 Subject: [PATCH 10/14] Fix Sonar reported code smell --- app/src/main/java/org/schabi/newpipe/RouterActivity.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 1a33c0ca4..432bbcdaa 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -758,10 +758,10 @@ public class RouterActivity extends AppCompatActivity { while (buffer.size() > 0) { final ResultRunnable runnable = buffer.elementAt(0); buffer.removeElementAt(0); - getActivityContext().runOnUiThread(() -> { + getActivityContext().runOnUiThread(() -> // execute queued task with new context, in case activity has been recreated - runnable.run(getActivityContext()); - }); + runnable.run(getActivityContext()) + ); } done(); } From 61da167b4f52d0732d23396c8d131faeb596f5af Mon Sep 17 00:00:00 2001 From: devlearner Date: Sat, 3 Dec 2022 13:23:36 +0800 Subject: [PATCH 11/14] Oops, added back missing return; --- app/src/main/java/org/schabi/newpipe/RouterActivity.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 432bbcdaa..dc7c8a77e 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -751,6 +751,7 @@ public class RouterActivity extends AppCompatActivity { private void playback() { if (activityGone()) { done(); + return; } if (buffer.size() == 0 || isPaused) { return; @@ -774,6 +775,7 @@ public class RouterActivity extends AppCompatActivity { private void runOnVisible(final ResultRunnable runnable) { if (activityGone()) { done(); + return; } if (isPaused) { buffer.add(runnable); From 40442f3f828e449dbbc4795f84756ddf43c528ea Mon Sep 17 00:00:00 2001 From: devlearner Date: Wed, 7 Dec 2022 13:43:27 +0000 Subject: [PATCH 12/14] Utilize Lifecycle observer I thought it would have required an extra dependency; apparently that doesn't seem to be the case... --- .../org/schabi/newpipe/RouterActivity.java | 103 ++++++++---------- 1 file changed, 46 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index dc7c8a77e..87d402654 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -36,6 +36,9 @@ import androidx.core.math.MathUtils; import androidx.fragment.app.DialogFragment; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; +import androidx.lifecycle.DefaultLifecycleObserver; +import androidx.lifecycle.Lifecycle; +import androidx.lifecycle.LifecycleOwner; import androidx.preference.PreferenceManager; import org.schabi.newpipe.database.stream.model.StreamEntity; @@ -88,7 +91,6 @@ import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Vector; import icepick.Icepick; import icepick.State; @@ -213,6 +215,7 @@ public class RouterActivity extends AppCompatActivity { if (dismissListener != null) { getSupportFragmentManager().unregisterFragmentLifecycleCallbacks(dismissListener); } + disposables.clear(); } @@ -699,9 +702,20 @@ public class RouterActivity extends AppCompatActivity { public static class PersistentFragment extends Fragment { private WeakReference context; - private boolean isPaused = true; - private final Vector buffer = new Vector<>(); private final CompositeDisposable disposables = new CompositeDisposable(); + private int running = 0; + + private synchronized void inFlight(final boolean started) { + if (started) { + running++; + } else { + running--; + if (running <= 0 && getActivityContext() != null) { + getActivityContext().getSupportFragmentManager() + .beginTransaction().remove(this).commit(); + } + } + } public interface ResultRunnable { void run(AppCompatActivity context); @@ -726,59 +740,44 @@ public class RouterActivity extends AppCompatActivity { disposables.clear(); } - @Override - public void onPause() { - isPaused = true; - super.onPause(); - } - - @Override - public void onResume() { - isPaused = false; - playback(); - super.onResume(); - } - private AppCompatActivity getActivityContext() { return context == null ? null : context.get(); } + private boolean activityGone() { + return getActivityContext() == null || getActivityContext().isFinishing(); + } + // guard against IllegalStateException in calling DialogFragment.show() whilst in background // (which could happen, say, when the user pressed the home button while waiting for // the network request to return) when it internally calls FragmentTransaction.commit() // after the FragmentManager has saved its states (isStateSaved() == true) // (ref: https://stackoverflow.com/a/39813506) - private void playback() { - if (activityGone()) { - done(); - return; - } - if (buffer.size() == 0 || isPaused) { - return; - } - while (buffer.size() > 0) { - final ResultRunnable runnable = buffer.elementAt(0); - buffer.removeElementAt(0); - getActivityContext().runOnUiThread(() -> - // execute queued task with new context, in case activity has been recreated - runnable.run(getActivityContext()) - ); - } - done(); - } - private boolean activityGone() { - return getActivityContext() == null || getActivityContext().isFinishing(); - } - - // a DefaultLifecycleObserver is probably a good candidate here, but for now - // let's stick with a vanilla approach to avoid pulling in an extra artifact just for this private void runOnVisible(final ResultRunnable runnable) { if (activityGone()) { - done(); + inFlight(false); return; } - if (isPaused) { - buffer.add(runnable); + if (getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.STARTED)) { + getActivityContext().runOnUiThread(() -> { + runnable.run(getActivityContext()); + inFlight(false); + }); + } else { + getLifecycle().addObserver(new DefaultLifecycleObserver() { + @Override + public void onResume(@NonNull final LifecycleOwner owner) { + getLifecycle().removeObserver(this); + if (activityGone()) { + inFlight(false); + return; + } + getActivityContext().runOnUiThread(() -> { + runnable.run(getActivityContext()); + inFlight(false); + }); + } + }); // this trick doesn't seem to work on Android 10+ (API 29) // which places restrictions on starting activities from the background if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q @@ -788,23 +787,12 @@ public class RouterActivity extends AppCompatActivity { i.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT); startActivity(i); } - } else { - getActivityContext().runOnUiThread(() -> { - runnable.run(getActivityContext()); - done(); - }); - } - } - - private void done() { - if (getActivityContext() != null) { - getActivityContext().getSupportFragmentManager() - .beginTransaction().remove(this).commit(); } } @SuppressLint("CheckResult") - protected void openDownloadDialog(final int currentServiceId, final String currentUrl) { + private void openDownloadDialog(final int currentServiceId, final String currentUrl) { + inFlight(true); disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, true) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -820,6 +808,7 @@ public class RouterActivity extends AppCompatActivity { } private void openAddToPlaylistDialog(final int currentServiceId, final String currentUrl) { + inFlight(true); disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, false) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -870,7 +859,7 @@ public class RouterActivity extends AppCompatActivity { return persistFragment; } - protected void pleaseWait() { + private void pleaseWait() { // Getting the stream info usually takes a moment // Notifying the user here to ensure that no confusion arises Toast.makeText( From 28109fef38b2f00a8759cd34ad1473af83db50bf Mon Sep 17 00:00:00 2001 From: devlearner Date: Sun, 11 Dec 2022 18:10:00 +0000 Subject: [PATCH 13/14] Improve showing of toast We provide visual feedback via a toast to the user that, well, they're supposed to wait; but with the benefit of the cache openAddToPlaylistDialog() may return (almost) immediately, which would render the toast otiose (if not a bit confusing). This commit improves that by cancelling the toast once the wait's over ... (by 'abusing' RxJava's ambWith(); ref on compose() and Transformer: https://blog.danlew.net/2015/03/02/dont-break-the-chain/ and for me, first time laying my hands at RxJava so kindly bear with me; open for suggestions) --- .../org/schabi/newpipe/RouterActivity.java | 57 ++++++++++++------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index 87d402654..d660b6f3d 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -97,6 +97,7 @@ import icepick.State; import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; import io.reactivex.rxjava3.core.Observable; import io.reactivex.rxjava3.core.Single; +import io.reactivex.rxjava3.core.SingleTransformer; import io.reactivex.rxjava3.disposables.CompositeDisposable; import io.reactivex.rxjava3.disposables.Disposable; import io.reactivex.rxjava3.functions.Consumer; @@ -790,12 +791,32 @@ public class RouterActivity extends AppCompatActivity { } } + SingleTransformer pleaseWait() { + return single -> single + // 'abuse' ambWith() here to cancel the toast for us when the wait is over + .ambWith(Single.create(emitter -> { + if (!activityGone()) { + getActivityContext().runOnUiThread(() -> { + // Getting the stream info usually takes a moment + // Notifying the user here to ensure that no confusion arises + final Toast t = Toast.makeText( + getActivityContext().getApplicationContext(), + getString(R.string.processing_may_take_a_moment), + Toast.LENGTH_LONG); + t.show(); + emitter.setCancellable(t::cancel); + }); + } + })); + } + @SuppressLint("CheckResult") private void openDownloadDialog(final int currentServiceId, final String currentUrl) { inFlight(true); disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, true) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) + .compose(pleaseWait()) .subscribe(result -> runOnVisible(ctx -> { final FragmentManager fm = ctx.getSupportFragmentManager(); @@ -812,17 +833,23 @@ public class RouterActivity extends AppCompatActivity { disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, false) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) + .compose(pleaseWait()) .subscribe( - info -> runOnVisible(ctx -> + info -> { + if (!activityGone()) { PlaylistDialog.createCorrespondingDialog( - ((RouterActivity) ctx).getThemeWrapperContext(), + getActivityContext(), List.of(new StreamEntity(info)), - playlistDialog -> { - // dismiss listener to be handled by FragmentManager - final FragmentManager fm = ctx.getSupportFragmentManager(); - playlistDialog.show(fm, "addToPlaylistDialog"); - } - )), + playlistDialog -> + runOnVisible(ctx -> { + // dismiss listener to be handled by FragmentManager + final FragmentManager fm = + ctx.getSupportFragmentManager(); + playlistDialog.show(fm, "addToPlaylistDialog"); + }) + ); + } + }, throwable -> runOnVisible(ctx -> handleError(ctx, new ErrorInfo( throwable, UserAction.REQUESTED_STREAM, @@ -835,14 +862,10 @@ public class RouterActivity extends AppCompatActivity { } private void openAddToPlaylistDialog() { - pleaseWait(); - getPersistFragment().openAddToPlaylistDialog(currentServiceId, currentUrl); } private void openDownloadDialog() { - pleaseWait(); - getPersistFragment().openDownloadDialog(currentServiceId, currentUrl); } @@ -859,16 +882,6 @@ public class RouterActivity extends AppCompatActivity { return persistFragment; } - private void pleaseWait() { - // Getting the stream info usually takes a moment - // Notifying the user here to ensure that no confusion arises - Toast.makeText( - getApplicationContext(), - getString(R.string.processing_may_take_a_moment), - Toast.LENGTH_LONG) - .show(); - } - @Override public void onRequestPermissionsResult(final int requestCode, @NonNull final String[] permissions, From 944e295ae70a9641c43494b7fda1ce340cb28589 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 11 Jan 2023 15:08:10 +0100 Subject: [PATCH 14/14] Use Optional for simpler code --- .../org/schabi/newpipe/RouterActivity.java | 160 +++++++++--------- 1 file changed, 77 insertions(+), 83 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index d660b6f3d..010f35bc3 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -91,16 +91,16 @@ import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Optional; +import java.util.function.Consumer; import icepick.Icepick; import icepick.State; import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; import io.reactivex.rxjava3.core.Observable; import io.reactivex.rxjava3.core.Single; -import io.reactivex.rxjava3.core.SingleTransformer; import io.reactivex.rxjava3.disposables.CompositeDisposable; import io.reactivex.rxjava3.disposables.Disposable; -import io.reactivex.rxjava3.functions.Consumer; import io.reactivex.rxjava3.schedulers.Schedulers; /** @@ -702,7 +702,7 @@ public class RouterActivity extends AppCompatActivity { } public static class PersistentFragment extends Fragment { - private WeakReference context; + private WeakReference weakContext; private final CompositeDisposable disposables = new CompositeDisposable(); private int running = 0; @@ -711,21 +711,23 @@ public class RouterActivity extends AppCompatActivity { running++; } else { running--; - if (running <= 0 && getActivityContext() != null) { - getActivityContext().getSupportFragmentManager() - .beginTransaction().remove(this).commit(); + if (running <= 0) { + getActivityContext().ifPresent(context -> context.getSupportFragmentManager() + .beginTransaction().remove(this).commit()); } } } - public interface ResultRunnable { - void run(AppCompatActivity context); - } - @Override public void onAttach(@NonNull final Context activityContext) { super.onAttach(activityContext); - context = new WeakReference<>((AppCompatActivity) activityContext); + weakContext = new WeakReference<>((AppCompatActivity) activityContext); + } + + @Override + public void onDetach() { + super.onDetach(); + weakContext = null; } @SuppressWarnings("deprecation") @@ -741,12 +743,13 @@ public class RouterActivity extends AppCompatActivity { disposables.clear(); } - private AppCompatActivity getActivityContext() { - return context == null ? null : context.get(); - } - - private boolean activityGone() { - return getActivityContext() == null || getActivityContext().isFinishing(); + /** + * @return the activity context, if there is one and the activity is not finishing + */ + private Optional getActivityContext() { + return Optional.ofNullable(weakContext) + .flatMap(context -> Optional.ofNullable(context.get())) + .filter(context -> !context.isFinishing()); } // guard against IllegalStateException in calling DialogFragment.show() whilst in background @@ -754,60 +757,56 @@ public class RouterActivity extends AppCompatActivity { // the network request to return) when it internally calls FragmentTransaction.commit() // after the FragmentManager has saved its states (isStateSaved() == true) // (ref: https://stackoverflow.com/a/39813506) - private void runOnVisible(final ResultRunnable runnable) { - if (activityGone()) { - inFlight(false); - return; - } - if (getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.STARTED)) { - getActivityContext().runOnUiThread(() -> { - runnable.run(getActivityContext()); - inFlight(false); - }); - } else { - getLifecycle().addObserver(new DefaultLifecycleObserver() { - @Override - public void onResume(@NonNull final LifecycleOwner owner) { - getLifecycle().removeObserver(this); - if (activityGone()) { - inFlight(false); - return; + private void runOnVisible(final Consumer runnable) { + getActivityContext().ifPresentOrElse(context -> { + if (getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.STARTED)) { + context.runOnUiThread(() -> { + runnable.accept(context); + inFlight(false); + }); + } else { + getLifecycle().addObserver(new DefaultLifecycleObserver() { + @Override + public void onResume(@NonNull final LifecycleOwner owner) { + getLifecycle().removeObserver(this); + getActivityContext().ifPresentOrElse(context -> + context.runOnUiThread(() -> { + runnable.accept(context); + inFlight(false); + }), + () -> inFlight(false) + ); } - getActivityContext().runOnUiThread(() -> { - runnable.run(getActivityContext()); - inFlight(false); - }); + }); + // this trick doesn't seem to work on Android 10+ (API 29) + // which places restrictions on starting activities from the background + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q + && !context.isChangingConfigurations()) { + // try to bring the activity back to front if minimised + final Intent i = new Intent(context, RouterActivity.class); + i.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT); + startActivity(i); } - }); - // this trick doesn't seem to work on Android 10+ (API 29) - // which places restrictions on starting activities from the background - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q - && !getActivityContext().isChangingConfigurations()) { - // try to bring the activity back to front if minimised - final Intent i = new Intent(getActivityContext(), RouterActivity.class); - i.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT); - startActivity(i); } - } + + }, () -> { + // this branch is executed if there is no activity context + inFlight(false); + }); } - SingleTransformer pleaseWait() { - return single -> single - // 'abuse' ambWith() here to cancel the toast for us when the wait is over - .ambWith(Single.create(emitter -> { - if (!activityGone()) { - getActivityContext().runOnUiThread(() -> { - // Getting the stream info usually takes a moment - // Notifying the user here to ensure that no confusion arises - final Toast t = Toast.makeText( - getActivityContext().getApplicationContext(), - getString(R.string.processing_may_take_a_moment), - Toast.LENGTH_LONG); - t.show(); - emitter.setCancellable(t::cancel); - }); - } - })); + Single pleaseWait(final Single single) { + // 'abuse' ambWith() here to cancel the toast for us when the wait is over + return single.ambWith(Single.create(emitter -> getActivityContext().ifPresent(context -> + context.runOnUiThread(() -> { + // Getting the stream info usually takes a moment + // Notifying the user here to ensure that no confusion arises + final Toast toast = Toast.makeText(context, + getString(R.string.processing_may_take_a_moment), + Toast.LENGTH_LONG); + toast.show(); + emitter.setCancellable(toast::cancel); + })))); } @SuppressLint("CheckResult") @@ -816,7 +815,7 @@ public class RouterActivity extends AppCompatActivity { disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, true) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .compose(pleaseWait()) + .compose(this::pleaseWait) .subscribe(result -> runOnVisible(ctx -> { final FragmentManager fm = ctx.getSupportFragmentManager(); @@ -833,23 +832,18 @@ public class RouterActivity extends AppCompatActivity { disposables.add(ExtractorHelper.getStreamInfo(currentServiceId, currentUrl, false) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .compose(pleaseWait()) + .compose(this::pleaseWait) .subscribe( - info -> { - if (!activityGone()) { - PlaylistDialog.createCorrespondingDialog( - getActivityContext(), - List.of(new StreamEntity(info)), - playlistDialog -> - runOnVisible(ctx -> { - // dismiss listener to be handled by FragmentManager - final FragmentManager fm = - ctx.getSupportFragmentManager(); - playlistDialog.show(fm, "addToPlaylistDialog"); - }) - ); - } - }, + info -> getActivityContext().ifPresent(context -> + PlaylistDialog.createCorrespondingDialog(context, + List.of(new StreamEntity(info)), + playlistDialog -> runOnVisible(ctx -> { + // dismiss listener to be handled by FragmentManager + final FragmentManager fm = + ctx.getSupportFragmentManager(); + playlistDialog.show(fm, "addToPlaylistDialog"); + }) + )), throwable -> runOnVisible(ctx -> handleError(ctx, new ErrorInfo( throwable, UserAction.REQUESTED_STREAM,