Skip to content

Fix background crash focus#11789

Merged
Profpatsch merged 2 commits intoTeamNewPipe:devfrom
Thompson3142:fix_background_crash_focus
May 9, 2025
Merged

Fix background crash focus#11789
Profpatsch merged 2 commits intoTeamNewPipe:devfrom
Thompson3142:fix_background_crash_focus

Conversation

@Thompson3142
Copy link
Copy Markdown
Contributor

@Thompson3142 Thompson3142 commented Dec 12, 2024

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR addresses the issue of NewPipe stealing focus to report an error even while it is in the background.
To check if the app has been in the background a monitoring class (that could also be used for different purposes) and a shared preference have been introduced to ensure that the state is correctly remembered and can then handle the reporting accordingly e.g. only show a notification if the crash happened while in foreground and show a not.
This was tested via delayed test crashes:

new Handler(Looper.getMainLooper()).postDelayed(() -> {
            // Deliberately throw a RuntimeException
            throw new RuntimeException("Simulated crash for testing purposes.");
        }, 20000);

Occasionally i have seen this misbehave seemingly when another crash occurred before with a different foreground/background state. Unfortunately i have not found a fix for this, if somebody wants to take a look or can reproduce it I would be grateful. I am also open to completely different approaches since this approach is of course not optimal.

Fixes the following issue(s)

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. You can find more info and a video demonstration on this wiki page.

Due diligence

@github-actions github-actions Bot added the size/medium PRs with less than 250 changed lines label Dec 12, 2024
@ShareASmile ShareASmile added the bug Issue is related to a bug label Dec 15, 2024
@sonarqubecloud
Copy link
Copy Markdown

Comment thread app/src/main/java/org/schabi/newpipe/error/AppLifecycleObserver.kt Outdated
@github-actions github-actions Bot added size/small PRs with less than 50 changed lines and removed size/medium PRs with less than 250 changed lines labels Dec 17, 2024
@Profpatsch
Copy link
Copy Markdown
Contributor

Can we add a button to the Settings -> Debug menu that does a delayed crash, so you can simulate the problem here? I don’t know how to reproduce it.

@Thompson3142
Copy link
Copy Markdown
Contributor Author

I don't know how necessary it is to have a dedicated button for that, but if you want to reproduce it, you can do it very easily using something like:

new Handler(Looper.getMainLooper()).postDelayed(() -> {
            // Deliberately throw a RuntimeException
            throw new RuntimeException("Simulated crash for testing purposes.");
 }, 20000);

You could create a new button like the crashTheAppPreference in DebugSettingsFragment with this snippet, what i did was simply calling it when the player is opened but thats up to you.

@Profpatsch
Copy link
Copy Markdown
Contributor

I couldn’t reproduce it by either inserting that into MainActivity nor Player; do you have a commit with the exact position where you added that?

@Thompson3142
Copy link
Copy Markdown
Contributor Author

I never commited it because I don't wanted to mess with the git history, maybe I am missunderstanding though what you can not reproduce?

@Profpatsch
Copy link
Copy Markdown
Contributor

what you can not reproduce?

I cannot reproduce the error screen popping up and taking focus when NewPipe is in the background.

If you want, you can commit the example on a fresh branch and just link the branch or the commit directly.

@Thompson3142
Copy link
Copy Markdown
Contributor Author

Maybe it's something device specific? I can provide a commit that I used to test but it really just boils down to adding the code I mentioned into the first line after handleIntent in the Player class. Also can you not reproduce it in general or just with my attempt to fix it?

Stypox
Stypox previously requested changes Jan 25, 2025
Comment thread app/src/main/java/org/schabi/newpipe/MainActivity.java
@Stypox
Copy link
Copy Markdown
Member

Stypox commented Jan 25, 2025

Maybe it's something device specific?

Yes I think from Android 13 the background intents are blocked anyway

@Profpatsch
Copy link
Copy Markdown
Contributor

Ah, so I need to test on an older Android version, I see.

@Profpatsch Profpatsch force-pushed the fix_background_crash_focus branch from 5dc48af to 4f28e70 Compare May 9, 2025 20:28
Fix crashing behaviour with entry in SharedPreferences

A few minor improvements

Added docs for isInBackground

Some more minor changes

Overwrite methods in MainActivity instead of creating a new class
@Profpatsch Profpatsch force-pushed the fix_background_crash_focus branch from 4f28e70 to 76202e6 Compare May 9, 2025 20:29
@Profpatsch
Copy link
Copy Markdown
Contributor

Squashed & rebased.

I’d merge once the checks go through because it seems a simple enough change and I can’t really test.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 9, 2025

@Profpatsch Profpatsch requested a review from Stypox May 9, 2025 21:38
@Profpatsch Profpatsch dismissed Stypox’s stale review May 9, 2025 21:41

shared preference gets saved over crash

@Profpatsch Profpatsch merged commit f3858e7 into TeamNewPipe:dev May 9, 2025
7 checks passed
whistlingwoods pushed a commit to whistlingwoods/FoxPipe that referenced this pull request Jul 27, 2025
Stypox added a commit to Stypox/NewPipe that referenced this pull request Aug 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue is related to a bug size/small PRs with less than 50 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not steal focus to report a guru meditation

4 participants