1891335 - [fission] Twitter embeds load slowly again (fission)

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Rukko]

Since I can't reopen this https://bugzilla.mozilla.org/show_bug.cgi?id=1745869 I'm cloning it, because it still hapens in last version (124.0.2) ->

Steps to reproduce:

1. Make sure fission is enabled (fission.autostart = true).
2. Open up the attached website (contains 10 Twitter embeds) in https://bugzilla.mozilla.org/attachment.cgi?id=9255177
3. Notice how slowly everything loads.
4. Disable fission (fission.autostart = false) and restart the browser.
5. Open the website again.
6. Notice how fast everything loads.

Actual results:

Each embed loads extremely slowly. On pages with even more Twitter embeds, it can take minutes before everything loads.

Expected results:

Twitter embeds should have loaded quickly.

Please, feel free to close this and reopen the original thread.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Hmm, so we removed that 1s grace period because I made throttling consistent on fission and non-fission in bug 1847929 and bug 1847584.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Hmm... The fission iframes there should be 0 width and 0 height, and I'm pretty sure I tested that page when working on bug 1847929... Will look at it further tho.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1891335 - Compute EffectInfo updates at IntersectionObserver time. r=smaug,tnikkel

What goes on here is that there's a couple of unfortunate style change
sequences which end up making us not do the EffectsInfo dance correctly.

Twitter uses (maybe didn't use to, which would explain the regression) a
visibility: hidden, out-of-flow iframe for a bit (which we correctly
throttle). But then they switch to an in-flow, visible, zero-height
iframe. That we should not throttle. However, we end up not getting to
the display list code at all, because nsBlockFrame decides that we don't
need to descend into an empty line1.

It seems less error prone to re-use the IntersectionObserver timing and
computation to determine whether the iframe is visible. That completely
matches in-process iframes, too.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Ok, added a test and I addressed Tim's comments: https://treeherder.mozilla.org/jobs?repo=try&revision=374878d46981c1bfaa960fabd7124bcb58013b14

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Any more feedback in the patch? Otherwise I want to get it landed sooner rather than later, then make progress on https://github.com/whatwg/html/issues/10333 and co.

Comment 6

[Timothy Nikkel (:tnikkel)]

Sorry, I will take a look later today.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80db8b7ff2af
Compute EffectInfo updates at IntersectionObserver time. r=smaug,hiro

Comment 8

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in test_bug1639328.html.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | dom/base/test/test_bug1639328.html | Test timed out. -

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f96e847e742
Compute EffectInfo updates at IntersectionObserver time. r=smaug,hiro

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/5f96e847e742

Comment 11

[Ina Popescu, Desktop QA]

I've replicated this issue using Nightly 126.0a1 (2024-04-13) on macOS 13 following the STR from Comment 0.
Verified as fixed in the latest Firefox 132.0b8 version on Windows 10 x64, macOS 13, and Ubuntu 22.04, as the issue no longer occurs.