Skip to content

GitLab

T
tangled
Commit 5eefd373 authored by rune's avatar rune Committed by Commit bot
Browse files
Options

Use hash set instead of vector for changed RuleSets. 

That way, we don't have to consider the same RuleSet multiple times for
invalidation on active stylesheet update. This fixes a regression in
PerformanceTests/CSS/StyleSheetInsert.html which would have been
introduced by https://codereview.chromium.org/2557533005

This works because the same style element source text used multiple
times will make us use the same StyleSheetContents from the cache and
hence the same RuleSet for all 50 sheets added in that test. It's a bit
like cheating, but this will also make sure we don't invalidate for the
same RuleSet twice if we re-order stylesheets by removing/inserting a
style element where the CSSStyleSheet pointer will be different, but
the RuleSet stays the same.

R=sashab@chromium.org,esprehn@chromium.org
BUG=567021

Review-Url: https://codereview.chromium.org/2569733003
Cr-Commit-Position: refs/heads/master@{#438062}
parent abcca45f
Hide whitespace changes
Inline Side-by-side
Showing with 52 additions and 49 deletions
third_party/WebKit/Source/core/css/ActiveStyleSheets.cpp
View file @ 5eefd373
......@@ -16,7 +16,7 @@ namespace blink {
ActiveSheetsChange compareActiveStyleSheets(
const ActiveStyleSheetVector& oldStyleSheets,
const ActiveStyleSheetVector& newStyleSheets,
HeapVector<Member<RuleSet>>& changedRuleSets) {
HeapHashSet<Member<RuleSet>>& changedRuleSets) {
unsigned newStyleSheetCount = newStyleSheets.size();
unsigned oldStyleSheetCount = oldStyleSheets.size();
......@@ -32,32 +32,34 @@ ActiveSheetsChange compareActiveStyleSheets(
continue;
if (newStyleSheets[index].second)
changedRuleSets.append(newStyleSheets[index].second);
changedRuleSets.add(newStyleSheets[index].second);
if (oldStyleSheets[index].second)
changedRuleSets.append(oldStyleSheets[index].second);
changedRuleSets.add(oldStyleSheets[index].second);
}
if (index == oldStyleSheetCount) {
if (index == newStyleSheetCount)
return changedRuleSets.size() ? ActiveSheetsChanged
: NoActiveSheetsChanged;
if (index == newStyleSheetCount) {
return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
: ActiveSheetsChanged;
}
// Sheets added at the end.
for (; index < newStyleSheetCount; index++) {
if (newStyleSheets[index].second)
changedRuleSets.append(newStyleSheets[index].second);
changedRuleSets.add(newStyleSheets[index].second);
}
return changedRuleSets.size() ? ActiveSheetsAppended
: NoActiveSheetsChanged;
return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
: ActiveSheetsAppended;
}
if (index == newStyleSheetCount) {
// Sheets removed from the end.
for (; index < oldStyleSheetCount; index++) {
if (oldStyleSheets[index].second)
changedRuleSets.append(oldStyleSheets[index].second);
changedRuleSets.add(oldStyleSheets[index].second);
}
return changedRuleSets.size() ? ActiveSheetsChanged : NoActiveSheetsChanged;
return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
: ActiveSheetsChanged;
}
DCHECK(index < oldStyleSheetCount && index < newStyleSheetCount);
......@@ -83,7 +85,7 @@ ActiveSheetsChange compareActiveStyleSheets(
(*mergedIterator).first != sheet1.first) {
// Sheet either removed or inserted.
if (sheet1.second)
changedRuleSets.append(sheet1.second);
changedRuleSets.add(sheet1.second);
continue;
}
......@@ -96,11 +98,12 @@ ActiveSheetsChange compareActiveStyleSheets(
// Active rules for the given stylesheet changed.
// DOM, CSSOM, or media query changes.
if (sheet1.second)
changedRuleSets.append(sheet1.second);
changedRuleSets.add(sheet1.second);
if (sheet2.second)
changedRuleSets.append(sheet2.second);
changedRuleSets.add(sheet2.second);
}
return changedRuleSets.size() ? ActiveSheetsChanged : NoActiveSheetsChanged;
return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
: ActiveSheetsChanged;
}
} // namespace blink
third_party/WebKit/Source/core/css/ActiveStyleSheets.h
View file @ 5eefd373
......@@ -25,7 +25,7 @@ enum ActiveSheetsChange {
CORE_EXPORT ActiveSheetsChange
compareActiveStyleSheets(const ActiveStyleSheetVector& oldStyleSheets,
const ActiveStyleSheetVector& newStyleSheets,
HeapVector<Member<RuleSet>>& changedRuleSets);
HeapHashSet<Member<RuleSet>>& changedRuleSets);
} // namespace blink
......
third_party/WebKit/Source/core/css/ActiveStyleSheetsTest.cpp
View file @ 5eefd373
......@@ -57,7 +57,7 @@ ShadowRoot& ApplyRulesetsTest::attachShadow(Element& host) {
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_NoChange) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
EXPECT_EQ(NoActiveSheetsChanged,
compareActiveStyleSheets(oldSheets, newSheets, changedRuleSets));
......@@ -80,7 +80,7 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_NoChange) {
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_AppendedToEmpty) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -96,7 +96,7 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_AppendedToEmpty) {
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_AppendedToNonEmpty) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -113,7 +113,7 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_AppendedToNonEmpty) {
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_Mutated) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -135,15 +135,15 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_Mutated) {
EXPECT_EQ(ActiveSheetsChanged,
compareActiveStyleSheets(oldSheets, newSheets, changedRuleSets));
ASSERT_EQ(2u, changedRuleSets.size());
EXPECT_EQ(&sheet2->contents()->ruleSet(), changedRuleSets[0]);
EXPECT_EQ(oldSheets[1].second, changedRuleSets[1]);
EXPECT_EQ(2u, changedRuleSets.size());
EXPECT_TRUE(changedRuleSets.contains(&sheet2->contents()->ruleSet()));
EXPECT_TRUE(changedRuleSets.contains(oldSheets[1].second));
}
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_Inserted) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -158,14 +158,14 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_Inserted) {
EXPECT_EQ(ActiveSheetsChanged,
compareActiveStyleSheets(oldSheets, newSheets, changedRuleSets));
ASSERT_EQ(1u, changedRuleSets.size());
EXPECT_EQ(&sheet2->contents()->ruleSet(), changedRuleSets[0]);
EXPECT_EQ(1u, changedRuleSets.size());
EXPECT_TRUE(changedRuleSets.contains(&sheet2->contents()->ruleSet()));
}
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_Removed) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -180,14 +180,14 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_Removed) {
EXPECT_EQ(ActiveSheetsChanged,
compareActiveStyleSheets(oldSheets, newSheets, changedRuleSets));
ASSERT_EQ(1u, changedRuleSets.size());
EXPECT_EQ(&sheet2->contents()->ruleSet(), changedRuleSets[0]);
EXPECT_EQ(1u, changedRuleSets.size());
EXPECT_TRUE(changedRuleSets.contains(&sheet2->contents()->ruleSet()));
}
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_RemovedAll) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -205,7 +205,7 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_RemovedAll) {
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_InsertedAndRemoved) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -219,15 +219,15 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_InsertedAndRemoved) {
EXPECT_EQ(ActiveSheetsChanged,
compareActiveStyleSheets(oldSheets, newSheets, changedRuleSets));
ASSERT_EQ(2u, changedRuleSets.size());
EXPECT_EQ(&sheet1->contents()->ruleSet(), changedRuleSets[0]);
EXPECT_EQ(&sheet3->contents()->ruleSet(), changedRuleSets[1]);
EXPECT_EQ(2u, changedRuleSets.size());
EXPECT_TRUE(changedRuleSets.contains(&sheet1->contents()->ruleSet()));
EXPECT_TRUE(changedRuleSets.contains(&sheet3->contents()->ruleSet()));
}
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_AddNullRuleSet) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -245,7 +245,7 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_AddNullRuleSet) {
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_RemoveNullRuleSet) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -263,7 +263,7 @@ TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_RemoveNullRuleSet) {
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_AddRemoveNullRuleSet) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -284,7 +284,7 @@ TEST_F(ActiveStyleSheetsTest,
CompareActiveStyleSheets_RemoveNullRuleSetAndAppend) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......@@ -298,14 +298,14 @@ TEST_F(ActiveStyleSheetsTest,
EXPECT_EQ(ActiveSheetsChanged,
compareActiveStyleSheets(oldSheets, newSheets, changedRuleSets));
ASSERT_EQ(1u, changedRuleSets.size());
EXPECT_EQ(&sheet3->contents()->ruleSet(), changedRuleSets[0]);
EXPECT_EQ(1u, changedRuleSets.size());
EXPECT_TRUE(changedRuleSets.contains(&sheet3->contents()->ruleSet()));
}
TEST_F(ActiveStyleSheetsTest, CompareActiveStyleSheets_ReorderedImportSheets) {
ActiveStyleSheetVector oldSheets;
ActiveStyleSheetVector newSheets;
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
CSSStyleSheet* sheet1 = createSheet();
CSSStyleSheet* sheet2 = createSheet();
......
third_party/WebKit/Source/core/dom/StyleEngine.cpp
View file @ 5eefd373
......@@ -895,7 +895,7 @@ void StyleEngine::scheduleNthPseudoInvalidations(ContainerNode& nthParent) {
void StyleEngine::scheduleRuleSetInvalidationsForElement(
Element& element,
const HeapVector<Member<RuleSet>>& ruleSets) {
const HeapHashSet<Member<RuleSet>>& ruleSets) {
AtomicString id;
const SpaceSplitString* classNames = nullptr;
......@@ -938,7 +938,7 @@ void StyleEngine::invalidateSlottedElements(HTMLSlotElement& slot) {
void StyleEngine::scheduleInvalidationsForRuleSets(
TreeScope& treeScope,
const HeapVector<Member<RuleSet>>& ruleSets) {
const HeapHashSet<Member<RuleSet>>& ruleSets) {
#if DCHECK_IS_ON()
// Full scope recalcs should be handled while collecting the ruleSets before
// calling this method.
......@@ -1097,7 +1097,7 @@ enum RuleSetFlags {
FullRecalcRules = 1 << 2
};
unsigned getRuleSetFlags(const HeapVector<Member<RuleSet>> ruleSets) {
unsigned getRuleSetFlags(const HeapHashSet<Member<RuleSet>> ruleSets) {
unsigned flags = 0;
for (auto& ruleSet : ruleSets) {
ruleSet->compactRulesIfNeeded();
......@@ -1117,7 +1117,7 @@ void StyleEngine::applyRuleSetChanges(
TreeScope& treeScope,
const ActiveStyleSheetVector& oldStyleSheets,
const ActiveStyleSheetVector& newStyleSheets) {
HeapVector<Member<RuleSet>> changedRuleSets;
HeapHashSet<Member<RuleSet>> changedRuleSets;
ScopedStyleResolver* scopedResolver = treeScope.scopedStyleResolver();
bool appendAllSheets =
......
third_party/WebKit/Source/core/dom/StyleEngine.h
View file @ 5eefd373
......@@ -250,7 +250,7 @@ class CORE_EXPORT StyleEngine final
Element& afterElement);
void scheduleNthPseudoInvalidations(ContainerNode&);
void scheduleInvalidationsForRuleSets(TreeScope&,
const HeapVector<Member<RuleSet>>&);
const HeapHashSet<Member<RuleSet>>&);
unsigned styleForElementCount() const { return m_styleForElementCount; }
void incStyleForElementCount() { m_styleForElementCount++; }
......@@ -323,7 +323,7 @@ class CORE_EXPORT StyleEngine final
bool shouldSkipInvalidationFor(const Element&) const;
void scheduleRuleSetInvalidationsForElement(
Element&,
const HeapVector<Member<RuleSet>>&);
const HeapHashSet<Member<RuleSet>>&);
void invalidateSlottedElements(HTMLSlotElement&);
void updateViewport();
......
third_party/WebKit/Source/core/dom/StyleEngineTest.cpp
View file @ 5eefd373
......@@ -50,13 +50,13 @@ StyleEngineTest::scheduleInvalidationsForRules(TreeScope& treeScope,
StyleSheetContents* sheet =
StyleSheetContents::create(CSSParserContext(HTMLStandardMode, nullptr));
sheet->parseString(cssText);
HeapVector<Member<RuleSet>> ruleSets;
HeapHashSet<Member<RuleSet>> ruleSets;
RuleSet& ruleSet = sheet->ensureRuleSet(MediaQueryEvaluator(),
RuleHasDocumentSecurityOrigin);
ruleSet.compactRulesIfNeeded();
if (ruleSet.needsFullRecalcForRuleSetInvalidation())
return RuleSetInvalidationFullRecalc;
ruleSets.append(&ruleSet);
ruleSets.add(&ruleSet);
styleEngine().scheduleInvalidationsForRuleSets(treeScope, ruleSets);
return RuleSetInvalidationsScheduled;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment