[webkit-reviews] review denied: [Bug 94472] [CSSRegions]Add support for text-shadow in region styling : [Attachment 163610] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Andrei Onea <onea at adobe.com>'s request
for review:
Bug 94472: [CSSRegions]Add support for text-shadow in region styling
https://bugs.webkit.org/show_bug.cgi?id=94472

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

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


> Source/WebCore/rendering/InlineFlowBox.cpp:163
> -	       RenderStyle* childStyle =
child->renderer()->style(isFirstLineStyle());
> +	       RefPtr<RenderStyle> childStyle =
child->styleInRegion(child->regionDuringLayout());

Let's follow this through to its logical conclusion, which is that all style
access during layout that needs to get a style that can vary from region to
region is going to have to be put in a refptr just to keep it from going away?
Ick. If someone repeatedly asks for style, they're going to cause it to be
computed again and again.

It seems obvious that we have to change the architecture here for what to do
with region styling during layout. My opinion is that the styles should be
cached and updated once they are accessed once and they should stick around as
long as the region lives (and as long as the renderer that the style is
applying to lives). If you had a cache like that it could work for both layout
and painting.

Computing the styles and then tossing them for painting is not exactly a great
approach either, since you do way more work when painting than you need to.

I think ultimately there's going to need to be a method like styleAtOffset that
layout / painting use to fetch the appropriate region-specific style for an
object. If the style doesn't exist, then it could be created at that time.

I have no problem with a first-cut cache destroying all render styles at the
end of every layout (similar to what painting does today). If you want to take
that approach initially of doing something kind of like what painting does, I'd
be cool with that.