[webkit-reviews] review granted: [Bug 216466] [LFC Display] First steps towards LFC painting in WebKit2 : [Attachment 408666] Patch

[bugzilla-daemon at webkit.org]

zalan <zalan at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 216466: [LFC Display] First steps towards LFC painting in WebKit2
https://bugs.webkit.org/show_bug.cgi?id=216466

Attachment 408666: Patch

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




--- Comment #3 from zalan <zalan at apple.com> ---
Comment on attachment 408666
  --> https://bugs.webkit.org/attachment.cgi?id=408666
Patch

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

> Source/WebCore/layout/display/DisplayLayerController.cpp:83
> +    if (!layoutState.hasDisplayBox(rootLayoutBox))
> +	   return;

Shouldn't this be an ASSERT instead? We should never try to paint unless we
have computed geometry (completed a layout).

> Source/WebCore/layout/display/DisplayView.cpp:89
> +

extra line

> Source/WebCore/layout/display/DisplayView.h:65
> +    // FIXME: Do we need to use LayoutTreeContent?

I really hope not.
I think we should figure out a way to have no dependency on LayoutState either.
Display::Boxes are part of the state as they provide geometry information to
subsequent layouts. However it seems a bit odd to see LayoutState in the
context of painting and hittesting (this is not new to this patch)