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

Issue 2773883003: Add CompositionMarkerList in preparation for DocumentMarkerController refactor (Closed)

Created:
3 years, 9 months ago by rlanday
Modified:
3 years, 8 months ago
Reviewers:
*yosin_UTC9, Xiaocheng
CC:
blink-reviews, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add CompositionMarkerList in preparation for DocumentMarkerController refactor This CL depends on https://codereview.chromium.org/2772423002 (add EditingMarkerList) BUG=707867

Patch Set 1 #

Patch Set 2 : Fix code that doesn't compile...what am I doing... #

Total comments: 24

Patch Set 3 : Respond to comments #

Total comments: 6

Patch Set 4 : Make requested changes, refactor tests #

Total comments: 9

Patch Set 5 : Attempt to fix weird compilation error on android_arm64 #

Patch Set 6 : Make requested changes (before splitting off DocumentMarkerList) #

Patch Set 7 : Split off DocumentMarkerList and EditingMarkerList into separate CLs #

Patch Set 8 : Rebase #

Total comments: 1

Patch Set 9 : Rebase (leaving allowedMarkerType()) as-is #

Patch Set 10 : Rebase #

Patch Set 11 : Make allowedMarkerType() private #

Patch Set 12 : Rebase #

Patch Set 13 : Rebase #

Patch Set 14 : Rebase #

Patch Set 15 : Rebase #

Patch Set 16 : Rebase #

Patch Set 17 : Remove "explicit" #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -0 lines) Patch
M third_party/WebKit/Source/core/editing/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +2 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/editing/markers/CompositionMarkerList.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +23 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/editing/markers/CompositionMarkerList.cpp View 1 2 3 1 chunk +15 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 66 (51 generated)
rlanday
3 years, 9 months ago (2017-03-24 02:40:18 UTC) #5
rlanday
The questions around when to sort the marker list still need to be resolved; right ...
3 years, 9 months ago (2017-03-24 02:42:20 UTC) #6
rlanday
I got halfway through modifying DocumentMarkerList::copyMarkers() to optimize the copying in the case where the ...
3 years, 9 months ago (2017-03-24 03:03:30 UTC) #11
rlanday
https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/EditingMarkerListTest.cpp File third_party/WebKit/Source/core/editing/markers/EditingMarkerListTest.cpp (right): https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/EditingMarkerListTest.cpp#newcode382 third_party/WebKit/Source/core/editing/markers/EditingMarkerListTest.cpp:382: // delete middle of marker If we have a ...
3 years, 9 months ago (2017-03-24 03:14:31 UTC) #12
Xiaocheng
Haven't checked the unit tests. Will review them tomorrow. https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h File third_party/WebKit/Source/core/editing/markers/DocumentMarker.h (right): https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h#newcode165 third_party/WebKit/Source/core/editing/markers/DocumentMarker.h:165: ...
3 years, 9 months ago (2017-03-24 03:57:47 UTC) #13
rlanday
https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h File third_party/WebKit/Source/core/editing/markers/DocumentMarker.h (right): https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h#newcode165 third_party/WebKit/Source/core/editing/markers/DocumentMarker.h:165: // Other classes should not call following setters. On ...
3 years, 9 months ago (2017-03-24 17:45:49 UTC) #16
rlanday
https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h File third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h (right): https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h#newcode37 third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h:37: void sortMarkerList(); On 2017/03/24 at 17:45:49, rlanday wrote: > ...
3 years, 9 months ago (2017-03-24 18:01:24 UTC) #17
rlanday
Updated
3 years, 9 months ago (2017-03-24 18:16:38 UTC) #19
Xiaocheng
https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h File third_party/WebKit/Source/core/editing/markers/DocumentMarker.h (right): https://codereview.chromium.org/2773883003/diff/20001/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h#newcode175 third_party/WebKit/Source/core/editing/markers/DocumentMarker.h:175: // Offset modifications are done by DocumentMarkerController. On 2017/03/24 ...
3 years, 9 months ago (2017-03-24 19:43:50 UTC) #21
rlanday
Ok, I'll make those changes. I'm also going to modify the test cases so the ...
3 years, 9 months ago (2017-03-24 20:23:40 UTC) #22
rlanday
Updated
3 years, 9 months ago (2017-03-24 22:32:09 UTC) #26
Xiaocheng
Finally I went through all the unit tests... Btw, the base class DocumentMarkerList itself contains ...
3 years, 9 months ago (2017-03-25 00:05:36 UTC) #30
rlanday
https://codereview.chromium.org/2773883003/diff/60001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h File third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h (right): https://codereview.chromium.org/2773883003/diff/60001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h#newcode28 third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h:28: virtual void push_back(DocumentMarker*) = 0; On 2017/03/25 at 00:05:35, ...
3 years, 8 months ago (2017-03-27 19:51:30 UTC) #35
Xiaocheng
lgtm https://codereview.chromium.org/2773883003/diff/140001/third_party/WebKit/Source/core/editing/markers/CompositionMarkerList.h File third_party/WebKit/Source/core/editing/markers/CompositionMarkerList.h (right): https://codereview.chromium.org/2773883003/diff/140001/third_party/WebKit/Source/core/editing/markers/CompositionMarkerList.h#newcode16 third_party/WebKit/Source/core/editing/markers/CompositionMarkerList.h:16: DocumentMarker::MarkerType allowedMarkerType() const final; nit: move it to ...
3 years, 8 months ago (2017-03-27 22:40:01 UTC) #41
yosin_UTC9
3 years, 8 months ago (2017-03-30 01:25:39 UTC) #48
lgtm

Powered by Google App Engine
This is Rietveld 408576698