1367217 - ClearAllUndisplayedContentIn and ClearAllUndisplayedContentIn duplicate work

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

These two functions are always called together, like so:

        ClearAllDisplayContentsIn(node->mContent);
        ClearAllUndisplayedContentIn(node->mContent);

or

    ClearAllUndisplayedContentIn(content);
    ClearAllDisplayContentsIn(content);

or

      ClearAllDisplayContentsIn(aContent);
      ClearAllUndisplayedContentIn(aContent);

Then ClearAllUndisplayedContentIn does:


  FlattenedChildIterator iter(aParentContent);
  for (nsIContent* child = iter.GetNextChild(); child; child = iter.GetNextChild()) {
    if (child->GetParent() != aParentContent) {
      ClearUndisplayedContentIn(child, child->GetParent());
    }
  }

while ClearAllDisplayContentsIn does:

  FlattenedChildIterator iter(aParentContent);
  for (nsIContent* child = iter.GetNextChild(); child; child = iter.GetNextChild()) {
    if (child->GetParent() != aParentContent) {
      ClearDisplayContentsIn(child, child->GetParent());
      ClearUndisplayedContentIn(child, child->GetParent());
    }
  }

That second walk over the kids repeats all the work from the first walk and adds a bit more.  Could we just do a single walk over the kids that does the work from ClearAllDisplayContentsIn?

Comment 1

[Mats Palmgren (:mats)]

Yeah, I think we should just merge these two methods.

Re: the loop at the end - we could skip that altogether if aParentContent
had a HaveXBLOrShadowDOMChildren flag or a cheap way to test that, does it?

Comment 2

[Mats Palmgren (:mats)]

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

Comment 3

[Mats Palmgren (:mats)]

Attachment: Merge ClearAllUndisplayedContentIn / ClearAllDisplayContentsIn to avoid doing duplicated work

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8875377 [details] [diff] [review]
Merge ClearAllUndisplayedContentIn / ClearAllDisplayContentsIn to avoid doing duplicated work

Review of attachment 8875377 [details] [diff] [review]:
-----------------------------------------------------------------

r=me (looks like this preserves existing behavior, aside from the duplicate-work elimination)

Thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee20b7f985f5
Merge ClearAllUndisplayedContentIn / ClearAllDisplayContentsIn to avoid doing duplicated work.  r=dholbert

Comment 6

[Boris Zbarsky [:bzbarsky]]

> we could skip that altogether if aParentContent had a HaveXBLOrShadowDOMChildren
> flag or a cheap way to test that, does it?

I don't know of one offhand.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ee20b7f985f5