Crosh extension incorrectly protects all tabs in window against discarding |
||||
Issue descriptionWhen TabManagerDelegate retrieves a list of TabLifecycleUnits (active tabs) in preparation for killing some tabs to reduce memory pressure, all of the tabs are categorized as PROTECTED_BACKGROUND. I've seen this behavior during manual memory pressure tests with anywhere from 4 to 60 tabs. Tabs can be categorized as FOCUSED_TAB (currently focused tab), BACKGROUND (unfocused tabs), or PROTECTED_BACKGROUND (unfocused tab that TabManagerDelegate should avoid discarding). Normally, tabs are protected from discarding if there is some unsaved state (unsubmitted form, partial URL in navigation bar), if it is perceptible by the user from background (playing media, notifications) or protected by an extension. From logging the decision_details object that is used to decide whether a tab should be PROTECTED_BACKGROUND (https://cs.chromium.org/chromium/src/chrome/browser/resource_coordinator/tab_lifecycle_unit.cc?rcl=f213164d219da210c4c0383d4878c02633599965&l=529), it looks like the tabs have their autoDiscardable property set to false by an extension. By default, this property is true. When there are no extensions installed according to chrome://extensions, the tabs still have autoDiscardable set to false, and are categorized PROTECTED_BACKGROUND.
,
Dec 10
Hi wvk, Are you sure it's disabled by some extension? Do you mean the failure_reason is always LIVE_STATE_EXTENSION_DISALLOWED but no other values? What do you see in about://discards ?
,
Dec 11
In latest versions of Chrome OS, a PROTECTED_BACKGROUND_TAB can be discarded when there is memory pressure. The "protection" only means that it won't be discarded if discarding all non-protected tabs is not enough to exit the memory pressure state. Can you provide the Chrome version and complete log when you observe this issue?
,
Dec 18
Sorry, it looks like I might've filed this bug unnecessarily. I took another look at chrome://discards and it looks like only the bookmarked tabs are protected by an extension. When I was manually creating memory pressure, I had launched all of the tabs from bookmarks, which is why I saw them all get ranked as PROTECTED_BACKGROUND_TAB.
,
Dec 18
You have an extension that protects bookmarked tabs? I don't think this is a functionality provided by Chrome.
,
Dec 19
Not that I'm aware of. These bookmarked tabs are ranked as PROTECTED_BACKGROUND_TAB with reason "Tab is protected by extension" even when no extensions are installed (according to chrome://extensions).
,
Dec 19
"Tab has been protected by an extension" should only appear when an extension protects a tab using the autoDiscardable API (https://developer.chrome.com/extensions/tabs) or when you simulate this by clicking in the "Auto Discardable" column in chrome://discards. If you reproduce this, can you provide the exact steps + your Chrome version number? Thanks!
,
Dec 19
I did a little bit more digging, and it seems that it is not bookmarked tabs, but rather the crosh extension. I noticed that tabs become PROTECTED_BACKGROUND as soon as crosh is opened, and indeed, the code responsible is here: https://cs.corp.google.com/chromeos_public/src/third_party/libapps/nassh/js/nassh.js?rcl=dc00f9157f9ad498b627d952697ca8a23417ff2e&l=246 nassh.disableTabDiscarding() is called at window.onload, and all the tabs in that window have their autoDiscardable property set to false. I was seeing this behavior in my tests because crosh was one of my bookmarked tabs. I'm unsure whether this is the extension's intended behavior, because in the original bug ( https://crbug.com/868155 ) it seems like they intended to prevent only the crosh tab itself from being discarded.
,
Dec 19
The steps I took to reproduce on caroline R73-11424.0.0: 1. Open a few tabs (e.g. reddit.com, nytimes.com, bbc.com) 2. Open chrome://discards and note the reasons under 'Can discard?' for each tab 3. Open crosh (Ctrl+Alt+T) in the same window 4. Open chrome://discards again, all tabs should now have 'Tab has been protected by an extension' in their 'Can discard?' section
,
Dec 20
wvk@: Thanks for investigating this. vapier@: The crosh extension protects all tabs in the current window. Could you update the code to instead protect only tabs that match the extension's url? Relevant code is here: https://cs.corp.google.com/chromeos_public/src/third_party/libapps/nassh/js/nassh.js?rcl=dc00f9157f9ad498b627d952697ca8a23417ff2e&l=246
,
Dec 20
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/apps/libapps/+/469621dbc9010b73f46ae137f082baf1d35b42d5 commit 469621dbc9010b73f46ae137f082baf1d35b42d5 Author: Mike Frysinger <vapier@chromium.org> Date: Thu Dec 20 19:39:33 2018 nassh: fix tab discarding to only affect current tab When we're opened in a tab, chrome.tabs.query will return all tabs in that window, so we'd end up disabling automatic tab discarding on all the other (non-crosh/nassh) tabs. Only look up the current tabid and disable tab discarding on it. Bug: 912786 Change-Id: Id197abb56e2087b6f6c80830263fa3e6a6482fd0 Reviewed-on: https://chromium-review.googlesource.com/c/1387066 Reviewed-by: Vitaliy Shipitsyn <vsh@google.com> Tested-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/469621dbc9010b73f46ae137f082baf1d35b42d5/nassh/js/nassh.js
,
Dec 21
Thanks a lot for fixing this quickly! |
||||
►
Sign in to add a comment |
||||
Comment 1 by sonny...@google.com
, Dec 7