Stash ComputedStyles on new HeapHashMap on Document.

third_party/WebKit/Source/core/dom/Document.cpp

Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index cc1a8a9ad6e01f3107621b34bdde57373c14ca40..fd5e9f7301da01091c9373a72fe833e18d62fdda 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -1246,6 +1246,24 @@ Element* Document::scrollingElement() {
   return body();
 }
 
+// We use HashMap::set over HashMap::add here as we want to
+// replace the ComputedStyle but not the Element if the Element is
+// already present.
+void Document::addNonAttachedStyle(Element* element,
+                                   RefPtr<ComputedStyle> computedStyle) {
+  m_nonAttachedStyle.set(element, computedStyle);
+}
+
+RefPtr<ComputedStyle> Document::getNonAttachedStyle(Element* element) {
+  return m_nonAttachedStyle.get(element);
+}
+
+void Document::clear() {

[esprehn, 2016/10/05 01:04:25]

Remove this function, you can't add something so generic as clear() to Document.

[nainar, 2016/10/05 02:00:01]

Done.

+  for (NonAttachedStyleMap::iterator iter = m_nonAttachedStyle.begin();
+       iter != m_nonAttachedStyle.end(); ++iter)
+    m_nonAttachedStyle.remove(iter);

[esprehn, 2016/10/05 01:04:25]

m_nonAttachedStyle.clear(), note that calling remove() like this on a hash map
is not allowed. You can't mutate the hash map while you're iterating. This is
also going to be very slow since you're causing the hashmap to repeatedly resize

[nainar, 2016/10/05 02:00:01]

Done.

+}
+
 /*
  * Performs three operations:
  *  1. Convert control characters to spaces
@@ -1876,6 +1894,9 @@ void Document::updateStyle() {
   HTMLFrameOwnerElement::UpdateSuspendScope suspendWidgetHierarchyUpdates;
   m_lifecycle.advanceTo(DocumentLifecycle::InStyleRecalc);
 
+  // Reserving capacity to avoid rehashes.
+  m_nonAttachedStyle.reserveCapacityForSize(initialElementCount);

[esprehn, 2016/10/05 01:04:25]

initialElementCount doesn't mean what you think it does, it's the number of
times we called styleForElement in recalcStyle (for all time). That number is
ever increasing, which means you're creating a bigger and bigger hash map here
every time.

I'd leave this out

[nainar, 2016/10/05 02:00:01]

Done.

+
   StyleRecalcChange change = NoChange;
   if (getStyleChangeType() >= SubtreeStyleChange)
     change = Force;
@@ -1917,6 +1938,9 @@ void Document::updateStyle() {
 
   view()->recalcOverflowAfterStyleChange();
 
+  // Only retain the HashMap for the duration of StyleRecalc and
+  // LayoutTreeConstruction.
+  m_nonAttachedStyle.clear();
   clearChildNeedsStyleRecalc();
 
   resolver.clearStyleSharingList();
@@ -1927,6 +1951,7 @@ void Document::updateStyle() {
   DCHECK(!childNeedsStyleRecalc());
   DCHECK(inStyleRecalc());
   DCHECK_EQ(styleResolver(), &resolver);
+  DCHECK(m_nonAttachedStyle.isEmpty());
   m_lifecycle.advanceTo(DocumentLifecycle::StyleClean);
   if (shouldRecordStats) {
     TRACE_EVENT_END2("blink,blink_style", "Document::updateStyle",
@@ -6326,6 +6351,7 @@ DEFINE_TRACE(Document) {
   visitor->trace(m_snapCoordinator);
   visitor->trace(m_resizeObserverController);
   visitor->trace(m_propertyRegistry);
+  visitor->trace(m_nonAttachedStyle);
   Supplementable<Document>::trace(visitor);
   TreeScope::trace(visitor);
   ContainerNode::trace(visitor);