Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Issue 2820633002: [DMC #2] Add DocumentMarkerList interface and GenericDocumentMarkerListImpl (Closed)
Patch Set: Add TODO Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h ('k') | third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h » ('j') | third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
diff --git a/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
index cdb98a30896cb1db02e89065cec3f65ecd6b0c35..1365036205cc3e29f626b15e034b98ca73ec2be9 100644
--- a/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
+++ b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
@@ -35,6 +35,7 @@
#include "core/dom/Text.h"
#include "core/editing/iterators/TextIterator.h"
#include "core/editing/markers/DocumentMarkerListEditor.h"
+#include "core/editing/markers/GenericDocumentMarkerListImpl.h"
#include "core/editing/markers/RenderedDocumentMarker.h"
#include "core/frame/FrameView.h"
#include "core/layout/LayoutObject.h"
@@ -66,9 +67,9 @@ DocumentMarker::MarkerTypeIndex MarkerTypeToMarkerIndex(
} // namespace
-Member<DocumentMarkerController::MarkerList>&
-DocumentMarkerController::ListForType(MarkerLists* marker_lists,
- DocumentMarker::MarkerType type) {
+Member<DocumentMarkerList>& DocumentMarkerController::ListForType(
+ MarkerLists* marker_lists,
+ DocumentMarker::MarkerType type) {
const size_t marker_list_index = MarkerTypeToMarkerIndex(type);
return (*marker_lists)[marker_list_index];
}
@@ -205,11 +206,14 @@ void DocumentMarkerController::AddMarker(Node* node,
}
const DocumentMarker::MarkerType new_marker_type = new_marker->GetType();
- if (!ListForType(markers, new_marker_type))
- ListForType(markers, new_marker_type) = new MarkerList;
+ if (!ListForType(markers, new_marker_type)) {
+ // TODO(rlanday): add method for deciding what type of list to create based
+ // on the MarkerType
+ ListForType(markers, new_marker_type) = new GenericDocumentMarkerListImpl;
+ }
- Member<MarkerList>& list = ListForType(markers, new_marker_type);
- DocumentMarkerListEditor::AddMarker(list, new_marker);
+ DocumentMarkerList* list = ListForType(markers, new_marker_type);
yosin_UTC9 2017/04/25 08:20:58 nit: s/DocumentMarkerList*/DocumentMarkerList* con
+ list->Add(new_marker);
// repaint the affected node
if (node->GetLayoutObject()) {
@@ -242,15 +246,18 @@ void DocumentMarkerController::MoveMarkers(Node* src_node,
bool doc_dirty = false;
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- MarkerList* src_list = ListForType(src_markers, type);
+ DocumentMarkerList* src_list = ListForType(src_markers, type);
yosin_UTC9 2017/04/25 08:20:58 nit: s/DocumentMarkerList*/DocumentMarkerList* con
if (!src_list)
continue;
- if (!ListForType(dst_markers, type))
- ListForType(dst_markers, type) = new MarkerList;
- MarkerList* dst_list = ListForType(dst_markers, type);
+ if (!ListForType(dst_markers, type)) {
+ // TODO(rlanday): add method for deciding what type of list to create
+ // based on the MarkerType
+ ListForType(dst_markers, type) = new GenericDocumentMarkerListImpl;
+ }
- if (DocumentMarkerListEditor::MoveMarkers(src_list, length, dst_list))
+ DocumentMarkerList* dst_list = ListForType(dst_markers, type);
+ if (src_list->MoveMarkers(length, dst_list))
doc_dirty = true;
}
@@ -280,21 +287,21 @@ void DocumentMarkerController::RemoveMarkersInternal(
bool doc_dirty = false;
size_t empty_lists_count = 0;
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- Member<MarkerList>& list = ListForType(markers, type);
+ DocumentMarkerList* list = ListForType(markers, type);
yosin_UTC9 2017/04/25 08:20:58 nit: s/DocumentMarkerList*/DocumentMarkerList* con
if (!list || list->IsEmpty()) {
- if (list.Get() && list->IsEmpty())
- list.Clear();
+ if (list && list->IsEmpty())
+ ListForType(markers, type) = nullptr;
++empty_lists_count;
continue;
}
if (!marker_types.Contains(type))
continue;
- if (DocumentMarkerListEditor::RemoveMarkers(list, start_offset, length))
+ if (list->RemoveMarkers(start_offset, length))
doc_dirty = true;
if (list->IsEmpty()) {
- list.Clear();
+ ListForType(markers, type) = nullptr;
++empty_lists_count;
}
}
@@ -322,13 +329,11 @@ DocumentMarkerVector DocumentMarkerController::MarkersFor(
return result;
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- Member<MarkerList>& list = ListForType(markers, type);
- if (!list || list->IsEmpty() ||
- !marker_types.Contains((*list->begin())->GetType()))
+ DocumentMarkerList* list = ListForType(markers, type);
yosin_UTC9 2017/04/25 08:20:58 nit: s/DocumentMarkerList*/DocumentMarkerList* con
+ if (!list || list->IsEmpty() || !marker_types.Contains(type))
continue;
- for (size_t i = 0; i < list->size(); ++i)
- result.push_back(list->at(i).Get());
+ result.AppendVector(list->GetMarkers());
yosin_UTC9 2017/04/25 08:20:58 Wow, AppendVector() is easy.
rlanday 2017/04/25 19:29:52 Except that as far as I'm aware, we haven't solved
}
std::sort(result.begin(), result.end(),
@@ -344,9 +349,10 @@ DocumentMarkerVector DocumentMarkerController::Markers() {
for (MarkerMap::iterator i = markers_.begin(); i != markers_.end(); ++i) {
MarkerLists* markers = i->value.Get();
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- Member<MarkerList>& list = ListForType(markers, type);
- for (size_t j = 0; list.Get() && j < list->size(); ++j)
- result.push_back(list->at(j).Get());
+ DocumentMarkerList* list = ListForType(markers, type);
+ if (!list)
+ continue;
+ result.AppendVector(list->GetMarkers());
}
}
std::sort(result.begin(), result.end(),
@@ -406,16 +412,15 @@ Vector<IntRect> DocumentMarkerController::RenderedRectsForMarkers(
continue;
MarkerLists* markers = node_iterator->value.Get();
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- Member<MarkerList>& list = ListForType(markers, type);
+ DocumentMarkerList* list = ListForType(markers, type);
yosin_UTC9 2017/04/25 08:20:59 nit: s/DocumentMarkerList*/DocumentMarkerList* con
if (!list || list->IsEmpty() || type != marker_type)
continue;
- for (unsigned marker_index = 0; marker_index < list->size();
- ++marker_index) {
- RenderedDocumentMarker* marker = list->at(marker_index).Get();
- UpdateMarkerRenderedRectIfNeeded(node, *marker);
- if (!marker->IsRendered())
+
+ for (RenderedDocumentMarker* rendered_marker : list->GetMarkers()) {
+ UpdateMarkerRenderedRectIfNeeded(node, *rendered_marker);
+ if (!rendered_marker->IsRendered())
continue;
- result.push_back(marker->RenderedRect());
+ result.push_back(rendered_marker->RenderedRect());
}
}
}
@@ -445,10 +450,12 @@ void DocumentMarkerController::InvalidateRectsForMarkersInNode(
if (!marker_list || marker_list->IsEmpty())
continue;
- for (auto& marker : *marker_list)
+ const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+ marker_list->GetMarkers();
+ for (auto& marker : markers_in_list)
marker->Invalidate();
- if (marker_list->front()->GetType() == DocumentMarker::kTextMatch)
+ if (markers_in_list.front()->GetType() == DocumentMarker::kTextMatch)
InvalidatePaintForTickmarks(node);
}
}
@@ -460,10 +467,12 @@ void DocumentMarkerController::InvalidateRectsForAllMarkers() {
if (!marker_list || marker_list->IsEmpty())
continue;
- for (auto& marker : *marker_list)
- marker->Invalidate();
+ const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+ marker_list->GetMarkers();
+ for (DocumentMarker* marker : markers_in_list)
+ ToRenderedDocumentMarker(marker)->Invalidate();
- if (marker_list->front()->GetType() == DocumentMarker::kTextMatch)
+ if (markers_in_list.front()->GetType() == DocumentMarker::kTextMatch)
InvalidatePaintForTickmarks(node);
}
}
@@ -495,11 +504,11 @@ void DocumentMarkerController::RemoveSpellingMarkersUnderWords(
continue;
MarkerLists* markers = node_markers.value;
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- MarkerList* list = ListForType(markers, type);
+ DocumentMarkerList* list = ListForType(markers, type);
yosin_UTC9 2017/04/25 08:20:58 nit: s/DocumentMarkerList*/DocumentMarkerList* con
if (!list)
continue;
- bool removed_markers = DocumentMarkerListEditor::RemoveMarkersUnderWords(
- list, ToText(node).data(), words);
+ bool removed_markers =
yosin_UTC9 2017/04/25 08:20:58 nit: s/bool/const bool/
+ list->RemoveMarkersUnderWords(ToText(node).data(), words);
if (removed_markers && type == DocumentMarker::kTextMatch)
InvalidatePaintForTickmarks(node);
}
@@ -538,16 +547,16 @@ void DocumentMarkerController::RemoveMarkersFromList(
MarkerLists* markers = iterator->value.Get();
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- Member<MarkerList>& list = ListForType(markers, type);
+ DocumentMarkerList* list = ListForType(markers, type);
yosin_UTC9 2017/04/25 08:20:59 nit: s/DocumentMarkerList*/DocumentMarkerList* con
if (!list || list->IsEmpty()) {
- if (list.Get() && list->IsEmpty())
- list.Clear();
+ if (list && list->IsEmpty())
+ ListForType(markers, type) = nullptr;
++empty_lists_count;
continue;
}
if (marker_types.Contains(type)) {
- list->clear();
- list.Clear();
+ list->Clear();
+ ListForType(markers, type) = nullptr;
++empty_lists_count;
needs_repainting = true;
}
@@ -587,7 +596,7 @@ void DocumentMarkerController::RepaintMarkers(
// inner loop: process each marker in the current node
MarkerLists* markers = i->value.Get();
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- Member<MarkerList>& list = ListForType(markers, type);
+ DocumentMarkerList* list = ListForType(markers, type);
yosin_UTC9 2017/04/25 08:20:58 nit: s/DocumentMarkerList*/DocumentMarkerList* con
if (!list || list->IsEmpty() || !marker_types.Contains(type))
continue;
@@ -636,16 +645,22 @@ bool DocumentMarkerController::SetMarkersActive(Node* node,
return false;
bool doc_dirty = false;
- Member<MarkerList>& list = ListForType(markers, DocumentMarker::kTextMatch);
+ DocumentMarkerList* list = ListForType(markers, DocumentMarker::kTextMatch);
yosin_UTC9 2017/04/25 08:20:59 nit: s/DocumentMarkerList*/DocumentMarkerList* con
+
if (!list)
return false;
- MarkerList::iterator start_pos = std::upper_bound(
- list->begin(), list->end(), start_offset,
- [](size_t start_offset, const Member<RenderedDocumentMarker>& marker) {
+
+ const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+ list->GetMarkers();
+ // TODO(rlanday): this assumes that the markers are stored in sorted order.
+ // This method should probably eventually be implemented by a
+ // TextMatch-specific marker list
+ const auto start_pos = std::upper_bound(
+ markers_in_list.begin(), markers_in_list.end(), start_offset,
+ [](size_t start_offset, const Member<DocumentMarker>& marker) {
return start_offset < marker->EndOffset();
});
- for (MarkerList::iterator marker = start_pos; marker != list->end();
- ++marker) {
+ for (auto marker = start_pos; marker != markers_in_list.end(); ++marker) {
// Markers are returned in order, so stop if we are now past the specified
// range.
if ((*marker)->StartOffset() >= end_offset)
@@ -673,10 +688,10 @@ void DocumentMarkerController::ShowMarkers() const {
builder.Append(String::Format("%p", node));
MarkerLists* markers = markers_.at(node);
for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
- Member<MarkerList>& list = ListForType(markers, type);
- for (unsigned marker_index = 0; list.Get() && marker_index < list->size();
- ++marker_index) {
- DocumentMarker* marker = list->at(marker_index).Get();
+ DocumentMarkerList* list = ListForType(markers, type);
+ const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+ list->GetMarkers();
+ for (const DocumentMarker* marker : markers_in_list) {
builder.Append(" ");
builder.AppendNumber(marker->GetType());
builder.Append(":[");
@@ -709,12 +724,11 @@ void DocumentMarkerController::DidUpdateCharacterData(CharacterData* node,
return;
bool did_shift_marker = false;
- for (MarkerList* list : *markers) {
+ for (DocumentMarkerList* list : *markers) {
if (!list)
continue;
- if (DocumentMarkerListEditor::ShiftMarkers(list, offset, old_length,
- new_length))
+ if (list->ShiftMarkers(offset, old_length, new_length))
did_shift_marker = true;
}
« no previous file with comments | « third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h ('k') | third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h » ('j') | third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698