[webkit-reviews] review denied: [Bug 101332] [CSS Regions] Add Region info for RootLineBoxes and pack the pagination data : [Attachment 174417] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Andrei Bucur <abucur at adobe.com>'s
request for review:
Bug 101332: [CSS Regions] Add Region info for RootLineBoxes and pack the
pagination data
https://bugs.webkit.org/show_bug.cgi?id=101332

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

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=174417&action=review


r-

> Source/WebCore/rendering/RenderBlock.cpp:7254
> +    // Just bail if we still don't have a region.
> +    if (!rootBox->hasContainingRegion() && !currentRegion)
> +	   return false;
> +    // Just bail if the region didn't change.
> +    if (rootBox->hasContainingRegion() && rootBox->containingRegion() ==
currentRegion)
> +	   return false;

Isn't all this just equivalent to:

if (rootBox->containingRegion() == currentRegion)
    return false;

?

I'm not really sure what all the extra code is buying you. If both regions are
null, the == check will cause you to bail anyway, and you won't ever go from
null to non-null or vice versa.