Fixed restarting not working properly #7068
Conversation
* Using [``process-phoenix``](https://github.com/JakeWharton/ProcessPhoenix)
|
The readme of the library states
So is it safe to use in release/production use? Even if it is safe, should it be used? |
I think this was written wrongly by the author. I think he refers to state changes in the app.
I don't know that but it's certainly better than the current solution of right out crashing 😆 It's been around since 2016 has currently 5 issues and 1.5k stars so looks pretty solid to me. Maybe someone could test this with an production grade APK because I'm unable to build one (no documentation for that; running |
You are missing the flavour, which should be build. In this case Did you check if storing the import/export path on import is still working. Because since https://github.com/TeamNewPipe/NewPipe/pull/6531/files it relies on the app being closed orderly, which may or may not happen with this library.
Rather than that the root cause should be found instead of workaround, especially if a new library has to be introduced. So I took a closer look and the root cause has nothing to do with the restart. If you create a new export and import that one, it works. Since the breaking change was already made, there is nothing to be done there. |
|
If the change was recent and not released yet, then it can be solved "nicely" by using a different key and we don't need the extra logic to throw away the key |
|
@Stypox Seems like it was this PR https://github.com/TeamNewPipe/NewPipe/pull/3546/files beofre it was a bool https://github.com/TeamNewPipe/NewPipe/pull/3546/files#diff-f03dff133131d8391e0e529ec1b55f92ae78b782538484da9808bb88012c3894L214 and now its a Set https://github.com/TeamNewPipe/NewPipe/pull/3546/files#diff-cbf4fcc69345af02c1f5e3dff77d0ebc7ee544867016c6af4ca7d29f4a986c12R135 |
|
Can someone check which SettingMigrations are run after importing and at restart? Are they run at all? If not, that is the root cause of the problem. |
|
Looks like we do not run the settings migrations after importing the database. I was not able to reproduce the bug before, so I don't know it this addition solves the problem. Please test it: diff --git a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java
index 6e7e759326..21cf388789 100644
--- a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java
+++ b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java
@@ -240,6 +240,7 @@ public class ContentSettingsFragment extends BasePreferenceFragment {
dialog.dismiss();
manager.loadSharedPreferences(PreferenceManager
.getDefaultSharedPreferences(requireContext()));
+ SettingMigrations.initMigrations(requireContext(), false);
finishImport(importDataUri);
});
alert.show(); |
|
I think the comments are going into the wrong direction. This PR is not about fixing a currently broken database migration. |
|
There is also e.g. a Recaptcha cookie set from the preferences when initializing the downloader / app: Or other things like: This is also not done currently when importing the settings. |
Still works 😄
|
The crash in the video at top is caused by the change of the type of the of the As is said
I went this direction, because I only had the information available that the app is crashing when searching, which i took from the videos.
This changes some things, that I will have to look at |
|
I think my comment at #7066 (comment) was a bit misleading... Sorry. |
Well no, I made a migration so that the old boolean value is not discarded but is instead transformed into the |
Yes it does. The corresponding parts:
Restart occurs around
2x restart occurs. The first one kills the app completely. |
|
yes. |
What is it?
Description of the changes in your PR
The current method
NewPipe/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java
Lines 602 to 613 in 6a1d81f
Note: This introduces a new external library:
process-phoenixBefore/After Screenshots/Screen Record
NewPipeCrashOnImport.mp4
Note: ACRA fails to open, may be an additional problem (could be #5222)
Also note that it migrates the settings after the app crashes as seen in the log 😆:
NewPipeCrashOnImport.log
NoNewPipeCrashOnImport.mp4
NoNewPipeCrashOnImport.log
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence