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

It is not possible to stop tabs from being unloaded in the background, even when setting relevant about:config settings, and we need a control to turn it off #115

Closed
izarsha opened this issue Sep 15, 2020 · 19 comments · Fixed by #157
Assignees
Labels
affects-upstream Issues that also affect Mozilla Firefox (Fenix) builds bug Something isn't working enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed todo Task queue

Comments

@izarsha
Copy link

izarsha commented Sep 15, 2020

Steps to reproduce

Page reload in the background even after adding this to the config
accessibility.blockautorefresh=true
browser.tabs.disableBackgroundZombification=true
browser.tabs.unloadOnLowMemory=false
browser.meta_refresh_when_inactive.disabled=true

Page is still reloading in the background. On old version of ff version 68, i do this step and it does not reload my pages. Does about:config working properly?

Expected behavior

Open about 20 to 30 tabs.
Page reloading if you started to browse to all of your tabs.

Device information

Xiaomi Redmi Note 8 128GB 4GB ram

  • Android device: 9.0 Pie
  • Fenix version: fork 10 latest
@ale82to
Copy link

ale82to commented Sep 16, 2020

same issue with fenix that's why I lef fenix /iceweasel unfortunately

@interfect
Copy link
Collaborator

It's possible not all about:config options will work as advertised, because AFAIK they're internal to the browser engine/GeckoView, which may not be in charge of loading/unloading tabs anymore with the split of the browser into android-components backend and fenix frontend.

It sounds like we want a setting in the real settings panel where, if you set it, we will never unload any tabs (until the phone kills the app for using all its memory).

@izarsha Is it that some of the tabs stay loaded while others reload when you go back to them? Or is it that you come back to the app and all the tabs have been unloaded (like the whole app was suspended and restarted)?

@interfect interfect added affects-upstream Issues that also affect Mozilla Firefox (Fenix) builds bug Something isn't working enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed and removed needs:triage labels Sep 17, 2020
@interfect interfect changed the title [Bug]Page is refreshed/Auto reload in background or in low ram. It is not possible to stop tabs from being unloaded in the background, even when setting relevant about:config settings, and we need a control to turn it off Sep 17, 2020
@interfect
Copy link
Collaborator

I have no idea where this behavior is controlled/implemented. Someone will have to dig for it.

@ale82to
Copy link

ale82to commented Sep 17, 2020

I have reported this problem to github multiple times starting from old Firefox preview 5.x still nothing

@izarsha
Copy link
Author

izarsha commented Sep 17, 2020

It's possible not all about:config options will work as advertised, because AFAIK they're internal to the browser engine/GeckoView, which may not be in charge of loading/unloading tabs anymore with the split of the browser into android-components backend and fenix frontend.

It sounds like we want a setting in the real settings panel where, if you set it, we will never unload any tabs (until the phone kills the app for using all its memory).

@izarsha Is it that some of the tabs stay loaded while others reload when you go back to them? Or is it that you come back to the app and all the tabs have been unloaded (like the whole app was suspended and restarted)?

I opened 10 to 15 tabs, when i scrolled back to the first tab it reloads/refreshed/loading. The app does not restarted/crashed or whatsoever. If you want a video of whats going on i can record it and send it to you. But literally its the same like chrome which discarded tab in the background. I know you understand that situation if youre using chrome. That is really inconvenient as i am a blogger, a writer and researcher, it refreshed all my works and things and most importantly what i type on my blog in the background after switching several tabs for the research and it is difficult for me to write back what i wrote as i sometime does not save it. Right now im using the old 68 ver of fenix/ff as this version doesnt gave me any problem yet. Thank you for your work devs. Even so, i still love this project but until the problem is solved, i wont use this app as my daily basis.

@interfect
Copy link
Collaborator

OK, that's important information that you can reproduce this by just opening tabs and not leaving the app.

I think some relevant code might be here:

override fun onTrimMemory(level: Int) {
super.onTrimMemory(level)
runOnlyInMainProcess {
components.core.icons.onTrimMemory(level)
components.core.store.dispatch(SystemAction.LowMemoryAction(level))
}
}

That's what the app does when Android tells it it is running out of memory. It sends a LowMemoryAction to the android-components backend.

Is your device memory running low when your tabs go missing? And can you open more simple pages/fewer complicated and memory-heavy pages before tabs start being reloaded?

Here's where android-components decides if it needs to drop tabs on the floor. I think "suspending" might really just kill the whole page and lose what you had typed in:

https://github.com/mozilla-mobile/android-components/blob/38186676d46c555b5a24268e5fa361e45e57102c/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TrimMemoryMiddleware.kt#L35-L47

We could try and hide the message from android-components, either all the time, or at some memory pressure levels when we disagree with the settings hardcoded here:

https://github.com/mozilla-mobile/android-components/blob/38186676d46c555b5a24268e5fa361e45e57102c/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TrimMemoryMiddleware.kt#L53-L64

@interfect
Copy link
Collaborator

But if we just suppress this message forever, you'll either switch back to Firefox and find the whole app rebooted and all the tabs dropped, or you'll have it killed out from under you as you are using it by the system. I don't think the underlying browser engine can keep infinity tabs loaded by swapping old ones out to disk without the page noticing.

@ale82to
Copy link

ale82to commented Sep 18, 2020

I just wanted all you to know I edited/smali/mozilla/components/browser/session/engine/middleware/TrimMemoryMiddleware.smali
replacing all 0x1 to 0x0.
fixed the issue I can now have as many tabs I like with no fenix crash.
(keep in mind I use Firefox nighlty maybe in today build the solved the issue although I don t think so)

@izarsha
Copy link
Author

izarsha commented Sep 18, 2020

OK, that's important information that you can reproduce this by just opening tabs and not leaving the app.

I think some relevant code might be here:

override fun onTrimMemory(level: Int) {
super.onTrimMemory(level)
runOnlyInMainProcess {
components.core.icons.onTrimMemory(level)
components.core.store.dispatch(SystemAction.LowMemoryAction(level))
}
}

That's what the app does when Android tells it it is running out of memory. It sends a LowMemoryAction to the android-components backend.

Is your device memory running low when your tabs go missing? And can you open more simple pages/fewer complicated and memory-heavy pages before tabs start being reloaded?

Here's where android-components decides if it needs to drop tabs on the floor. I think "suspending" might really just kill the whole page and lose what you had typed in:

https://github.com/mozilla-mobile/android-components/blob/38186676d46c555b5a24268e5fa361e45e57102c/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TrimMemoryMiddleware.kt#L35-L47

We could try and hide the message from android-components, either all the time, or at some memory pressure levels when we disagree with the settings hardcoded here:

https://github.com/mozilla-mobile/android-components/blob/38186676d46c555b5a24268e5fa361e45e57102c/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TrimMemoryMiddleware.kt#L53-L64

I dont know how to digest with that kind of information because im just merely a basic user but if youre planning to release an apk based on what you see as a potential fix as you mentioned "android-component" or by editing its smali like @ale82to mentioned, im willing to try it as a tester.

@ale82to
Copy link

ale82to commented Sep 18, 2020

honestly it's amazing with my stupid smali editing after extending test I can say on my 4 giga phone I could have as much as 20 tabs with no reload at all and most important of all no strange crashes at all.
if anyone wants to test my apk :
https://drive.google.com/file/d/1ku5UwR1uM5h2c0SbNk5UxRivXTWh7c26/view
keep in mind it 's based on fenix and has a black amoled theme instead of dark.

@Namit-Nayan
Copy link

@ale82to your mod apk seems to work, Will need to test further to see if any side effects.
@interfect default fenix behaviour is it keeps the last tab opened in the memory, and reload all others.
(last opened tab is the tab which you were on before switching app, or going to home screen of your device)
(while typing this I realized I cannot edit anything in between the line, cursor always jumps to end)

@interfect
Copy link
Collaborator

OK, @ale82to's smali spelunking does suggest that we should be able to fiddle with the memory messages and solve this.

I'd want to put it behind a customize setting, at least if we want to let you fully disable the dropping of tabs ever. I think it's still best to drop background tabs if the whole app is about to die. But we could, by default, let other apps in the background die before we drop our background tabs.

So I have to learn how to add settings...

@interfect interfect added the todo Task queue label Sep 19, 2020
@CharmCityCrab
Copy link

CharmCityCrab commented Sep 19, 2020

OK, @ale82to's smali spelunking does suggest that we should be able to fiddle with the memory messages and solve this.

I'd want to put it behind a customize setting, at least if we want to let you fully disable the dropping of tabs ever. I think it's still best to drop background tabs if the whole app is about to die. But we could, by default, let other apps in the background die before we drop our background tabs.

There are some battery use implications here, too, which is a much bigger deal for a smartphone or a tablet than a desktop or even a laptop. Holding a lot of tabs in active memory probably decreases battery efficiency as well as risking RAM related crashes or auto-closes.

These days almost every manufacturer with it's own variant of Android contains a slightly different system of power management as well, so apps can be put to sleep or closed for battery related purposes based on different criteria and at different times even if all the same apps with identical versioning and code are being run on two smartphones with different branding, the unpredictability of which could make finding a balance that works for everyone tricky.

So, there is some question to me about whether keeping that 10th tab running is actually protecting user generated but I submitted content safer or not, because it could trigger the OS to shut down the app, or the device to run out of battery power. There's potentially a certain false sense of security there, I think.

I am not against having an option, but I think Interfect is right, it should be an option and not a default. We also might want to consider a "middle ground" third option between "current and previous tab only" (Firefox) and "every tab" (proposed). Perhaps the middle option could be something like "everything up to 5, but after 5, current and previous four only" or something like that. The exact numbers could be tested or surveyed and fiddled with a bit based on feedback from the earliest versions that incorporate it when people see how it works "in the field" for them.

Another thought would be that specifically protecting tabs with user input like message forums, reddit, GitHub, social media, a job application, a word processing web app, etc. could be worth considering prioritizing simply because a reload on one of those might risk losing a partially typed message whereas reloading a news article would generally not. However, I don't know if code can be written to detect that. I guess the value that one would be looking for there would be "fields in which text can be input", but I am not sure there is a coding shorthand for that which allows a browser to be set up that way.

@Namit-Nayan
Copy link

Namit-Nayan commented Sep 19, 2020

Putting it as an option is great. Will benefit everyone.
And this workaround isn't perfect, but is lot better than unloading all but one tabs from memory when device has ram available to keep them and other browsers can keep them in memory under similar conditions.
Maybe put a warning "This setting may increase Battery Usage" along with the option.

BTW I think it causes more battery usage when tabs are reloaded again while they could have been kept in memory.

And the ideal scenario may have been keeping similar number of tabs in memory in comparison to chromium (or other browsers) while using similar amount of RAM and battery.

@izarsha
Copy link
Author

izarsha commented Sep 19, 2020

OK, @ale82to's smali spelunking does suggest that we should be able to fiddle with the memory messages and solve this.

I'd want to put it behind a customize setting, at least if we want to let you fully disable the dropping of tabs ever. I think it's still best to drop background tabs if the whole app is about to die. But we could, by default, let other apps in the background die before we drop our background tabs.

So I have to learn how to add settings...

I havent really got a time to test the apk provided by @ale82to yet but i think its a good idea to put it as an option in the setting with little warning "may increase battery life". Honestly i dont really care much about battery life as long as i get a nearly "perfect" browser that save tons of my work, battery life came second.

@interfect interfect self-assigned this Sep 26, 2020
@interfect
Copy link
Collaborator

I think this is really a RAM thing rather than a battery thing. It might incidentally increase battery usage to not have discarded tabs when RAM was low (bacause page JS can still be running), but it shouldn't use more battery than it would if you closed all background processes before loading all the same tabs.

interfect added a commit that referenced this issue Sep 26, 2020
This adds a setting in "Customize" to fix
#115
interfect added a commit that referenced this issue Sep 26, 2020
This adds a setting in "Customize" to fix
#115
@interfect interfect mentioned this issue Sep 26, 2020
3 tasks
interfect added a commit that referenced this issue Sep 27, 2020
* Add a setting to hide memory pressure from a-c

This adds a setting in "Customize" to fix
#115

* Drop trailing spaces
@em108 em108 mentioned this issue Oct 4, 2020
@Namit-Nayan
Copy link

Namit-Nayan commented Nov 20, 2020

I just wanted all you to know I edited/smali/mozilla/components/browser/session/engine/middleware/TrimMemoryMiddleware.smali
replacing all 0x1 to 0x0.
fixed the issue I can now have as many tabs I like with no fenix crash.

@ale82to can you tell how to do this.(step by step guide)
I downloaded apk editor but can't locate the smali file location.(probably doing something wrong)

@interfect
Copy link
Collaborator

interfect commented Nov 20, 2020 via email

@Namit-Nayan
Copy link

Namit-Nayan commented Nov 20, 2020

APK hacking should no longer be necessary: there's now a setting at the bottom of Iceraven's Customize that you can turn off to stop the default behavior.

I know this. (Had tested the debug apk in Die For Your Ram pull request.)(And thanks for having some easily workaround)
Asked because I wanted to try Firefox(beta), and it still has the bug.

Edit: back on iceraven, I couldn't do that edit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-upstream Issues that also affect Mozilla Firefox (Fenix) builds bug Something isn't working enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed todo Task queue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants