[webkit-reviews] review granted: [Bug 234018] nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded : [Attachment 447254] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Gabriel Nava Marino
<gnavamarino at apple.com>'s request for review:
Bug 234018: nullptr deref in
ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffset
IfNeeded
https://bugs.webkit.org/show_bug.cgi?id=234018

Attachment 447254: Patch

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




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

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

This is great. Fine to land as is.

To refine further, we can take one more step by adding
inclusiveDescendantsOfType to RenderDescendantIterator.h, which we already have
in TypedElementDecendantIterator.h. If we do that, we can remove the check at
the top of the function, and have only the loop.

> Source/WebCore/rendering/RenderBlockFlow.cpp:2122
>	   if (!is<RenderBlockFlow>(block))
>	       continue;
>	   auto& blockFlow = downcast<RenderBlockFlow>(block);
> -	   contains |= blockFlow.subtreeContainsFloat(renderer);
> +	   if (blockFlow.containsFloat(renderer))
> +	       return true;

To refine further we *could* collapse these 5 lines into two:

    if (is<RenderBlockFlow>(block) &&
downcast<RenderBlockFlow>(block).containsFloat(renderer))
	return true;