Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix background crash focus #11789

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

Thompson3142
Copy link
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
Comment on lines 24 to 32
override fun onStart(owner: LifecycleOwner) {
editor.putBoolean(KEY_IS_IN_BACKGROUND, false).commit()
Log.d(TAG, "App moved to foreground")
}

override fun onPause(owner: LifecycleOwner) {
editor.putBoolean(KEY_IS_IN_BACKGROUND, true).commit()
Log.d(TAG, "App moved to background")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you override these functions directly in MainActivity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can, i just missed that AppCompatActivity has these methods to overwrite. Unfortunately it does not seem like it fixes the occasionally wrong behavior i mentioned, probably something related to ACRA but I am unsure how to fix it.

@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
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
3 participants