[webkit-reviews] review granted: [Bug 235272] Clean up some code around RenderElement::addLayers() : [Attachment 449267] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 235272: Clean up some code around RenderElement::addLayers()
https://bugs.webkit.org/show_bug.cgi?id=235272

Attachment 449267: Patch

https://bugs.webkit.org/attachment.cgi?id=449267&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 449267
  --> https://bugs.webkit.org/attachment.cgi?id=449267
Patch

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

> Source/WebCore/rendering/RenderElement.cpp:640
> +	   if (!beforeChild.has_value())

You can omit the ".has_value()" if you like. But if you think including it
makes the code clearer, that’s fine.

> Source/WebCore/rendering/RenderElement.cpp:643
> +	  
parentLayer.addChild(*downcast<RenderLayerModelObject>(currentRenderer).layer()
, beforeChild.value());

I often prefer *beforeChild, treating an optional as if it was a pointer, but
if you like value() better, I’m OK with it.

> Source/WebCore/rendering/RenderElement.cpp:699
> +	   for (auto* child = siblingToTraverseFrom ?
siblingToTraverseFrom->nextSibling() : firstChild(); child; child =
child->nextSibling()) {

Seems like childAfter helper function, either member or non-member, that
returns first if the argument is nullptr and nextSibling if it’s not would make
this nicer to read.