New issue
Advanced search Search tips

Issue 912786 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Crosh extension incorrectly protects all tabs in window against discarding

Project Member Reported by wvk@google.com, Dec 7

Issue description

When 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.

 
Cc: cylee@chromium.org fdoray@chromium.org semenzato@chromium.org conradlo@chromium.org vovoy@chromium.org
 I've seen this too.  Do we know which extension is doing this? 
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 ? 
 

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?
Status: Closed (was: Untriaged)
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.
You have an extension that protects bookmarked tabs? I don't think this is a functionality provided by Chrome.
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). 
"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!
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.
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
Components: -OS>Performance>Memory Internals>ResourceCoordinator Platform>Apps>Default>Hterm
Owner: vapier@chromium.org
Status: Assigned (was: Closed)
Summary: Crosh extension incorrectly protects all tabs in window against discarding (was: All tabs are PROTECTED_BACKGROUND in TabManagerDelegate)
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
Status: Fixed (was: Assigned)
Project Member

Comment 12 by bugdroid1@chromium.org, 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

Thanks a lot for fixing this quickly!

Sign in to add a comment