Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(567)

Unified Diff: Source/core/css/StyleSheetContents.cpp

Issue 196653006: Optimize StyleSheetContents::checkLoaded by keeping track of completed (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Use early return Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/core/css/StyleSheetContents.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/css/StyleSheetContents.cpp
diff --git a/Source/core/css/StyleSheetContents.cpp b/Source/core/css/StyleSheetContents.cpp
index 37c644fd5f241c0d4b727c1967276e499e8ce34b..c4b489185123388e83612cfb085fb928b371115f 100644
--- a/Source/core/css/StyleSheetContents.cpp
+++ b/Source/core/css/StyleSheetContents.cpp
@@ -374,10 +374,8 @@ bool StyleSheetContents::loadCompleted() const
return parentSheet->loadCompleted();
StyleSheetContents* root = rootStyleSheet();
- for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it) {
- if (!(*it)->loadCompleted())
- return false;
- }
+ if (root->m_loadingClients.size())
+ return false;
return true;
esprehn 2014/03/13 19:56:28 return rootStyleSheet()->m_loadingClients.isEmpty(
Mads Ager (chromium) 2014/03/14 09:30:18 Heh, yes, thanks!
}
@@ -398,7 +396,7 @@ void StyleSheetContents::checkLoaded()
}
StyleSheetContents* root = rootStyleSheet();
- if (root->m_clients.isEmpty())
+ if (root->m_loadingClients.isEmpty())
return;
// Avoid |CSSSStyleSheet| and |ownerNode| being deleted by scripts that run via
@@ -409,8 +407,9 @@ void StyleSheetContents::checkLoaded()
// FIXME: oilpan: with oilpan we do not need to protect the CSSStyleSheets
// explicitly during the iteration. The iterator will make the pointers
// strong during iteration so we can just directly iterate root->m_clients.
- WillBeHeapVector<RefPtrWillBeMember<CSSStyleSheet>, 5> protectedClients;
- for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it)
+ WillBeHeapVector<RefPtrWillBeMember<CSSStyleSheet> > protectedClients;
+ protectedClients.reserveInitialCapacity(root->m_loadingClients.size());
+ for (ClientsIterator it = root->m_loadingClients.begin(); it != root->m_loadingClients.end(); ++it)
protectedClients.append(*it);
esprehn 2014/03/13 19:56:28 This should just use copyToVector()
Mads Ager (chromium) 2014/03/14 09:30:18 Oh, I didn't know about copyToVector. Much better!
for (unsigned i = 0; i < protectedClients.size(); ++i) {
@@ -438,7 +437,9 @@ void StyleSheetContents::notifyLoadedSheet(const CSSStyleSheetResource* sheet)
void StyleSheetContents::startLoadingDynamicSheet()
{
StyleSheetContents* root = rootStyleSheet();
- for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it)
+ for (ClientsIterator it = root->m_loadingClients.begin(); it != root->m_loadingClients.end(); ++it)
+ (*it)->startLoadingDynamicSheet();
+ for (ClientsIterator it = root->m_completedClients.begin(); it != root->m_completedClients.end(); ++it)
(*it)->startLoadingDynamicSheet();
}
@@ -453,18 +454,17 @@ StyleSheetContents* StyleSheetContents::rootStyleSheet() const
bool StyleSheetContents::hasSingleOwnerNode() const
{
StyleSheetContents* root = rootStyleSheet();
- if (root->m_clients.isEmpty())
- return false;
- return root->m_clients.size() == 1;
+ return root->hasOneClient();
esprehn 2014/03/13 19:56:28 rootStyleSheet()->hasOneClient()
Mads Ager (chromium) 2014/03/14 09:30:18 Done.
}
Node* StyleSheetContents::singleOwnerNode() const
{
StyleSheetContents* root = rootStyleSheet();
- if (root->m_clients.isEmpty())
- return 0;
- ASSERT(root->m_clients.size() == 1);
- return (*root->m_clients.begin())->ownerNode();
+ if (root->hasOneClient()) {
+ return root->m_loadingClients.size() ? (*root->m_loadingClients.begin())->ownerNode() : (*root->m_completedClients.begin())->ownerNode();
esprehn 2014/03/13 19:56:28 Lets break this up since it's easier to read. if
Mads Ager (chromium) 2014/03/14 09:30:18 Yes, much better, thanks.
+ }
+ ASSERT(root->m_loadingClients.isEmpty() && root->m_completedClients.isEmpty());
+ return 0;
}
Document* StyleSheetContents::singleOwnerDocument() const
@@ -525,14 +525,29 @@ StyleSheetContents* StyleSheetContents::parentStyleSheet() const
void StyleSheetContents::registerClient(CSSStyleSheet* sheet)
{
- ASSERT(!m_clients.contains(sheet));
- m_clients.add(sheet);
+ ASSERT(!m_loadingClients.contains(sheet) && !m_completedClients.contains(sheet));
+ m_loadingClients.add(sheet);
esprehn 2014/03/13 19:56:28 Why is it a HashSet if you're just going to ASSERT
Mads Ager (chromium) 2014/03/14 09:30:18 The set of clients is a weak set. It does not keep
}
void StyleSheetContents::unregisterClient(CSSStyleSheet* sheet)
{
- ASSERT(m_clients.contains(sheet));
- m_clients.remove(sheet);
+ ASSERT(m_loadingClients.contains(sheet) || m_completedClients.contains(sheet));
+ m_loadingClients.remove(sheet);
+ m_completedClients.remove(sheet);
+}
+
+void StyleSheetContents::clientLoadCompleted(CSSStyleSheet* sheet)
+{
+ ASSERT(m_loadingClients.contains(sheet));
+ m_loadingClients.remove(sheet);
+ m_completedClients.add(sheet);
+}
+
+void StyleSheetContents::clientLoadStarted(CSSStyleSheet* sheet)
+{
+ ASSERT(m_completedClients.contains(sheet));
+ m_completedClients.remove(sheet);
+ m_loadingClients.add(sheet);
}
void StyleSheetContents::addedToMemoryCache()
@@ -564,6 +579,14 @@ RuleSet& StyleSheetContents::ensureRuleSet(const MediaQueryEvaluator& medium, Ad
return *m_ruleSet.get();
}
+static void clearResolvers(WillBeHeapHashSet<RawPtrWillBeWeakMember<CSSStyleSheet> >& clients)
+{
+ for (WillBeHeapHashSet<RawPtrWillBeWeakMember<CSSStyleSheet> >::iterator it = clients.begin(); it != clients.end(); ++it) {
+ if (Document* document = (*it)->ownerDocument())
+ document->styleEngine()->clearResolver();
+ }
+}
+
void StyleSheetContents::clearRuleSet()
{
if (StyleSheetContents* parentSheet = parentStyleSheet())
@@ -577,23 +600,26 @@ void StyleSheetContents::clearRuleSet()
// Clearing the ruleSet means we need to recreate the styleResolver data structures.
// See the StyleResolver calls in ScopedStyleResolver::addRulesFromSheet.
- for (ClientsIterator it = m_clients.begin(); it != m_clients.end(); ++it) {
- if (Document* document = (*it)->ownerDocument())
- document->styleEngine()->clearResolver();
- }
+ clearResolvers(m_loadingClients);
+ clearResolvers(m_completedClients);
m_ruleSet.clear();
}
-void StyleSheetContents::notifyRemoveFontFaceRule(const StyleRuleFontFace* fontFaceRule)
+static void removeFontFaceRules(WillBeHeapHashSet<RawPtrWillBeWeakMember<CSSStyleSheet> >& clients, const StyleRuleFontFace* fontFaceRule)
{
- StyleSheetContents* root = rootStyleSheet();
-
- for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it) {
+ for (WillBeHeapHashSet<RawPtrWillBeWeakMember<CSSStyleSheet> >::iterator it = clients.begin(); it != clients.end(); ++it) {
if (Node* ownerNode = (*it)->ownerNode())
ownerNode->document().styleEngine()->removeFontFaceRules(WillBeHeapVector<RawPtrWillBeMember<const StyleRuleFontFace> >(1, fontFaceRule));
}
}
+void StyleSheetContents::notifyRemoveFontFaceRule(const StyleRuleFontFace* fontFaceRule)
+{
+ StyleSheetContents* root = rootStyleSheet();
+ removeFontFaceRules(root->m_loadingClients, fontFaceRule);
+ removeFontFaceRules(root->m_completedClients, fontFaceRule);
+}
+
static void findFontFaceRulesFromRules(const WillBeHeapVector<RefPtrWillBeMember<StyleRuleBase> >& rules, WillBeHeapVector<RawPtrWillBeMember<const StyleRuleFontFace> >& fontFaceRules)
{
for (unsigned i = 0; i < rules.size(); ++i) {
@@ -626,7 +652,8 @@ void StyleSheetContents::trace(Visitor* visitor)
visitor->trace(m_ownerRule);
visitor->trace(m_importRules);
visitor->trace(m_childRules);
- visitor->trace(m_clients);
+ visitor->trace(m_loadingClients);
+ visitor->trace(m_completedClients);
visitor->trace(m_ruleSet);
}
« no previous file with comments | « Source/core/css/StyleSheetContents.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698