[webkit-reviews] review granted: [Bug 84796] Implement the new stacking layer needed by the Fullscreen API and the new <dialog> element : [Attachment 169862] sync

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Matt Falkenhagen
<falken at chromium.org>'s request for review:
Bug 84796: Implement the new stacking layer needed by the Fullscreen API and
the new <dialog> element
https://bugs.webkit.org/show_bug.cgi?id=84796

Attachment 169862: sync
https://bugs.webkit.org/attachment.cgi?id=169862&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=169862&action=review


> Source/WebCore/dom/Element.cpp:975
> +    setIsInTopLayer(false);

I wonder if you should check isInTopLayer() to avoid a perf hit.

> Source/WebCore/dom/NodeRenderingContext.cpp:272
> +#if ENABLE(DIALOG_ELEMENT)
> +    if (element && element->isInTopLayer()) {
> +	   parentRenderer = parentRenderer->view();
> +	   nextRenderer = 0;
> +	   Vector<RefPtr<Element> >& topLayerElements =
document->topLayerElements();
> +	   size_t topLayerPosition = topLayerElements.find(element);
> +	   ASSERT(topLayerPosition != notFound);
> +	   for (RenderObject* child = parentRenderer->firstChild(); child;
child = child->nextSibling()) {
> +	       Element* childElement = child->node()->isElementNode() ?
toElement(child->node()) : 0;
> +	       if (childElement && childElement->isInTopLayer()) {
> +		   size_t childTopLayerPosition =
topLayerElements.find(childElement);
> +		   ASSERT(childTopLayerPosition != notFound);
> +		   if (topLayerPosition < childTopLayerPosition) {
> +		       nextRenderer = child;
> +		       break;
> +		   }
> +	       }
> +	   }
> +    }
> +#endif

I think it's worth moving this into its own function.