[DMC #18] Eliminate DocumentMarkerTextMatch (subclass of DocumentMarkerDetails)

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

Index: third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
diff --git a/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp b/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
index b3b2c60c8eaa68108f474a5cbb60d7d438859b97..7a9fdd4801bd4c311eb6afa002c59d3aa4f49039 100644
--- a/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
+++ b/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
@@ -30,6 +30,7 @@
 
 #include "core/editing/markers/DocumentMarker.h"
 
+#include "core/editing/markers/TextMatchMarker.h"
 #include "platform/wtf/StdLibExtras.h"
 
 namespace blink {
@@ -62,43 +63,6 @@ inline DocumentMarkerDescription* ToDocumentMarkerDescription(
   return 0;
 }
 
-class DocumentMarkerTextMatch final : public DocumentMarkerDetails {
- public:
-  static DocumentMarkerTextMatch* Create(DocumentMarker::MatchStatus);
-
-  bool IsActiveMatch() const {
-    return match_status_ == DocumentMarker::MatchStatus::kActive;
-  }
-
-  bool IsTextMatch() const override { return true; }
-
- private:
-  explicit DocumentMarkerTextMatch(DocumentMarker::MatchStatus match_status)
-      : match_status_(match_status) {}
-
-  DocumentMarker::MatchStatus match_status_;
-};
-
-DocumentMarkerTextMatch* DocumentMarkerTextMatch::Create(
-    DocumentMarker::MatchStatus match_status) {
-  DEFINE_STATIC_LOCAL(
-      DocumentMarkerTextMatch, active_instance,
-      (new DocumentMarkerTextMatch(DocumentMarker::MatchStatus::kActive)));
-  DEFINE_STATIC_LOCAL(
-      DocumentMarkerTextMatch, inactive_instance,
-      (new DocumentMarkerTextMatch(DocumentMarker::MatchStatus::kInactive)));
-  return match_status == DocumentMarker::MatchStatus::kActive
-             ? &active_instance
-             : &inactive_instance;
-}
-
-inline DocumentMarkerTextMatch* ToDocumentMarkerTextMatch(
-    DocumentMarkerDetails* details) {
-  if (details && details->IsTextMatch())
-    return static_cast<DocumentMarkerTextMatch*>(details);
-  return 0;
-}
-
 class TextCompositionMarkerDetails final : public DocumentMarkerDetails {
  public:
   static TextCompositionMarkerDetails* Create(Color underline_color,
@@ -140,6 +104,14 @@ inline TextCompositionMarkerDetails* ToTextCompositionMarkerDetails(
 
 DocumentMarker::DocumentMarker(MarkerType type,
                                unsigned start_offset,
+                               unsigned end_offset)
+    : type_(type),
+      start_offset_(start_offset),
+      end_offset_(end_offset),
+      details_(nullptr) {}
+
+DocumentMarker::DocumentMarker(MarkerType type,
+                               unsigned start_offset,
                                unsigned end_offset,
                                const String& description)
     : type_(type),
@@ -151,14 +123,6 @@ DocumentMarker::DocumentMarker(MarkerType type,
 
 DocumentMarker::DocumentMarker(unsigned start_offset,
                                unsigned end_offset,
-                               DocumentMarker::MatchStatus match_status)
-    : type_(DocumentMarker::kTextMatch),
-      start_offset_(start_offset),
-      end_offset_(end_offset),
-      details_(DocumentMarkerTextMatch::Create(match_status)) {}
-
-DocumentMarker::DocumentMarker(unsigned start_offset,
-                               unsigned end_offset,
                                Color underline_color,
                                bool thick,
                                Color background_color)
@@ -225,11 +189,10 @@ void DocumentMarker::ShiftOffsets(int delta) {
   start_offset_ += delta;
   end_offset_ += delta;
 }
-
 void DocumentMarker::SetIsActiveMatch(bool active) {
-  details_ = DocumentMarkerTextMatch::Create(
-      active ? DocumentMarker::MatchStatus::kActive
-             : DocumentMarker::MatchStatus::kInactive);
+  if (GetType() != DocumentMarker::kTextMatch)

[rlanday, 2017/05/25 21:00:48]

Note the behavioral change here: calling SetIsActiveMatch() on a
non-TextMatchMarker currently overwrites the DocumentMarkerDetails pointer with
one that holds TextMatchMarker-specific data. I'm changing it to be a no-op in
this case. In a follow-up CL, I'll force callers to cast to TextMatchMarker to
call this method, avoiding the issue entirely.

[Xiaocheng, 2017/05/26 00:46:25]

We should be fine here, since DM::SetIsActiveMatch is called only by
DMC::SetTextMatchMarkersActive.

There shouldn't be real behavioral change.

Anyway, cleaning it up is good.

+    return;
+  ToTextMatchMarker(this)->SetIsActiveMatch(active);
 }
 
 const String& DocumentMarker::Description() const {
@@ -240,10 +203,9 @@ const String& DocumentMarker::Description() const {
 }
 
 bool DocumentMarker::IsActiveMatch() const {
-  if (DocumentMarkerTextMatch* details =
-          ToDocumentMarkerTextMatch(details_.Get()))
-    return details->IsActiveMatch();
-  return false;
+  if (GetType() != DocumentMarker::kTextMatch)
+    return false;
+  return ToTextMatchMarker(this)->IsActiveMatch();
 }
 
 Color DocumentMarker::UnderlineColor() const {