[webkit-reviews] review granted: [Bug 225102] Share style resolvers between author shadow trees without style sheets : [Attachment 427338] patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 225102: Share style resolvers between author shadow trees without style
sheets
https://bugs.webkit.org/show_bug.cgi?id=225102

Attachment 427338: patch

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




--- Comment #5 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 427338
  --> https://bugs.webkit.org/attachment.cgi?id=427338
patch

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

> Source/WebCore/ChangeLog:10
> +	   Prepare for sharing style resolvers (and all the related data)
between shadow trees beyond
> +	   the currently supported user agent shadow trees. This patch
refactors the sharing code
> +	   and adds support for sharing resolvers for author shadow trees
without style sheets.

How (if at all) do we test resolver sharing? Could we add some sort of dumping
of the resolver state if we don't have a better way?

> Source/WebCore/style/StyleResolver.h:144
> +    bool wasShared() const { return m_wasShared; }
> +    void setWasShared() { m_wasShared = true; }

If this is really meant to be only for shared between shadow trees (as I think
the change log indicates), I think making the name a bit more verbose and/or
adding a comment would be good.

> Source/WebCore/style/StyleScope.cpp:537
> +    Vector<RefPtr<CSSStyleSheet>> newStyleSheets;

You could make this slight more efficient using a
newStyleSheets.reserveInitialCapacity() here.

> Source/WebCore/style/StyleScope.cpp:539
> +    m_resolver->appendAuthorStyleSheets(newStyleSheets);

Seems like unfortunate refchurn. Can we move  newStyleSheets into
appendAuthorStyleSheets (may require making a appendAuthorStyleSheets overload
that takes an r-value).