Bug 225102

Summary: Share style resolvers between author shadow trees without style sheets
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, esprehn+autocc, ews-watchlist, kangil.han, rniwa, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 225187    
Attachments:
Description Flags
wip
none
patch
ews-feeder: commit-queue-
patch
ews-feeder: commit-queue-
patch
sam: review+
patch
none
patch
none
patch
none
patch none

Description Antti Koivisto 2021-04-27 06:38:23 PDT
Just for UA shadow tree resolver for now.
Comment 1 Antti Koivisto 2021-04-27 06:39:33 PDT
Created attachment 427145 [details]
wip
Comment 2 Antti Koivisto 2021-04-28 10:17:44 PDT
Created attachment 427276 [details]
patch
Comment 3 Antti Koivisto 2021-04-29 03:19:24 PDT
Created attachment 427336 [details]
patch
Comment 4 Antti Koivisto 2021-04-29 05:11:10 PDT
Created attachment 427338 [details]
patch
Comment 5 Sam Weinig 2021-04-29 09:36:06 PDT
Comment on attachment 427338 [details]
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).
Comment 6 Antti Koivisto 2021-04-29 10:02:23 PDT
> 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?

Good point, I'll add some sort of testing thing to Internals.
Comment 7 Antti Koivisto 2021-04-30 03:40:11 PDT
Created attachment 427404 [details]
patch
Comment 8 Antti Koivisto 2021-04-30 03:58:04 PDT
Created attachment 427405 [details]
patch
Comment 9 Antti Koivisto 2021-04-30 03:59:39 PDT
Created attachment 427406 [details]
patch
Comment 10 Antti Koivisto 2021-04-30 04:04:20 PDT
Created attachment 427408 [details]
patch
Comment 11 EWS 2021-04-30 05:19:37 PDT
Committed r276836 (237187@main): <https://commits.webkit.org/237187@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427408 [details].
Comment 12 Radar WebKit Bug Importer 2021-04-30 05:20:21 PDT
<rdar://problem/77377222>