Store nonAttachedStyle on NodeLayoutData instead of on HashMap in 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 9e6b7c37f71635fba78318c06a7f27d988e7ba0e..86ea126642208c22b37eac3877686b5ac9ba5188 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -1433,19 +1433,6 @@ Element* Document::ScrollingElementNoLayout() {
   return body();
 }
 
-// We use HashMap::set over HashMap::add here as we want to
-// replace the ComputedStyle but not the Node if the Node is
-// already present.
-void Document::AddNonAttachedStyle(const Node& node,
-                                   RefPtr<ComputedStyle> computed_style) {
-  DCHECK(node.IsElementNode() || node.IsTextNode());
-  non_attached_style_.Set(&node, computed_style);
-}
-
-ComputedStyle* Document::GetNonAttachedStyle(const Node& node) const {
-  return non_attached_style_.at(&node);
-}
-
 /*
  * Performs three operations:
  *  1. Convert control characters to spaces
@@ -2127,9 +2114,6 @@ void Document::UpdateStyle() {
 
   View()->RecalcOverflowAfterStyleChange();
 
-  // Only retain the HashMap for the duration of StyleRecalc and
-  // LayoutTreeConstruction.
-  non_attached_style_.Clear();

[rune, 2017/04/19 20:34:52]

Do we retain the ComputedStyle on each node now, even after we have created the
layout objects?

Also, does that mean that if we later detach a subtree (layout tree), we will
still retain all descendant ComputedStyles in a display:none subtree? If so, is
that intentional?

[nainar, 2017/04/19 23:43:41]

For the needs of this patch I should delete the computedStyle once we get the
info in rebuildLayoutTree but in a future patch I do want to retain
ComputedStyle for even display:non elements. 

I don't know what is the best place to clear the non attached ComputedStyle.
Can't do it in Element::rebuildLayoutTree because of the SharedStyleFinder which
may get called in LayoutTreeBuilder. And clearing it in LayoutTreeBuilder::style
would mean that elements that don't get a LayoutObject will still retain this
information. 

[rune, 2017/04/20 08:56:39]

So, if we have a subtree of N elements which is initially rendered, and make the
root of that subtree display:none and never display it again, should we retain
all N ComputedStyles for the lifetime of the subtree?

Later, it would make sense to unify non-attached style with the computed_style_
in ElementRareData?
It looks like we currently only store non-attached style for the root of a
layout tree which needs rebuilding (I know this is not the end goal). Can't you
clear the non-attached style after calling ReattachLayoutTree() in
Element::RebuildLayoutTree()?

[nainar, 2017/04/21 00:51:30]

We shouldn't be retaining the nonAttachedStyle for those nodes at this stage. I
have the same intent to unify it with the ComputedStyle stored on
ElementRareData. Iterating over all descendants clearing their nonAttachedStyle
when the root is set to display:none sounds liek a new perf regression to me
though. Am I wrong?
Done. 

[rune, 2017/04/21 07:41:46]

No, this was more of a future concern.
Detach will traverse over displayed descendants of the root and the rare-data
computed style is already cleared in DetachLayoutTree or AttachLayoutTree today:

https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/d...

[nainar, 2017/04/26 02:55:16]

Done. 

Had to change Node::setNonAttachedStyle() to incorporate the ability to set
nonAttachedStyle to null

[rune, 2017/04/26 08:47:28]

Sorry for being unclear, but I think the patch set at the time I lgtm'ed was
ready for landing (modulo any SetNonAttachedStyle() you had to change).

I was just thinking ahead about a future CL where we unify non-attached style
with non-layout-object-computed-style. I think the non-attached style is only
set for the root node of a subtree being re-attached as RecalcStyle is currently
skipping descendants when we get a kReattach change.

   ClearChildNeedsStyleRecalc();
   ClearChildNeedsReattachLayoutTree();
 
@@ -2141,7 +2125,6 @@ void Document::UpdateStyle() {
   DCHECK(!ChildNeedsReattachLayoutTree());
   DCHECK(InStyleRecalc());
   DCHECK_EQ(GetStyleResolver(), &resolver);
-  DCHECK(non_attached_style_.IsEmpty());
   lifecycle_.AdvanceTo(DocumentLifecycle::kStyleClean);
   if (should_record_stats) {
     TRACE_EVENT_END2(
@@ -6593,7 +6576,6 @@ DEFINE_TRACE(Document) {
   visitor->Trace(snap_coordinator_);
   visitor->Trace(resize_observer_controller_);
   visitor->Trace(property_registry_);
-  visitor->Trace(non_attached_style_);
   visitor->Trace(network_state_observer_);
   Supplementable<Document>::Trace(visitor);
   TreeScope::Trace(visitor);