Bug 119428 – Removing an empty style sheet shouldn't trigger style recalc.

• Component: CSS
• Status: RESOLVED FIXED

[Andreas Kling]

Removing an empty style sheet shouldn't trigger style recalc.

[Radar WebKit Bug Importer]

<rdar://problem/14629045>

[Andreas Kling]

Created attachment 207980 [details]
Proposed patch

[Ryosuke Niwa]

Comment on attachment 207980 [details]
Proposed patch

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

> Source/WebCore/css/CSSStyleSheet.cpp:172
> +    owner->styleResolverChanged(RecalcStyleIfNeeded);

Maybe we want to add DeferRecalcStyleIfNeeded so that we don't synchronously do a style recalc when a non-empty style element is modified.

> Source/WebCore/dom/DocumentStyleSheetCollection.cpp:430
> -static void filterEnabledCSSStyleSheets(Vector<RefPtr<CSSStyleSheet> >& result, const Vector<RefPtr<StyleSheet> >& sheets)
> +static void filterEnabledNonemptyCSSStyleSheets(Vector<RefPtr<CSSStyleSheet> >& result, const Vector<RefPtr<StyleSheet> >& sheets)

Can we rename this to collect* or get*?

[Andreas Kling]

Created attachment 207982 [details]
Proposed patch v2

Add a DeferRecalcStyleIfNeeded mode as suggested by rniwa.

[Antti Koivisto]

Comment on attachment 207982 [details]
Proposed patch v2

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

> Source/WebCore/css/CSSStyleSheet.h:77
> -    virtual void clearOwnerNode() OVERRIDE { didMutate(); m_ownerNode = 0; }
> +    virtual void clearOwnerNode() OVERRIDE;

didMutate() (and so this performance regression) was added here: http://trac.webkit.org/changeset/144713

> Source/WebCore/dom/Document.h:200
> -enum StyleResolverUpdateFlag { RecalcStyleImmediately, DeferRecalcStyle, RecalcStyleIfNeeded };
> +enum StyleResolverUpdateFlag { RecalcStyleImmediately, DeferRecalcStyle, RecalcStyleIfNeeded, DeferRecalcStyleIfNeeded };

It is unfortunate that this gets even more complicated. It is hard to reason from these.

[Andreas Kling]

Landed: http://trac.webkit.org/changeset/153641
Corrected bugzilla link: http://trac.webkit.org/changeset/153642 (Duh)