[webkit-reviews] review denied: [Bug 111029] [css exclusions] Dynamically removing shape-inside should cause relayout of child blocks' inline content : [Attachment 194661] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Bear Travis <betravis at adobe.com>'s
request for review:
Bug 111029: [css exclusions] Dynamically removing shape-inside should cause
relayout of child blocks' inline content
https://bugs.webkit.org/show_bug.cgi?id=111029

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

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


Patch looks fine. I'd just prefer enums instead of boolean arguments, since it
makes the call sites clearer.

> Source/WebCore/rendering/RenderBlock.cpp:1421
> +	   info = block->layoutExclusionShapeInsideInfo(true);

Use an enum here. Nobody is going to know what "true" means.

> Source/WebCore/rendering/RenderBlock.h:455
> +    ExclusionShapeInsideInfo* exclusionShapeInsideInfo(bool includeRemoved =
false) const

enum.

> Source/WebCore/rendering/RenderBlock.h:467
> +    ExclusionShapeInsideInfo* layoutExclusionShapeInsideInfo(bool
includeRemoved = false) const;

enum.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:80
> +ExclusionShapeInsideInfo* RenderBlock::layoutExclusionShapeInsideInfo(bool
includeRemoved) const

enum.