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

feat(webview): optional unload on pause #5051

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kariudo
Copy link

@kariudo kariudo commented Feb 18, 2025

Summary

As #979 caused me to use up several GB of mobile data in short order due to closing the app with a dashboard of 10 cameras open via the frigate card, I thought I would just fix this myself and solve my own problem.

My solution (which works for me in my testing), is to offer a setting to support a highly effective (if not slightly aggressive) approach (somewhat recommended by google for freeing resources in webviews):

  • Add Unload when backgrounded setting
  • If onPause event fires (minimize, lock screeen, etc.):
    • Confirm settings and state, and unload the webviews resources by loading about:blank
  • If onResume fires (screen unlocked, app restored from background):
    • Confirm settings and state, and simply "go back".
    • User will return to the last screen (URL).

The obvious caveat here to the solution is that it means pausing the app means the previous view must be reloaded (however, this is obviously needed when unloading has happened and ensures that no broken weirdness from trying to suspend the webview happens, at the trade-off of a reload).

Screenshots

Screenshot_20250218-160850

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

@jpelgrom
Copy link
Member

This makes no sense to me as an end-user feature. There is a bug, and either 1) the bug should be fixed in the frontend/webview/exoplayer/wherever the problem lies, or 2) there should be something smart to do this. You're proposing implementing a workaround. As a normal user, or if the issue is fixed, this has no value.

@kariudo
Copy link
Author

kariudo commented Feb 21, 2025

The bug is that there is an undisposed web view, so this conditionally in the most convenient way disposes of the resources. The only other practical option is to completely dispose the web view activity which seems silly.

This is a major problem for anyone who uses cameras.

Functionally this isn't so different than the existing option to navigate to the default dashboard URL every time the app opens; however that doesn't happen until the resume event, not the stop event.

@andonevris
Copy link

This makes no sense to me as an end-user feature. There is a bug, and either 1) the bug should be fixed in the frontend/webview/exoplayer/wherever the problem lies, or 2) there should be something smart to do this. You're proposing implementing a workaround. As a normal user, or if the issue is fixed, this has no value.

#979 has been ongoing for almost 5 years, yes that's 5 YEARS.

At this point I will take ANY fix even if it is a workaround, there seems to be no interest if fixing this the "proper way" whatever that means. I see no issue putting this fix in, it's optional and people who don't need it can leave things as they are.

For the people that this issue affects it's the most infuriating bug, coming back to your phone and realising you've just dumped 50% battery for no reason is ridiculous.

@MikeVensel
Copy link

This issue REALLY needs to be fixed and I see no reason not to take this fix. I don't even understand how this could be viewed as a workaround. The app is in the background. There should be no view refreshing in the background. Hell this "workaround" setting should be enabled by default. I can't imagine anyone expects an app to be using data and power to update a view they can't see in an app that is currently suspended.

@dshokouhi
Copy link
Member

So the reason why we call this a workaround is because it needs to be discovered. Users dont know about #979 until they get bit. Likewise users will not know about the fix until they get bit AND they do research to find the setting to use it.

Considering the current changes, we had a user also mention Chrome has the same exact issue (#979 (comment)). Chrome does not navigate to a blank page when it is paused to free up resources. So this fix would only benefit the app when the issue exists elsewhere out of the apps control.

@kariudo have you tested the changes when PiP is used? What happens there when this is enabled and user triggers PiP? We need to consider existing functionality here

Also, you mentioned that Google sorta recommends this for freeing up resources can you share that link with us?

@kariudo
Copy link
Author

kariudo commented Feb 25, 2025

So the reason why we call this a workaround is because it needs to be discovered. Users dont know about #979 until they get bit. Likewise users will not know about the fix until they get bit AND they do research to find the setting to use it.

That's probably unavoidable downstream short of defaulting to release of resources onPause.

Considering the current changes, we had a user also mention Chrome has the same exact issue (#979 (comment)). Chrome does not navigate to a blank page when it is paused to free up resources. So this fix would only benefit the app when the issue exists elsewhere out of the apps control.

I think their expectations are that consumer has context for what should be done, rather than behaving as iOS does (to my knowledge) and always disposing for resources.

@kariudo have you tested the changes when PiP is used? What happens there when this is enabled and user triggers PiP? We need to consider existing functionality here

I had not; however, I just did and in terms of frigate video streams on the frontend hosted in the Android app (I assume that's your question), the onPause event is still emit when pip is active (full screen does not cause an emit). So presumably for pip support to work with these changes it would need to check for:
https://developer.android.com/reference/android/app/Activity#isInPictureInPictureMode()

Also, you mentioned that Google sorta recommends this for freeing up resources can you share that link with us?

Sure. It is in the webview API doc, following the deprecation notice for the former clearview() method as the recommendation for reliable release of resources:

https://developer.android.com/reference/android/webkit/WebView#clearView()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants