-
-
Notifications
You must be signed in to change notification settings - Fork 692
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
base: master
Are you sure you want to change the base?
Conversation
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. |
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. |
#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. |
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. |
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? |
That's probably unavoidable downstream short of defaulting to release of resources onPause.
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.
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:
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() |
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):
onPause
event fires (minimize, lock screeen, etc.):about:blank
onResume
fires (screen unlocked, app restored from background):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
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes