Oilpan: Switch RefCountedGarbageCollected to GarbageCollectedFinalized for Node.

Source/core/dom/Node.h

Index: Source/core/dom/Node.h
diff --git a/Source/core/dom/Node.h b/Source/core/dom/Node.h
index ab6025ec6b05c7eb185dfea346072bce1dfc687f..f1908e8a0c951445f9198f60917d31d207c1a0fd 100644
--- a/Source/core/dom/Node.h
+++ b/Source/core/dom/Node.h
@@ -107,12 +107,12 @@ private:
     RenderObject* m_renderer;
 };
 
-class Node : public EventTarget, public ScriptWrappable, public TreeSharedWillBeRefCountedGarbageCollected<Node> {
+class Node : public EventTarget, public ScriptWrappable, public TreeSharedWillBeGarbageCollectedFinalized<Node> {

[haraken, 2014/06/11 04:47:55]

GarbageCollectedFinalized should be the left-most class.

However, I'm a bit confused:

- Why was the previous code working with having RefCountedGarbageCollected at
not the left-most class?

- Why doesn't the plugin detect the error?

[haraken, 2014/06/11 04:47:55]

Ian: This is a question not related to this CL, but are we ready to replace
RefCountedGarbageCollected of classes that derive EventTarget with
GarbageCollectedFinalized?

[haraken, 2014/06/11 04:59:04]

On the other hand, I remembered that TreeShared must be the right-most class,
because we want to align TreeShared::m_refCount (32 bit interger) and
Node::m_nodeFlags (32 bit interger) into one 64 bit slot. If we put the
TreeShared at the left-most class, it will increase sizeof(Node) by 8 byte.

So you'll need to do:

// Some comment
#if ENABLE(OILPAN)
class Node : public GarbageCollectedFinalized<Node>, public EventTarget, public
ScriptWrappable {
#else
class Node : public EventTarget, public ScriptWrappable, public TreeShared<Node>
{
#endif

[Mads Ager (chromium), 2014/06/11 05:48:22]

It seems that we got lucky that the layout of the object generated by the
compiler is such that a Node pointer always points to the start of the object
even though GarbageCollectedFinalized is not inherited as the left-most base
class. We should move it around as you suggest Haraken so that we are sure that
it becomes the left-most base class.
Good question. We should update the plugin to catch this.

[Mads Ager (chromium), 2014/06/11 05:48:22]

Yes, Ian's change to move all EventTargets to the heap and use tracing instead
of ref counting for them has landed, so this is good.

[tkent, 2014/06/12 10:12:49]

Done.

     friend class Document;
     friend class TreeScope;
     friend class TreeScopeAdopter;
 
-    DEFINE_EVENT_TARGET_REFCOUNTING(TreeSharedWillBeRefCountedGarbageCollected<Node>);
+    DEFINE_EVENT_TARGET_REFCOUNTING_WILL_BE_REMOVED(TreeSharedWillBeGarbageCollectedFinalized<Node>);
     WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(Node);
 public:
     enum NodeType {
@@ -903,7 +903,7 @@ DEFINE_COMPARISON_OPERATORS_WITH_REFERENCES_REFCOUNTED(Node)
 #define DEFINE_NODE_FACTORY(T) \
 PassRefPtrWillBeRawPtr<T> T::create(Document& document) \
 { \
-    return adoptRefWillBeRefCountedGarbageCollected(new T(document)); \
+    return adoptRefWillBeNoop(new T(document)); \
 }
 
 } // namespace WebCore