1223753 - Implement basic visibility tracking for -moz-element

Status: NEW

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Seth Fowler [:seth] [:s2h]]

Right now frame visibility doesn't take -moz-element into account, but it's not that hard. We can do so by having nsSVGIDRenderingObserver mark the subtree of the primary frame of the element it's observer visible. (Until it stops observing that element, in which case it can decrement the visible counts of the frames in the subtree.)

Right now the frame walking is extremely simple and doesn't cross into subdocuments or try to do anything clever about scrollframes. We can extend this further in followups; I just want to get the basic, common cases working first.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Implement basic visibility tracking for -moz-element.

Here's the patch.

Comment 2

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 1) - Make it possible to mark the frame subtree of certain content nodes visible.

OK, this needed to be reworked, because the frames associated with a content
node are not necessarily available at the time that nsSVGIDRenderingObserver
starts observing it. To make this work, I've instead added a list of content
nodes to the pres shell. If a content node is in the list, every frame in its
subtree is considered visible. We can then rely on the normal frame visibility
update mechanism to the keep the frame visibility list correct.

What's really nice is that this eliminates the limitations of the previous
approach, because we're reusing the existing frame walker. For example, we now
traverse subdocuments correctly.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 2) - Implement visibility tracking for -moz-element.

This patch actually implements visibility tracking that "sees through"
nsSVGIDRenderingObserver, and therefore through -moz-element. The idea is the
same as in the previous version of the patch, but now we make use of
Increment/DecrementVisibleCountOfContentSubtree().

Comment 4

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=68d2743b0670

Comment 5

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c41fcc4b60b

Comment 6

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b29b8698351

Comment 7

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 1) - Make it possible to mark the frame subtree of certain content nodes visible.

OK, so investigation of the failing test above made me realize that there is a
simpler and better way to implement this. Rather than forcing all frames in a
subtree to be visible, just call MarkFramesInSubtreeVisible() and pass the
visual overflow rect of the primary frame of the content we care about. The old
version of the patch was buggy, and this should fix that, but even better, it
means that for e.g. scrollframes we will only mark visible the frames that are
actually visible through the scrollframe, and not every single frame that may be
scrolled way out of view.

It also makes the patch even shorter and simpler, which is nice. =)

Comment 8

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=46ef34948608

Comment 9

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f94578f09a11

Comment 10

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=523dd9a49cec

Comment 11

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ecae8eb2e34

Comment 12

[Seth Fowler [:seth] [:s2h]]

Alright, moving the list of content nodes that we're forcing to be visible to the document turned out to be the trick. The pres shell doesn't have the right lifetime; the same document can have many pres shells over time.

The try job above is green. I'll upload the final version of the patches now.

Comment 13

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 1) - Make it possible to mark the frame subtree of certain content nodes visible.

This updated version of part one, as implied by the comment above, has one major
change: the list of content nodes that we're forcing to be considered visible
has moved to nsIDocument/nsDocument. That gives the list the same lifetime as
the content nodes themselves (unless the content is removed from the document,
of course, but we handle that in part 2 in the ElementChanged() callback). With
this change, these patches are now green on try.

I've made a couple of cosmetic changes as well, adding more comments and
renaming the methods to (hopefully) clarify what they do a bit more.

Comment 14

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 2) - Implement visibility tracking for -moz-element.

This version of part 2 is basically just rebased against the updated version of
part 1; the idea remains the same.

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.