1649367 - Explicitly use InProcess version of IsRootContentDocument for frame visibility

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

There are two nsPresContext::GetToplevelContentDocumentPresContext calls in PresShell::AssumeAllFramesVisible and in PresShell::ScheduleApproximateFrameVisibilityUpdateNow, it won't work properly in OOP iframes.

Comment 1

[Timothy Nikkel (:tnikkel)]

I think this has come up before: bug 1554832. I think it might be working as intended right now, but I'll have to look in detail.

Comment 2

[Neha Kochar [:neha]]

Emilio, do you know if this is fixed now?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1649367 - Fix some usage of IsRootContentDocument in PresShell. r=tnikkel

This doesn't change behavior, and is consistent with the changes made in
bug 1554832 to this code.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Yes, I think this is fixed, but let's be explicit on which document we care about.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Changing the name of the bug, as the original issue has now been fixed.

Comment 6

[Botond Ballo [:botond]]

Re-nominating for Fission based on the discussion in bug 1699846 comment 6. (Please let me know if I've misunderstood.)

I'm not super familiar with this code, but it looks like the (only?) consumer of frame visbility info is related to decoding images, so I think the failure mode here is, perhaps, that in OOP iframes, our heuristics to decode images when they enter the iframe's displayport may not kick in, and images may only get decoded once they enter the viewport? (Timothy, you might be able to confirm this.)

Comment 7

[Timothy Nikkel (:tnikkel)]

I guess there is that failure mode, and we could also decide all images are visible and decode them all and use too much memory.

Comment 8

[Botond Ballo [:botond]]

Ah, right, in case of an iframe with a large viewport (where we can limit the displayport to be smaller than the viewport). Good point.

Comment 9

[Chris Peterson [:cpeterson]]

I'm not super familiar with this code, but it looks like the (only?) consumer of frame visbility info is related to decoding images, so I think the failure mode here is, perhaps, that in OOP iframes, our heuristics to decode images when they enter the iframe's displayport may not kick in, and images may only get decoded once they enter the viewport?

Emilio, is your patch from six months ago still relevant for the current understanding of the problem, that we're not pre-decoding images in OOP iframes before they enter the viewport?

Tracking for Fission M8 because this could affect perceived page load performance.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Yeah. I tweaked the patch as suggested by tim.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6618c16b50f8
Fix some usage of IsRootContentDocument in PresShell. r=tnikkel

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/6618c16b50f8