Bug 62528 – Rename Document::setContainsFullScreenElementRecursively

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jer Noble]

Document::setContainsFullScreenElementRecursively is named poorly, as it does not actually use recursion.  Also, make explicit that the function walks up the ancestor chain and crosses frame boundaries.

(See Darin's comments in https://bugs.webkit.org/show_bug.cgi?id=62507#c5.)

[Jer Noble]

Created attachment 96933 [details]
Patch

[Jer Noble]

In the end, I made setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries() a member function in Element (alongside setContainsFullScreenElement()), so it could be used so:

m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true).

[Darin Adler]

Comment on attachment 96933 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96933&action=review

> Source/WebCore/dom/Element.cpp:996
> -    if (parentElement() && containsFullScreenElement() && !parentElement()->containsFullScreenElement())
> -        document()->setContainsFullScreenElementRecursively(parentElement(), true);
> +    if (containsFullScreenElement() && parentElement() && !parentElement()->containsFullScreenElement())
> +        setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true);

I don’t think the additional checks in this if statement are needed or helpful. I see no compelling reason to optimize for this case, and it seems it could cause problems if the element being inserted is the top level element; we’ll miss out on the code that follows the ownerElement pointer. But it may be that the top level element is never inserted into a tree at a time where this matters.

> Source/WebCore/dom/Element.cpp:1931
> +void Element::setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(bool flag)

I probably would not make this a member function, just because I don’t want to have to modify Element.h every time I change it. But I think it’s OK as one.

[WebKit Review Bot]

Comment on attachment 96933 [details]
Patch

Clearing flags on attachment: 96933

Committed r89205: <http://trac.webkit.org/changeset/89205>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.