Move markerlists to Oilpan heap and remove finalizer from DocumentMarkerController

Source/core/dom/DocumentMarkerController.h

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 24 matching lines @@
 
 namespace WebCore {
 
 class ContainerNode;
 class LayoutPoint;
 class LayoutRect;
 class Node;
 class Range;
 class RenderedDocumentMarker;
 
-class DocumentMarkerController FINAL : public NoBaseWillBeGarbageCollectedFinalized<DocumentMarkerController> {
+class DocumentMarkerController FINAL : public NoBaseWillBeGarbageCollected<DocumentMarkerController> {
     WTF_MAKE_NONCOPYABLE(DocumentMarkerController); WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED;
 public:
 
     DocumentMarkerController();
-    ~DocumentMarkerController();

[haraken, 2014/05/19 07:13:20]

You can use DEFINE_EMPTY_VIRTUAL_DESTRUCTOR_WILL_BE_REMOVED.

[Erik Corry, 2014/05/19 07:52:11]

Not writing anything at all seems better though.

[Erik Corry, 2014/05/20 08:40:37]

I put this back after our offline discussion this morning.

 
     void clear();
     void addMarker(Range*, DocumentMarker::MarkerType);
     void addMarker(Range*, DocumentMarker::MarkerType, const String& description);
     void addMarker(Range*, DocumentMarker::MarkerType, const String& description, uint32_t hash);
     void addTextMatchMarker(const Range*, bool activeMatch);
 
     void copyMarkers(Node* srcNode, unsigned startOffset, int length, Node* dstNode, int delta);
     bool hasMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
 
     void prepareForDestruction();
     // When a marker partially overlaps with range, if removePartiallyOverlappingMarkers is true, we completely
     // remove the marker. If the argument is false, we will adjust the span of the marker so that it retains
     // the portion that is outside of the range.
     enum RemovePartiallyOverlappingMarkerOrNot { DoNotRemovePartiallyOverlappingMarker, RemovePartiallyOverlappingMarker };
     void removeMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(), RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
     void removeMarkers(Node*, unsigned startOffset, int length, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(),  RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
 
     void removeMarkers(DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
     void removeMarkers(Node*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
     void repaintMarkers(DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
     void invalidateRenderedRectsForMarkersInRect(const LayoutRect&);
     void shiftMarkers(Node*, unsigned startOffset, int delta);
     void setMarkersActive(Range*, bool);
     void setMarkersActive(Node*, unsigned startOffset, unsigned endOffset, bool);
 
     DocumentMarker* markerContainingPoint(const LayoutPoint&, DocumentMarker::MarkerType);
-    Vector<DocumentMarker*> markersFor(Node*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
-    Vector<DocumentMarker*> markersInRange(Range*, DocumentMarker::MarkerTypes);
-    Vector<DocumentMarker*> markers();
+    WillBeHeapVector<DocumentMarker*> markersFor(Node*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());

[haraken, 2014/05/19 07:13:20]

Let's use WillBeHeapVector<Member<DocumentMarker>> (not for correctness but for
consistency of programming rules).

The same comment for other parts in this CL.

[Erik Corry, 2014/05/19 07:52:11]

DocumentMarker is not on the heap.

+    WillBeHeapVector<DocumentMarker*> markersInRange(Range*, DocumentMarker::MarkerTypes);
+    WillBeHeapVector<DocumentMarker*> markers();
     Vector<IntRect> renderedRectsForMarkers(DocumentMarker::MarkerType);
 
     void trace(Visitor*);
 
 #ifndef NDEBUG
     void showMarkers() const;
 #endif
 
 private:
     void addMarker(Node*, const DocumentMarker&);
 
-    typedef Vector<RenderedDocumentMarker> MarkerList;
-    typedef Vector<OwnPtr<MarkerList>, DocumentMarker::MarkerTypeIndexesCount> MarkerLists;
-    typedef WillBeHeapHashMap<RawPtrWillBeWeakMember<const Node>, OwnPtr<MarkerLists> > MarkerMap;
+    typedef WillBeHeapVector<RenderedDocumentMarker> MarkerList;

[sof, 2014/05/19 07:05:24]

Clarifying Q: DocumentMarker objects contain a RefPtr<>, so they need to be
finalized. All controlled and taken care of by the vector traits for
DocumentMarker (used by the MarkerList HeapVector<>) ?

[Erik Corry, 2014/05/19 07:52:11]

Yes, SimpleClassVectorTraits are allowed RefPtrs and are destructed.  They can't
have virtual methods though because they have canInitializeWithMemset

+    typedef WillBeHeapVector<OwnPtrWillBeMember<MarkerList>, DocumentMarker::MarkerTypeIndexesCount> MarkerLists;
+    typedef WillBeHeapHashMap<RawPtrWillBeWeakMember<const Node>, OwnPtrWillBeMember<MarkerLists> > MarkerMap;
     void mergeOverlapping(MarkerList*, DocumentMarker&);
     bool possiblyHasMarkers(DocumentMarker::MarkerTypes);
     void removeMarkersFromList(MarkerMap::iterator, DocumentMarker::MarkerTypes);
 
     MarkerMap m_markers;
     // Provide a quick way to determine whether a particular marker type is absent without going through the map.
     DocumentMarker::MarkerTypes m_possiblyExistingMarkerTypes;
 };
 
 } // namespace WebCore
 
 #ifndef NDEBUG
 void showDocumentMarkers(const WebCore::DocumentMarkerController*);
 #endif
 
 #endif // DocumentMarkerController_h