[webkit-reviews] review granted: [Bug 123396] Renderers should receive their style at construction. : [Attachment 215276] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Andreas Kling <akling at apple.com>'s
request for review:
Bug 123396: Renderers should receive their style at construction.
https://bugs.webkit.org/show_bug.cgi?id=123396

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=215276&action=review


> Source/WebCore/loader/icon/IconDatabase.cpp:266
> -    ASSERT(iconRecord || m_retainedPageURLs.contains(pageURLOriginal));
> +//	 ASSERT(iconRecord || m_retainedPageURLs.contains(pageURLOriginal));

Probably don't want to land this

> Source/WebCore/rendering/FlowThreadController.cpp:82
> +    auto flowRenderer = new RenderNamedFlowThread(
> +	   m_view->document(),
> +	   RenderFlowThread::createFlowThreadStyle(m_view->style()),
> +	   namedFlows->ensureFlowWithName(name)
> +    );

New fashionable way of calling functions?

> Source/WebCore/rendering/RenderElement.cpp:355
> +void RenderElement::initializeStyle()
> +{
> +    styleWillChange(StyleDifferenceEqual, *style());
> +
> +    m_hasInitializedStyle = true;

Would be nice to be able to do all style initialization during construction and
get rid of this function.

> Source/WebCore/rendering/RenderElement.h:168
> +    bool m_hasInitializedStyle : 1;

Bit of a hack but ok.

> Source/WebCore/style/StyleResolveTree.cpp:259
> +    // FIXME: There's probably a better way to factor this.
> +    // This just does what setAnimatedStyle() does, except with
setStyleInternal() instead of setStyle().
> +   
newRenderer->setStyleInternal(newRenderer->animation().updateAnimations(*newRen
derer, *newRenderer->style()));
> +
> +    newRenderer->initializeStyle();

Might be nicer to at least factor these to something like
initializeAnimatableStyle()