Whitelisted tabs being suspended

[deanoemcke]

As posted by Jonathan Hilton on the chrome webstore support forum:

The GS is suspending whitelisted tabs. I suspect it is due to the "instantly suspend when system memory gets very low" setting?

[deanoemcke]

You may be correct about this. I didn't foresee this being triggered that often, but you've helped me realise this may seem like a bug to a lot of users.
What it is really doing is detecting when chrome uses its native 'discarding' and instead suspending the tab.
The reason for this is that discarded tabs cannot be manipulated with the extension, so I was trying to avoid another apparent bug which is that tabs that should suspend would not due to having been discarded first.

An easy fix would be to check the whitelist status of the tab first, and allowing whitelisted tabs to discard but not suspend. I'll try to implement this as a hotfix.

[fwextensions]

@deanoemcke please turn that feature off by default! I just tried loading master from GitHub and it hammered by CPU:

I had to kill Chrome to regain control of my machine. Not as bad as #687 but it still took several minutes of slowly navigating to and launching Process Explorer before I could kill it.

I guess it was trying to undiscard and then suspend every discarded tab when the extension was enabled? I'm not convinced this feature is a good idea at all, but if it's available, I think it should only work prospectively, as tabs get discarded in the future. Trying to reload hundreds of discarded tabs at once will bring down older machines.

This is Win7 Chrome 68.

[deanoemcke]

I think you're probably encountering this bug actually: #722

It's being worked on right now. Sorry about this..

[fwextensions]

It's not clear what's happening in that bug, but what I saw was the CPU spiking as soon as the extension loaded. Tabs in the background, which I'm pretty sure had been discarded, started showing spinners, as if they were loading. That would be consistent with them being undiscarded, causing them to start loading, in order for them to then be suspended.

What's strange is that I don't think I lost any tabs after killing Chrome, restarting it, restoring the tabs, and immediately turning off the 104 version of the extension. I guess it either didn't successfully suspend any tabs or what was restored was the set of URLs immediately before installing 104 from a local folder.

[deanoemcke]

@fwextensions are you still CPU issues on the latest master? all known bugs have now been fixed.

[fwextensions]

@deanoemcke Is the change to call queueTabForSuspension() instead of forceTabSuspension()? I was looking at the pulled diff and didn't see anything else that seemed like it would be relevant to discarded tabs getting switched to the suspended state.

Anyway, this time I had proc explorer ready to kill Chrome if needed. I enabled the latest version from GitHub and... it mostly seemed fine. Some CPU spiking, but the machine was still usable:

This was with 362 tabs open across 22 windows. 330 of those were discarded, and 56 of the discarded tabs had also been suspended previously by either the Chrome store version or the .72 alpha (both were running).

However, I did notice that all the non-discarded suspended tabs got unsuspended. This time, I had only 7 of those. If all of those had been unsuspended at once (and usually I have more like 150 suspended) then I think the performance would've tanked. I looked at the code but couldn't tell where the startup sequence might be trying to unsuspend all those tabs.

Oh, I guess it's from checkTabsForResponsiveness()? Seems like pingTabScript() will reload a suspended tab that's not responding, but these tabs weren't suspended by the 109 version of the extension, so it's not surprising they didn't respond. The if (gsUtils.isSuspendedTab(tab)) { line seems like it should pass true to do an exact match, so that the extension isn't trying to reload tabs it didn't suspend, since reloading a suspended tab will normally cause it to load the unsuspended page, right?

It's certainly an edge case that would only affect people trying non-store versions, but I suspect this may be what happened with my #687 issue. The latest alpha at that time was probably treating all my suspended tabs as unresponsive and reloading them all at once, locking up the machine.

Also, I don't think it played into what happened to me before, but the processRequestTabSuspensionQueue() function may be a little aggressive. If I'm reading the code right, it would try to suspend 20 tabs/second until the queue is empty. With memory pressure and hundreds of tabs, I could see that locking up a machine, especially if most of the tabs it's trying to suspend are already discarded (I don't think it checks for this). Chrome would be trying to spin up 20 processes/second, reload each one's page, and then immediately switch each to the suspended page, which would take way more memory then just leaving them alone in the discarded state.

[fwextensions]

One other note: I'd had the 104 version installed, but disabled. I simply re-enabled it for the test above, which is when the suspended tabs got unsuspended. I noticed that the Extensions page still said 104, since Chrome doesn't reload the manifest when you re-enable an extension. I also noticed there was no logging in the background page, which I then saw was disabled in the code. So I set debugInfo to true and reloaded the extension. I captured the log from that reload here.

One thing I noticed was that it logged Reinjecting contentscript into unresponsive active tab. 56 times but with only 36 unique tab IDs. Shouldn't it work the first time?

For beta testers, you might want to set debugInfo based on the result from chrome.management.getSelf(). If installType == "development", then you could enable debugging, and leave it off otherwise.

[fwextensions]

Thinking about this a little more, it's not clear to me why it would matter if a suspended tab was unresponsive. If the user does an unsuspend all, and the content script doesn't respond to the command, the background script can just set the tab's URL directly. If the JS on suspend.html itself has crashed, then if the user wanted to unsuspend, they can just reload the page or click the title, which is a link to the page and should work without JS.

[deanoemcke]

@fwextensions there's a lot to unpack here.
firstly, to simplify things i'd like to leave complications due to using multiple versions of the extension off the table.

i'd also just like to clarify regarding the 2 different issues raised in this github ticket.

1. whitelisted tabs being suspended was indeed fixed with the change from forceTabSuspension() to queueTabForSuspension(). The former will force suspension of a page regardless of it's whitelist/etc state, whereas the latter will respect all the 'never suspend' options.
2. for the excessive cpu usage, that was a different issue (Excessive CPU usage with v7.0.104 #722) and was resolved by simplifying the dom and css of a suspended tab. the fix for this was wrapped up in the same update as the above issue.

i'm not sure where you read that processRequestTabSuspensionQueue() will try to suspend 20 tabs per second? it should only request a new tab suspend itself after the queue length (of tabs currently suspending) is less that MAX_TABS_IN_PROGRESS which should be 5 if screen capture mode is off and 3 if it is enabled.
so in fact, most of the seconds it does not queue any new tabs for suspension. only when one of the previous ones has finished suspending.

in regards to checkTabsForResponsiveness on suspended tabs. i'd rather that every tab, suspended or not, remains responsive to queries from the background script. there's a few features that rely on this communication, and it would appear buggy if some tabs did not behave the same as the rest (just look at the message listener of a suspended tab to see what is possible).

sorry if i haven't covered everything in your posts. i've got some other things to look into so need to be somewhat frugal with my time :)
but thanks for your significant contributions to this project. it's really appreciated.

[fwextensions]

Sorry for the novel above, and for the confusion on processRequestTabSuspensionQueue(). I didn't get that, on the next call, inProgressTabIds would get built back up and therefore prevent new ones to be queued. (Though MAX_TABS_IN_PROGRESS is set to 10 at the top of the file, and I didn't see that it was later changed to 5 or 3.)

I was posting on this issue as I thought what the 104 version was doing was undiscarding and then suspending every discarded tab due to the new "instantly suspend" option. What I now think it was doing was what I saw 109 do, which was unsuspend every suspend.html tab from another installed version of the extension. I can open another issue for that.

I've been pushing on these issues because the only times I've had trouble with TGS is when something has triggered a mass unsuspend and it locks up my machine. So I've been on the lookout for things that could cause that (like the still scary Unsuspend All menu item :).

[saurabhgayali]

In my PC whitelisted google pages are being suspended even in enough free memory just cause of long waiting time. Kindly fix.