New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 806228 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug
66



Sign in to add a comment

Empty tab preview in tab switcher

Project Member Reported by mar...@mwiacek.com, Jan 26 2018

Issue description

Device name: S7

From "Settings > About Chrome"
Application version:66.0.3332.0
Operating system:7

URLs (if applicable):any

Steps to reproduce:
(1) open few tabs
(2) close the tab on the bottom (current one)
(3) click Undo on popup showing that tab was closed

Expected result:

preview of the page

Actual result:

many times title of tab/webpage is shown in tab switcher, but preview is empty although site is 100% loaded.
 
Labels: Needs-triage-Mobile
Cc: sandeepkumars@chromium.org
Components: -UI UI>Browser>Mobile>TabSwitcher
Labels: 66 Triaged-Mobile
Status: Untriaged (was: Unconfirmed)
Tested the issue in Android and could reproduce the issue.

Steps Followed:
1. Launched Chrome .
2. Opened few tabs
3. Tried to close a tab and clicked on undo button
4. Observed the reopened tab doesn't have preview

Chrome versions tested:
65.0.3332.0

OS
Android 6.0.1

Android Devices
Samsung S7

Considering this issue as Non-Regression issue as same behavior is seen since M52.

Please navigate to below link for log's and video--
go/chrome-androidlogs/806228

Cc: mdjones@chromium.org rlanday@chromium.org
Labels: android-fe-triaged
This is an interesting issue.  I do agree that it is weird and not ideal, but I don't think we should fix it.

Without doing a full investigation, I suspect the following is happening.

1.) When you enter the tab switcher, the last selected tab is still "live".  We are rendering whatever we get back from the renderer process.
2.) When you close that tab, we automatically select the previous tab.  This causes this tab to now be live.
3.) When you undo the first deletion, we bring back that tab and attempt to show a snapshot of the page.  This is the problem because we never captured a snapshot because it went from live -> marked for deletion.

Conceptually, we could capture the tab that is marked for deletion just "in case" you undo the deletion, but that is going to cause a lot of churn for something that simply doesn't happen enough.

I'm inclined to say WontFix because I don't want to add code to slow down the normal path for the infrequent one.  With the caveat, that this doesn't look great, but might be the best compromise.  I'll let mdjones@ and rlanday@ make the final decision though.
I think there are probably a number of fit and finish issues we could address with the tab picker to make it feel nicer. So I might be inclined to actually fix this one. You’re worried about slowdown from actually capturing the preview? Is it complicated to delete the preview once the close is no longer undoable?
The tab being closed has to be the live one in order to reproduce this bug. I tried closing/restoring inactive tabs and the preview was always there. So this is almost certainly a case of us not having a snapshot available when the tab is closed. I'm inclined to agree that this is a WontFix since it would require the overhead of capturing a screenshot for very limited use.

It looks like rlanday@ is going to investigate further, so I'll leave it open for now.

Comment 6 by mar...@mwiacek.com, Feb 7 2018

I've just opened two more bugs for Tab Switcher: https://bugs.chromium.org/p/chromium/issues/detail?id=810086, https://bugs.chromium.org/p/chromium/issues/detail?id=810089

My experience say, that small bugs are many times moving into big one (that's why it would be good to fix this one), from the second hand this discussion looks very good and reasonable (thx for tedchoc@ for starting it in #3)
Owner: rlanday@chromium.org
Status: Assigned (was: Untriaged)
How come it doesn’t happen when there’s only one tab open?
I'd guess that it's the live tab when you close it, and since there are no other tabs, it is still the live tab when you re-open it (therefore no thumbnail required).
Cc: dtrainor@chromium.org
@4, I'm worried for all aspects of capturing the screenshot.  The screenshot itself is a bitmap capture of the page, which can cause a multiple MB allocation.  I believe this is all done natively, but it used to capture natively and store in java and thus be double the size.

The screenshot capturing is off the UI thread, so I would expect actually very little jank.

I think undo-ing a tab is done less than half a percent of the time (AndroidTabCloseUndo.Toast).

I'm wondering if we could capture a screenshot for background tab.  The question I don't know is whether we can capture a screenshot for a tab who's not currently producing content.  In the case you close a tab in the switcher, we actually don't "really" close it until the snackbar disappears, so there is a chance the renderer of the closed tab could still be alive when the close is undone (but it likely will not be selected).

Another potential fix is to not actually select another tab while there are close snackbars shown and only do that once they've expired.

+dtrainor who added the close snackbar.
@3 Is correct.  I think this might be fixed if we re-focused the tab when we un-closed it.  It'd start producing frames again.  Right now it looks like we don't update the focused index when this happens.  This might be the right fix if it's a good user experience?

@4 - We could grab a thumbnail for this but it does do a lot of work and definitely isn't free.  In general here's what happens when we need to grab a thumbnail:
- Pin the texture so we don't lose the frame even if you select another tab.
- Redraw to a frame buffer.
- Pull that data back to the CPU.
- Push the new thumbnail into the thumbnail cache system which will:
  - Compress it to ETC1 (to save space).
  - Write it out to disk.
  - Potentially evict another thumbnail for another tab.

@8 - I think this gets fixed because of my earlier comment on #3.  When we restore the tab it gets selected so we start producing real frames for it again.

@9 - We could do this but I think a lot of the system relies on there being a valid selected index if we have tabs?  At least I'm not 100% sure what doing something like exiting the tab switcher would do.  It might work... it might also require some fun code changes.
What is the concrete concern here? If I have 50 tabs open, we've already run the tab capture logic 49 times for the 49 inactive tabs, right? We're worried that running it one more time upon closing a tab is going to be problematic?

Is it possible that more people would be using the undo button if they weren't getting confused or dissuaded by the blank preview?
@11 - Yeah it might be worth playing around and trying!  I'd be more worried about evicting another high res snapshot (we only store 5/6 in memory - forget the # on disk - IIRC), although it probably won't be noticed.

Could we try my suggestion at the top of #10 though, if acceptable to UX?
@11, the concern is that this is doing unneeded work.  It will block threads that could have been otherwise doing something better for the user.  I have concerns this will jank things unexpected that local testing wouldn't find, that it will cause OOMs unnecessarily, that it would potentially kill other background tabs due to memory pressure, that it causes more IO for something that is done so infrequently.  In your example, we've saved 49 tabs, but we didn't necessarily do it at the same time and likely happened over a long time frame.

I'm more convinced that we shouldn't do this based on the number of things that "could" go wrong by doing unnecessary (at least in my opinion) memory churn.

If you wanted to run an experiment to see if this had any impact on undo, then I think that is something you could pursue.  But I'd be willing to bet that you'd see no impact on increased usage and I'd again push back on launching this if it didn't improve the usage.

I'd still be tempted to pursue not changing the selected index (or having an unset one...both are scary I know).  Selecting tabs is also quite expensive and potentially does network requests, potentially evicts the pending closed tab and other fun stuff.  With the current tab list implementation, I don't think we can have a selected index that is a value in the pending close list, so I do think this could be a headache.  I think dtrainor's comment in @11 seems OK, maybe not the ideal, but a decent compromise.  
Status: WontFix (was: Assigned)
Ok, I'm going to close this as WontFix.

Maybe at some point we should add a way to enable tab picker enhancements (e.g. higher resolution, non-JPEG previews) for devices with enough memory.

Comment 15 by mar...@mwiacek.com, Feb 12 2018

I'm very confused now and please give me hint, what is wrong with my thinking and why we don't have things done this way:

1. we have tab with all data
2. we press "x" and tab is hidden
3. user see info "you can undo deleting tab" for 2 seconds
4. if user pressed undo, tab is unhidden; if user haven't pressed undo, tab is deleted

what is missed now? why some data seems to be deleted before undo possibility is closed?

could you explain it to such newbie like me?
There are (at least) two ways to fix the problem:

1. Take a screenshot of the tab before closing it. This is very easy to do (see https://chromium-review.googlesource.com/c/chromium/src/+/907321) but there's some concern that the process of taking the screenshot might stir up other problems because it performs a large memory allocation (see comment 13).

2. Keep the data for the tab in memory until the user switches to a new tab (which eliminates the undo option) or hits "undo." The problem with this is we're not sure we're allowed to have the "current" tab be one that we've already started closing, so we're thinking this approach might be more trouble than it's worth.

Comment 17 by mar...@mwiacek.com, Feb 12 2018

thx,

I'm no so familiar with this code to understand, why option 2 is risky, but can be maybe done something with transparency? 

(I mean - tab preview for tab marked for deletion is going to be 100% transparent/invisible until tab is completely closed).

I'm just loud thinking, I'm sorry if it looks funny.

Comment 18 Deleted

Comment 19 by mar...@mwiacek.com, Feb 12 2018

or can we ask renderer to give preview again when tab is returned?
Cc: sbirch@chromium.org yus...@chromium.org
 Issue 852154  has been merged into this issue.

Sign in to add a comment