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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 13 15:53:46 PST 2022

Darin Adler <darin at apple.com> has granted Gabriel Nava Marino
<gnavamarino at apple.com>'s request for review:
Bug 234018: nullptr deref in

Attachment 449117: Patch


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

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

> Source/WebCore/ChangeLog:9
> +	   Implement InclusiveRenderDescendantIteratorAdapter and use
inclusiveDescendantsOfType in
> +	   RenderBlockFlow::subtreeContainsFloat and

I suspect there is a way to implement this sharing more code with the
non-inclusive case, since non-inclusive is just like inclusive, but just
skipping the thing itself, but it’s more important how we *use*
inclusiveDescendantsOfType than how smooth the implementation is. That can
always be refined later.

I would suggest, though, that we address the const-ness issue eventually. Maybe
even at the same time.

Ideally the descendantsOfType on a const reference would iterate const
references, and non-const would iterate non-const. Ideally without duplicating
all the code twice, so maybe using some const_cast in the implementation.

> Source/WebCore/rendering/RenderBlockFlow.cpp:1957
> +    for (auto& block :
inclusiveDescendantsOfType<RenderBlock>(const_cast<RenderBlockFlow&>(*this))) {
> +	   if (is<RenderBlockFlow>(block) &&
>	       return true;
>      }

Hmm, can we write this?

    for (auto& block : inclusiveDescendantsOfType<RenderBlockFlow>(*this)) {
	if (block.containsFloat(renderer))
	    return true;

That’s the whole point of having the xxxOfType feature (and assumes the const).

More information about the webkit-reviews mailing list