[WIP] Refactor DocumentMarker

third_party/WebKit/Source/core/editing/markers/DocumentMarker.h

 /*
  * This file is part of the DOM implementation for WebCore.
  *
  * Copyright (C) 2006 Apple Computer, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 13 matching lines @@
 #define DocumentMarker_h
 
 #include "core/CoreExport.h"
 #include "platform/graphics/Color.h"
 #include "platform/heap/Handle.h"
 #include "wtf/VectorTraits.h"
 #include "wtf/text/WTFString.h"
 
 namespace blink {
 
-class DocumentMarkerDetails;
-
 // A range of a node within a document that is "marked", such as the range of a
 // misspelled word. It optionally includes a description that could be displayed
 // in the user interface. It also optionally includes a flag specifying whether
 // the match is active, which is ignored for all types other than type
 // TextMatch.
-class CORE_EXPORT DocumentMarker : public GarbageCollected<DocumentMarker> {
+class CORE_EXPORT DocumentMarker
+    : public GarbageCollectedFinalized<DocumentMarker> {
  public:
   enum MarkerTypeIndex {
     SpellingMarkerIndex = 0,
     GrammarMarkerIndex,
     TextMatchMarkerIndex,
     CompositionMarkerIndex,
     MarkerTypeIndexesCount
   };
 
   enum MarkerType {
@@ skipping 75 matching lines @@
   class MisspellingMarkers : public MarkerTypes {
    public:
     MisspellingMarkers() : MarkerTypes(Spelling | Grammar) {}
   };
 
   class SpellCheckClientMarkers : public MarkerTypes {
    public:
     SpellCheckClientMarkers() : MarkerTypes(Spelling | Grammar) {}
   };
 
-  virtual DocumentMarker* clone() const;
+  virtual ~DocumentMarker();
 
-  DocumentMarker(MarkerType,
-                 unsigned startOffset,
-                 unsigned endOffset,
-                 const String& description);
-  DocumentMarker(unsigned startOffset,
-                 unsigned endOffset,
-                 Color underlineColor,
-                 bool thick,
-                 Color backgroundColor);
+  virtual DocumentMarker* clone() const = 0;
 
-  MarkerType type() const { return m_type; }
+  virtual MarkerType type() const = 0;
   unsigned startOffset() const { return m_startOffset; }
   unsigned endOffset() const { return m_endOffset; }
 
-  const String& description() const;
-  bool activeMatch() const;
-  Color underlineColor() const;
-  bool thick() const;
-  Color backgroundColor() const;
-  DocumentMarkerDetails* details() const;
-
-  void setActiveMatch(bool);
-  void clearDetails() { m_details.clear(); }
+  virtual const String& description() const;

[Xiaocheng, 2017/03/29 23:40:11]

These functions only make sense in subclasses. Can we move them down there?

More down-castings are expected after this change, but I think it's worthy.

+  virtual bool activeMatch() const;
+  virtual Color underlineColor() const;
+  virtual bool thick() const;
+  virtual Color backgroundColor() const;
 
   struct ShiftMarkerResult {
     unsigned newStartOffset;
     unsigned newEndOffset;
     bool shouldRemoveMarker;
   };
 
   ShiftMarkerResult getShiftedMarkerPosition(unsigned offset,
                                              unsigned oldLength,
                                              unsigned newLength) const;
 
   // Offset modifications are done by DocumentMarkerController and subclasses of
   // DocumentMarkerList. Other classes should not call following setters.
   void setStartOffset(unsigned offset) { m_startOffset = offset; }
   void setEndOffset(unsigned offset) { m_endOffset = offset; }
   void shiftOffsets(int delta);
 
-  bool operator==(const DocumentMarker& o) const {
-    return type() == o.type() && startOffset() == o.startOffset() &&
-           endOffset() == o.endOffset();
+  bool operator==(const DocumentMarker& other) const;

[Xiaocheng, 2017/03/29 23:40:11]

These comparison methods seem not used at all.

Could you remove them with another patch?

+
+  bool operator!=(const DocumentMarker& other) const {
+    return !(*this == other);
   }
 
-  bool operator!=(const DocumentMarker& o) const { return !(*this == o); }
-
-  DECLARE_TRACE();
+  DEFINE_INLINE_VIRTUAL_TRACE() {}
 
  protected:
   DocumentMarker(const DocumentMarker&);
-  DocumentMarker(unsigned startOffset, unsigned endOffset, bool activeMatch);
+  DocumentMarker(unsigned startOffset, unsigned endOffset);
 
  private:
-  MarkerType m_type;
   unsigned m_startOffset;
   unsigned m_endOffset;
-  Member<DocumentMarkerDetails> m_details;
 };
 
 using DocumentMarkerVector = HeapVector<Member<DocumentMarker>>;
 
-inline DocumentMarkerDetails* DocumentMarker::details() const {
-  return m_details.get();
-}
-
-class DocumentMarkerDetails
-    : public GarbageCollectedFinalized<DocumentMarkerDetails> {
- public:
-  DocumentMarkerDetails() {}
-  virtual ~DocumentMarkerDetails();
-  virtual bool isDescription() const { return false; }
-  virtual bool isTextMatch() const { return false; }
-  virtual bool isComposition() const { return false; }
-
-  DEFINE_INLINE_VIRTUAL_TRACE() {}
-};
-
 }  // namespace blink
 
 #endif  // DocumentMarker_h