Bug 119568

Summary: Inserting multiple rules into an empty style sheet should avoid style recalc if possible.
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, kangil.han, kling, koivisto, macpherson, menard, webkit-bug-importer
Priority: P2 Keywords: InRadar, Performance
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Short patch name
none
More hardcore version
none
Totally rad patch koivisto: review+

Andreas Kling
Reported 2013-08-08 02:43:07 PDT
Spin-off from bug 119475. Let's take this to the next level.
Attachments
Short patch name (13.71 KB, patch)
2013-08-08 03:00 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
More hardcore version (18.89 KB, patch)
2013-08-08 07:15 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Totally rad patch (18.99 KB, patch)
2013-08-08 07:45 PDT, Andreas Kling 		koivisto: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Andreas Kling
Comment 1 2013-08-08 03:00:09 PDT
Created attachment 208321 [details] Short patch name
Antti Koivisto
Comment 2 2013-08-08 04:18:33 PDT
Comment on attachment 208321 [details] Short patch name View in context: https://bugs.webkit.org/attachment.cgi?id=208321&action=review > Source/WebCore/css/CSSStyleSheet.cpp:289 > - RuleMutationScope mutationScope(this, mutationType); > + RuleMutationScope mutationScope(this, RuleInsertion); The mutation can trigger copy-on-write. In that case we might be left with dangling pointers in StyleResolver.
Andreas Kling
Comment 3 2013-08-08 07:15:12 PDT
Created attachment 208341 [details] More hardcore version
Andreas Kling
Comment 4 2013-08-08 07:45:46 PDT
Created attachment 208342 [details] Totally rad patch
Antti Koivisto
Comment 5 2013-08-08 08:42:59 PDT
Comment on attachment 208342 [details] Totally rad patch View in context: https://bugs.webkit.org/attachment.cgi?id=208342&action=review > Source/WebCore/css/CSSStyleSheet.h:89 > + enum WhetherContentsWereClonedForMutation { ContentsWereNotClonedForMutation = 0, ContentsWereClonedForMutation }; poetic! > Source/WebCore/dom/DocumentStyleSheetCollection.h:74 > + enum UpdateFlag { NoUpdate = 0, OptimizedUpdate, FullUpdate }; Does the = 0 do something here? > Source/WebCore/dom/DocumentStyleSheetCollection.h:137 > + // This is a mirror of m_activeAuthorStyleSheets that gets populated on demand for activeStyleSheetsContains(). > + mutable OwnPtr<HashSet<const CSSStyleSheet*>> m_weakCopyOfActiveStyleSheetListForFastLookup; Bit clunky but I don't have great suggestions.
Antti Koivisto
Comment 6 2013-08-08 08:44:04 PDT
It would also be good to check we have test coverage for insertion that causes cloning.
Andreas Kling
Comment 7 2013-08-08 10:40:14 PDT
Committed r153829: <http://trac.webkit.org/changeset/153829>
Radar WebKit Bug Importer
Comment 8 2013-08-08 11:26:25 PDT
<rdar://problem/14687740>
Note You need to log in before you can comment on or make changes to this bug.