986782 - [e10s] Flash of random memory when switching tabs

Status: RESOLVED WORKSFORME

(Firefox :: Tabbed Browser, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

When switching tabs in an e10s browser, we appear to draw one (or maybe more) frames where the content is entirely black. It's very noticeable. This happens with the basic compositor on Linux and with the d2d11 compositor.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

er, d3d11 of course

Comment 2

[Andreas Gal :gal]

How can we test this better? Can we enable the basic compositor in parallel to accelerated compositor and pixel compare during an automation run?

Comment 3

[Matt Woodrow (:mattwoodrow)]

I strongly suspect this is a race condition.

We tear down the Layer tree for the old tab, and then start building the new layer tree. If the compositor triggers before we get the new layer tree uploaded, then we have nothing to draw for the content area and we're left with blank (which will be black for windows without transparency).

I don't know if there's a way to stop this entirely, since we don't wait to block the compositor on the child process.

B2G handles this by assigning a background color to the remote browser/tab object (in the parent process) so that we always have something to display, regardless of what the child is doing (being slow or crashing).

Comment 4

[Benoit Girard (:BenWa)]

I'm not a fan of using a background color. It means that if the content process takes 100ms to paint we're going to flash a solid color temporarily. I think we should wait with some timeout.

Tentatively taking this bug for now.

Comment 5

[Benoit Girard (:BenWa)]

This platform is architectural and not tied to a platform.

Comment 6

[Benoit Girard (:BenWa)]

Attachment: Force slow tabswitch testcase

This testcase is useful for seeing the problem and seeing what others do.

Comment 7

[Benoit Girard (:BenWa)]

Bugzilla already has a ? in the URL. You will need to download the testcase for it to work.

Comment 8

[Benoit Girard (:BenWa)]

Chrome: They will show the old content right away but you can't interact with it. This is bad because if you modify the DOM while the page is in the background you will show something that is very stale that will be replaced. We really shouldn't do this IMO!

Safari: They will wait ~300ms for the content before reacting to user input. After which they will display white regardless of the background color.

Firefox non-e10s: Jank.

Firefox e10s: Black flash. (This bug)

IE11: Switched the URL bar right away, waits until the content is ready to paint.

Comment 9

[Benoit Girard (:BenWa)]

I'm personally torn between Safari and IE11 but we should rope UX for advice.

Assuming we want to implement Safari or IE11' approach:
* Leave the old iframe intact
* Prepare the new iframe element and style it with 'will-change: opacity; opacity: 0;' (this will prevent visibility testing from optimizing away the paint).
* Request mozAferPaint in the child process
* If-and-only if we go with the safari approach: After 300ms we replace the old iframe with a background: white element.
* Once mozAfterPaint fires in the child remove the style attribute. The old iframe can now be removed.

Comment 10

[Benoit Girard (:BenWa)]

I believe this can be entirely fix by the front end, removing from the graphics component.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Benoit Girard (:BenWa) from comment #9)
> I'm personally torn between Safari and IE11 but we should rope UX for advice.
> 
> Assuming we want to implement Safari or IE11' approach:
> * Leave the old iframe intact
> * Prepare the new iframe element and style it with 'will-change: opacity;
> opacity: 0;' (this will prevent visibility testing from optimizing away the
> paint).
> * Request mozAferPaint in the child process
> * If-and-only if we go with the safari approach: After 300ms we replace the
> old iframe with a background: white element.
> * Once mozAfterPaint fires in the child remove the style attribute. The old
> iframe can now be removed.

Hi Phillipp, what do you want to do here?

Comment 12

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #11)
> (In reply to Benoit Girard (:BenWa) from comment #9)
> > I'm personally torn between Safari and IE11 but we should rope UX for advice.
> > 
> > Assuming we want to implement Safari or IE11' approach:
> > * Leave the old iframe intact
> > * Prepare the new iframe element and style it with 'will-change: opacity;
> > opacity: 0;' (this will prevent visibility testing from optimizing away the
> > paint).
> > * Request mozAferPaint in the child process
> > * If-and-only if we go with the safari approach: After 300ms we replace the
> > old iframe with a background: white element.
> > * Once mozAfterPaint fires in the child remove the style attribute. The old
> > iframe can now be removed.
> 
> Hi Phillipp, what do you want to do here?

From your descriptions, the behavior of IE11 seems to be the most reasonable.
I'm having problem to reproduce this though. I tried the test case, but it looks the same to me in every browser…

Comment 13

[Benoit Girard (:BenWa)]

To use the testcase you will need to download the file locally. For IE11 you will also need to serve it from an HTTP Server. You will need to append something like ?3000 for a second wait to the URL.

Comment 14

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Thanks for the clarification!

I'm not too concerned about showing old content, since in the majority of cases, the DOM shouldn't change while a tab is in the background. It also depends a little on what kinds of delays we are expecting. Are we talking milliseconds or seconds?

How about the following:
- we switch the chrome (URL bar, tab shape) immediately
- wait for up to 200ms like Safari
- if the tab is still not ready show the old content (non-interactive)

Comment 15

[Benoit Girard (:BenWa)]

We don't retain the layers for the tab when we hide it. So we would have to take a thumbnail as we switch tabs (and slow down the switch to the other tab). Also right now users can register a requestAnimationFrame that allow them to tweak the frame before it goes to the screen including in situation like tab switched. By following chrome' behavior we break that.

Comment 16

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to Benoit Girard (:BenWa) from comment #15)
> We don't retain the layers for the tab when we hide it. So we would have to
> take a thumbnail as we switch tabs (and slow down the switch to the other
> tab). Also right now users can register a requestAnimationFrame that allow
> them to tweak the frame before it goes to the screen including in situation
> like tab switched. By following chrome' behavior we break that.

Alright, thanks for the clarification!
In this case, let's use the same behavior as IE11.
- Switch chrome immediately
- Keep content of previous tab in the viewport until the switched-to tab is ready

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Benoit Girard (:BenWa) from comment #15)
> We don't retain the layers for the tab when we hide it. So we would have to
> take a thumbnail as we switch tabs (and slow down the switch to the other
> tab). Also right now users can register a requestAnimationFrame that allow
> them to tweak the frame before it goes to the screen including in situation
> like tab switched. By following chrome' behavior we break that.

Can we jump-start the thumbnail collecting process by doing it on mouseover of the tabstrip and keydown of a bubbled-not-default-prevented ctrl+t keydown?

rAF support for tab-switching isn't enough to negate investigating how we can make the largest part of the UI (the content being shown) feel responsive.

Comment 21

[Benoit Girard (:BenWa)]

Maybe, but it's a complex problem. What if the user does a mouseover of each tab in a row? We don't want to start rendering all of the visible tabs in the background (since we can't cancel the work once we request it).

There's certain things we can explore but we should use this bug to focus on fixing the flash into something we control and we should deal with responsive feel in another bug since it's a much more complex issue (unless you believe the approaches are orthogonal and wont require rAF).

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Benoit Girard (:BenWa) from comment #21)
> Maybe, but it's a complex problem. What if the user does a mouseover of each
> tab in a row? We don't want to start rendering all of the visible tabs in
> the background (since we can't cancel the work once we request it).

This wouldn't be rendering all of the visible tabs, only kickstarting of capturing the screenshot of the currently selected tab.

Comment 23

[Mike Conley (:mconley) (:⚙️)]

I think with bug 1009628 landed, this should be fixed... I'm not seeing the black flashes on Windows. Are you still seeing them on Linux, billm?

Comment 24

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Looks good to me. Thanks guys!

Comment 25

[Mike Conley (:mconley) (:⚙️)]

Cool, fixed by bug 1009628. we're done here.