Bug 119568 - Inserting multiple rules into an empty style sheet should avoid style recalc if possible.
Summary: Inserting multiple rules into an empty style sheet should avoid style recalc ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: InRadar, Performance
Depends on:
Blocks:
 
Reported: 2013-08-08 02:43 PDT by Andreas Kling
Modified: 2013-08-08 11:26 PDT (History)
9 users (show)

See Also:

Attachments
Short patch name (13.71 KB, patch)
2013-08-08 03:00 PDT, Andreas Kling 		no flags Details | Formatted Diff | Diff
More hardcore version (18.89 KB, patch)
2013-08-08 07:15 PDT, Andreas Kling 		no flags Details | Formatted Diff | Diff
Totally rad patch (18.99 KB, patch)
2013-08-08 07:45 PDT, Andreas Kling 		koivisto: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2013-08-08 02:43:07 PDT 
Spin-off from bug 119475. Let's take this to the next level.
Comment 1 Andreas Kling 2013-08-08 03:00:09 PDT 
Created attachment 208321 [details]
Short patch name
Comment 2 Antti Koivisto 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.
Comment 3 Andreas Kling 2013-08-08 07:15:12 PDT 
Created attachment 208341 [details]
More hardcore version
Comment 4 Andreas Kling 2013-08-08 07:45:46 PDT 
Created attachment 208342 [details]
Totally rad patch
Comment 5 Antti Koivisto 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.
Comment 6 Antti Koivisto 2013-08-08 08:44:04 PDT 
It would also be good to check we have test coverage for insertion that causes cloning.
Comment 7 Andreas Kling 2013-08-08 10:40:14 PDT 
Committed r153829: <http://trac.webkit.org/changeset/153829>
Comment 8 Radar WebKit Bug Importer 2013-08-08 11:26:25 PDT 
<rdar://problem/14687740>