Oilpan: Move DOMEditor to the heap

Source/core/inspector/DOMEditor.h

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 13 matching lines @@
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #ifndef DOMEditor_h
 #define DOMEditor_h
 
+#include "platform/heap/Handle.h"
 #include "wtf/text/WTFString.h"
 
 namespace WebCore {
 
 class Element;
 class ExceptionState;
 class InspectorHistory;
 class Node;
 class Text;
 
 typedef String ErrorString;
 
-class DOMEditor {
-    WTF_MAKE_NONCOPYABLE(DOMEditor); WTF_MAKE_FAST_ALLOCATED;
+class DOMEditor FINAL : public NoBaseWillBeGarbageCollected<DOMEditor> {

[pfeldman, 2014/05/22 14:47:21]

What is the motivation of this change? DOMEditor is a simple non-refcounted
class with clear lifetime.

[haraken, 2014/05/22 20:19:31]

- InspectorHistory is moved to the heap.
- So if we keep DOMEditor off-heap, we need to use a
Persistent<InspectorHistory> for DOMEditor::m_history.
- However, we don't want to increase the number of persistent handles, it would
be better to move DOMEditor to the heap at the same time and use
Member<DOMEditor> for DOMEditor::m_history.

[pfeldman, 2014/05/22 20:43:25]

Why moving history to heap? Because some actions point to soon-to-be heap
objects?

I am a bit concerned with conversion of simple non-hierarchical data structures
with clear lifetime such as DOMEditor into the Oilpan heap. It makes it hard to
follow the ownership and track the new object lifetimes.

So we need to draw the line somewhere and I am not sure I know what the
guidelines / criteria for that is. Or do we want to move as much as possible
into heap? Would LocalFrame stay a regular object?

[haraken, 2014/05/22 21:21:57]

- We're currently moving the Node hierarchy to the heap.
- Because the Action hierarchy has a bunch of RefPtrs to the Node, we want to
move the Action hierarchy to the heap.
- Because InspectorHistory has a vector of Actions, we want to move
InspectorHistory to the heap.

If we don't move them to the heap at the same time, we need to use persistent
handles, which we want to avoid as much as possible.
In the end, we're planning to move:

- almost all RefCounted objects
- objects that have references to the RefCounted objects (Otherwise we need to
use persistent handles)
- strings and collections

to the heap.

The reason why we want to avoid persistent handles is that (1) persistent
handles are heavy in terms of both performance and memory and that (2)
persistent handles can cause leaks.

For example:

class A {
  Persistent<B> m_b;
};

class B : public GarbageCollectedFinalized<B> {
  OwnPtr<A> m_a;
};

will cause a cycle, so we need to move A to the heap at the same time:

class A : public GarbageCollected<A> {
  Member<B> m_b;
};

class B : public GarbageCollected<B> {
  Member<A> m_a;
};

+    WTF_MAKE_NONCOPYABLE(DOMEditor);
+    WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED;
 public:
     explicit DOMEditor(InspectorHistory*);
-    ~DOMEditor();
+
+    void trace(Visitor*);
 
     bool insertBefore(Node* parentNode, PassRefPtr<Node>, Node* anchorNode, ExceptionState&);
     bool removeChild(Node* parentNode, Node*, ExceptionState&);
     bool setAttribute(Element*, const String& name, const String& value, ExceptionState&);
     bool removeAttribute(Element*, const String& name, ExceptionState&);
     bool setOuterHTML(Node*, const String& html, Node** newNode, ExceptionState&);
     bool replaceWholeText(Text*, const String& text, ExceptionState&);
     bool replaceChild(Node* parentNode, PassRefPtr<Node> newNode, Node* oldNode, ExceptionState&);
     bool setNodeValue(Node* parentNode, const String& value, ExceptionState&);
 
     bool insertBefore(Node* parentNode, PassRefPtr<Node>, Node* anchorNode, ErrorString*);
     bool removeChild(Node* parentNode, Node*, ErrorString*);
     bool setAttribute(Element*, const String& name, const String& value, ErrorString*);
     bool removeAttribute(Element*, const String& name, ErrorString*);
     bool setOuterHTML(Node*, const String& html, Node** newNode, ErrorString*);
     bool replaceWholeText(Text*, const String& text, ErrorString*);
 
 private:
     class DOMAction;
     class RemoveChildAction;
     class InsertBeforeAction;
     class RemoveAttributeAction;
     class SetAttributeAction;
     class SetOuterHTMLAction;
     class ReplaceWholeTextAction;
     class ReplaceChildNodeAction;
     class SetNodeValueAction;
 
-    InspectorHistory* m_history;
+    RawPtrWillBeMember<InspectorHistory> m_history;
 };
 
 
 } // namespace WebCore
 
 #endif // !defined(DOMEditor_h)