RESOLVED FIXED119428
Removing an empty style sheet shouldn't trigger style recalc. 
https://bugs.webkit.org/show_bug.cgi?id=119428
Summary Removing an empty style sheet shouldn't trigger style recalc.
Andreas Kling
Reported 2013-08-01 21:44:40 PDT
Removing an empty style sheet shouldn't trigger style recalc.
Attachments
Proposed patch (4.81 KB, patch)
2013-08-01 22:12 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Proposed patch v2 (6.83 KB, patch)
2013-08-01 22:55 PDT, Andreas Kling 		koivisto: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2013-08-01 21:44:51 PDT
<rdar://problem/14629045>
Andreas Kling
Comment 2 2013-08-01 22:12:47 PDT
Created attachment 207980 [details] Proposed patch
Ryosuke Niwa
Comment 3 2013-08-01 22:26:21 PDT
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
Comment 4 2013-08-01 22:55:33 PDT
Created attachment 207982 [details] Proposed patch v2 Add a DeferRecalcStyleIfNeeded mode as suggested by rniwa.
Antti Koivisto
Comment 5 2013-08-02 02:06:17 PDT
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
Comment 6 2013-08-02 02:22:17 PDT
Landed: http://trac.webkit.org/changeset/153641 Corrected bugzilla link: http://trac.webkit.org/changeset/153642 (Duh)
Note You need to log in before you can comment on or make changes to this bug.