Move LayoutObject to satellite NodeLayoutData that hangs from a Node.

third_party/WebKit/Source/core/dom/Node.h

Index: third_party/WebKit/Source/core/dom/Node.h
diff --git a/third_party/WebKit/Source/core/dom/Node.h b/third_party/WebKit/Source/core/dom/Node.h
index c3f294d8681dada445c22b00b0cb7067f96d6572..2b47b186a04f0f0cc8eb1352a5fc06e820899663 100644
--- a/third_party/WebKit/Source/core/dom/Node.h
+++ b/third_party/WebKit/Source/core/dom/Node.h
@@ -57,6 +57,7 @@ class EventDispatchHandlingState;
 class NodeList;
 class NodeListsNodeData;
 class NodeOrString;
+class NodeLayoutData;
 class NodeRareData;
 class QualifiedName;
 class RegisteredEventListener;
@@ -98,21 +99,39 @@ enum class SlotChangeType {
   kChained,
 };
 
-class NodeRareDataBase {
+class NodeLayoutData {
  public:
+  explicit NodeLayoutData(LayoutObject* layout_object)
+      : layout_object_(layout_object) {}
   LayoutObject* GetLayoutObject() const { return layout_object_; }
   void SetLayoutObject(LayoutObject* layout_object) {
     layout_object_ = layout_object;

[esprehn, 2017/04/13 00:30:47]

DCHECK_NE(&SharedEmptyData(), this);

[nainar, 2017/04/13 15:02:02]

Done. 

   }
+  static NodeLayoutData& SharedEmptyData() {
+    static NodeLayoutData shared_empty_data_(nullptr);
+    return shared_empty_data_;
+  }
+
+ private:
+  LayoutObject* layout_object_;
+};
+
+class NodeRareDataBase {
+ public:
+  NodeLayoutData* GetNodeLayoutData() const {
+    return node_layout_data_ ? node_layout_data_

[esprehn, 2017/04/13 00:30:47]

The point of the SharedEmptyData() is that we don't need this null check, we
should always either have node_layout_data_ be SharedEmptyData() or a unique
one, it's not nullable.

[nainar, 2017/04/13 15:02:02]

Changed to 
return node_layout_data_;

+                             : &NodeLayoutData::SharedEmptyData();
+  }
+  void SetNodeLayoutData(NodeLayoutData* node_layout_data) {

[esprehn, 2017/04/13 00:30:47]

DCHECK(node_layout_data)

[nainar, 2017/04/13 15:02:02]

Done.

+    node_layout_data_ = node_layout_data;
+  }
 
  protected:
-  NodeRareDataBase(LayoutObject* layout_object)
-      : layout_object_(layout_object) {}
+  NodeRareDataBase(NodeLayoutData* node_layout_data)
+      : node_layout_data_(node_layout_data) {}
 
  protected:
-  // LayoutObjects are fully owned by their DOM node. See LayoutObject's
-  // LIFETIME documentation section.
-  LayoutObject* layout_object_;
+  NodeLayoutData* node_layout_data_;

[esprehn, 2017/04/13 00:30:47]

you need a destructor for NodeRareDataBase and inside ~Node, that does:

if (node_layout_data_ && node_layout_data_ != SharedEmptyData())
  delete node_layout_data_;

or you need to make NodeLayoutData GarbageCollected and then this should all
work automatically since the static Persistent in SharedEmptyData would keep it
alive. Making it garbage collected is less code which is nice, you'll need to
modify the tracing function here:
https://cs.chromium.org/webrtc/src/third_party/WebKit/Source/core/dom/Node.cp...

Right now you're leaking this object though.

[nainar, 2017/04/13 15:02:02]

Done. 

 };
 
 class Node;
@@ -568,14 +587,24 @@ class CORE_EXPORT Node : public EventTarget {
   // Note that if a Node has a layoutObject, it's parentNode is guaranteed to
   // have one as well.
   LayoutObject* GetLayoutObject() const {
-    return HasRareData() ? data_.rare_data_->GetLayoutObject()
-                         : data_.layout_object_;
+    return HasRareData()
+               ? data_.rare_data_->GetNodeLayoutData()->GetLayoutObject()
+               : data_.node_layout_data_->GetLayoutObject();
   }
   void SetLayoutObject(LayoutObject* layout_object) {

[esprehn, 2017/04/13 00:30:47]

This function has become so big, I'd suggest moving out of line into the .cpp
file. The one function call is unlikely to be a perf issue since we're already
doing something so expensive when we call setLayoutObject.

[nainar, 2017/04/13 15:02:02]

Done. 

-    if (HasRareData())
-      data_.rare_data_->SetLayoutObject(layout_object);
-    else
-      data_.layout_object_ = layout_object;
+    if (HasRareData()) {

[nainar, 2017/04/13 00:10:22]

Can make this less complicated by adding a EnsureLayoutObject method on
NodeLayoutData, 

NodeLayoutData* NodeLayoutData::EnsureLayoutObject(layout_object) {
   if this == &NodeLayoutData::SharedEmptyData()
       return new NodeLayoutData(layout_object)
   else 
       SetLayoutObject(layout_object);
}

+      data_.rare_data_->GetNodeLayoutData() ==

[esprehn, 2017/04/13 00:30:47]

we should put this in a method on NodeLayoutData like bool IsShared()

[nainar, 2017/04/13 15:02:02]

Done.

+              &NodeLayoutData::SharedEmptyData()
+          ? data_.rare_data_->SetNodeLayoutData(
+                new NodeLayoutData(layout_object))
+          : data_.rare_data_->GetNodeLayoutData()->SetLayoutObject(
+                layout_object);
+    } else {
+      if (data_.node_layout_data_ == &NodeLayoutData::SharedEmptyData())

[esprehn, 2017/04/13 00:30:47]

if (data_.node_layout_data_->IsShared())

[nainar, 2017/04/13 15:02:02]

Done.

+        data_.node_layout_data_ = new NodeLayoutData(layout_object);
+      else
+        data_.node_layout_data_->SetLayoutObject(layout_object);
+    }
   }
 
   // Use these two methods with caution.
@@ -930,10 +959,10 @@ class CORE_EXPORT Node : public EventTarget {
   Member<Node> next_;
   // When a node has rare data we move the layoutObject into the rare data.
   union DataUnion {
-    DataUnion() : layout_object_(nullptr) {}
+    DataUnion() : node_layout_data_(&NodeLayoutData::SharedEmptyData()) {}
     // LayoutObjects are fully owned by their DOM node. See LayoutObject's
     // LIFETIME documentation section.
-    LayoutObject* layout_object_;
+    NodeLayoutData* node_layout_data_;
     NodeRareDataBase* rare_data_;
   } data_;
 };