1753085 - [Bug]: previous tab reloads in a flash before next tab is painted when switching tabs by swiping

Status: NEW

(Core :: Graphics, defect, P3)

Description

[[ex-Mozilla] Agi Sferro | :agi]

From github: https://github.com/mozilla-mobile/fenix/issues/23469.

Steps to reproduce

Open new multiple tabs
Swipe between them until a tab needs to be reloaded (the issue also happens when the next tab is reloaded from memory but less visible)

Expected behaviour

The next tab should be painted directly

Actual behaviour

On swiping :
The url changes to the one of the next tab
The screen gets dark
The previous page is re-displayed as a flash, for a very short time
Then only the new page is painted, either from memory or by reloading it
See here : https://photos.app.goo.gl/2bLSzhhxdbpZ59pn9

Device name

Redmi note 7

Android version

Android 9

Firefox release type

Firefox Nightly

Firefox version

98

Device logs

No response

Additional information

No response

Change performed by the Move to Bugzilla add-on.

Comment 1

[Emily Toop (:fluffyemily)]

Is this a graphics bug?

Comment 2

[Miko Mynttinen]

(In reply to Emily Toop (:fluffyemily) from comment #1)

Is this a graphics bug?

This is a good question. The original github report mentioned investigation related to restoring tabs on Android.

Comment 3

[Jamie Nicol [:jnicol]]

I cannot reproduce what's described in this report at will, though I have accidentally a few times. Specifically that it shows the previous tab. But it's trivial to reproduce it displaying stale content of the correct tab:

• Close all tabs and firefox. reopen firefox
• Open tab 1 and load a page.
• Open tab 2 and load a different page
• Scroll down the page in tab 2
• Swipe the address bar back to tab 1
• Swipe the address bar back to tab 2
• Tab 2's page will momentarily be displayed at the initial scroll position or even partially loaded, before jumping to the correct position.

I can't think of how webrender would be responsible for that, though it's certainly possible. But my hunch is that Fenix is displaying a screenshot of the page while it waits for us to render the page. If that is true, then I think it's most likely that this bug is caused by Fenix displaying the wrong page's screenshot. (Or perhaps geckoview/webrender took a screenshot of the wrong page in the first place.)

Comment 4

[Mark]

Reliable STR (mostly) happens if you drop Fenix into the background.

• open non-trivial tab 1 and tab 2 (I used bbc.co.uk/news and mozilla.org but it works on most sites)
• switch to tab 1
• reload tab 1 and QUICKLY
• • swipe to tab 2 and then switch to the Home screen ie put Fenix in the background with tab 2 showing (I think you need to do this before tab 1 has fully loaded)
• wait a few seconds for tab 1 to fully load in the background
• bring Fenix to the foreground
• swipe from tab 2 to tab 1
  -> you'll see tab 2 content flash briefly before tab 1 displays - fairly reliable but not 100%

Other issues, I suspect they are all related??? But while you are looking at this ;)

(a) In these STR I don't see the "black flash" but that is a real thing sometimes, I think when a tab was suspended and reloads?

(b) When Fenix comes to the foreground it's a mess, during the about 0.5 second zoom animation it shows (i) the whole page then (ii) just the URL bar then (iii) the URL bar and the notification bar but not the content then (iv) the whole page again. . I'll attach a screen grab.

Enjoy

Comment 5

[Mark]

Attachment: Screenshot 2022-02-09 at 18.10.51.png

Comment 6

[[ex-Mozilla] Agi Sferro | :agi]

Jamie: talking to the AC/Fenix team they told me from their investigation they believed this was due to the surface displaying stale data before WR/Gecko had a chance to update it again, so yeah it might not be a WR bug specifically.

Comment 7

[Jamie Nicol [:jnicol]]

That sounds plausible, though I don't understand why it'd show stale content of the tab being switched to.

But in any case if it's the Surface's fault can we just use the coverUntilFirstPaint mechanism to hide it?

Comment 8

[[ex-Mozilla] Agi Sferro | :agi]

We do use coverUntilFirstPaint, AFAIK. This could be a bug in it actually.

Comment 9

[Jamie Nicol [:jnicol]]

This seems to be the only location we cover it. And only if hasSession is false... which I presume is not the case if we're switching to a loaded tab? From a quick search of the code we seem to always pass a non-null session, meaning we don't cover it?

(Also, is it a bug that we always cover WHITE there instead of it depending on the theme?)

In android-components they call it here. They set a different colour there depending on the theme, but we don't cache that. Should we be caching the colour and using it whenever geckoview itself decides to cover?

Comment 10

[Jamie Nicol [:jnicol]]

I think this is Fenix doing something like I guessed in comment 3: https://searchfox.org/mozilla-mobile/rev/c8ec80d154917909a0cfe402d7cb895af469fe67/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt#46

I think webrender in the past was very guilty of always flashing white instead of respecting the background colour. And perhaps we weren't using coverUntilFirstPaint as much as we should, so Fenix tried to work around it with screenshots. But I think now we're in a bad place of sometimes showing the wrong screenshot, followed by a blank screen, then finally the actual rendered page.

Even if the screenshot was always of the correct page and up-to-date, I think part of the problem is it's really difficult for webrender to know when what it has rendered is the actual page or just an empty background. So the onFirstPaint callback fires after the first composite which is probably a blank page (although it should now respect the theme's background colour at least). That means if we're using that callback to decide when to remove a screenshot, it's going to go screenshot -> blank -> page rather than just screenshot -> page. If we just cover with the background color instead of using a screenshot then it'll be blank (because of coverUntilFirstPaint) -> blank (because of webrender) -> page, which should look better from the user's perspective.

I could of course be misunderstanding what that Fenix code does in which case everything I wrote above is irrelevant :)

Comment 11

[Mark]

https://github.com/mozilla-mobile/fenix/issues/23825