New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Fixed
Owner:
Closed: Jul 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 672259



Sign in to add a comment
Refactor DocumentMarkerController
Project Member Reported by rlanday@chromium.org, Apr 3 2017 Back to list
Original design doc:
https://goo.gl/VVJlDz

Revised design doc:
https://goo.gl/PCxjw2
 
Blocking: 672259
Components: -Blink
Description: Show this description
Project Member Comment 3 by bugdroid1@chromium.org, Apr 5 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed

commit 2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed
Author: rlanday <rlanday@chromium.org>
Date: Wed Apr 05 20:05:35 2017

Rename DocumentMarker::activeMatch() to IsActiveMatch()

What: Rename DocumentMarker::activeMatch() to IsActiveMatch()
Why: Adding "Is" makes it very clear that this is an accessor returning a boolean value, and aligns with other boolean predicates, e.g. isFocused() etc.
Outcome: Improve readability and code health

Suggested by yosin: https://codereview.chromium.org/2801483002#msg20

BUG=707867
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2789373003
Cr-Commit-Position: refs/heads/master@{#462190}

[modify] https://crrev.com/2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
[modify] https://crrev.com/2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
[modify] https://crrev.com/2cfc514b3ee3bc1c29d7e853421fe96fb7a9d7ed/third_party/WebKit/Source/core/testing/Internals.cpp

Project Member Comment 4 by bugdroid1@chromium.org, Apr 5 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81e0a769c432ef75f388feca117cd6fac2a516a4

commit 81e0a769c432ef75f388feca117cd6fac2a516a4
Author: rlanday <rlanday@chromium.org>
Date: Wed Apr 05 20:20:57 2017

Rename DocumentMarker::setActiveMatch() to setIsActiveMatch()

Suggested by @yosin here:
https://codereview.chromium.org/2801483002#msg20

What: Rename DM::setActiveMatch() to setIsActiveMatch()
Why: Align with the rename of accessor method activeMatch() to IsActiveMatch() in https://codereview.chromium.org/2789373003
Outcome: Improve readability and code health

BUG=707867

Review-Url: https://codereview.chromium.org/2802533002
Cr-Commit-Position: refs/heads/master@{#462198}

[modify] https://crrev.com/81e0a769c432ef75f388feca117cd6fac2a516a4/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/81e0a769c432ef75f388feca117cd6fac2a516a4/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/81e0a769c432ef75f388feca117cd6fac2a516a4/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Project Member Comment 5 by bugdroid1@chromium.org, Apr 6 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c198feadcc54ea92134df2dc3a5f4f390fcd9493

commit c198feadcc54ea92134df2dc3a5f4f390fcd9493
Author: rlanday <rlanday@chromium.org>
Date: Thu Apr 06 01:00:27 2017

Add DocumentMarker::MatchStatus enum for text match markers

Suggested by @yosin here:
https://codereview.chromium.org/2801483002#msg20

Modify the DocumentMarker() constructor for text match markers and DocumentMarkerController::addTextMatchMarker() to take an enum for whether or not the text match is active instead of a bool. This will make it more clear in call sites what the parameter means.

BUG=707867

Review-Url: https://codereview.chromium.org/2801483002
Cr-Commit-Position: refs/heads/master@{#462303}

[modify] https://crrev.com/c198feadcc54ea92134df2dc3a5f4f390fcd9493/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/c198feadcc54ea92134df2dc3a5f4f390fcd9493/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/c198feadcc54ea92134df2dc3a5f4f390fcd9493/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/c198feadcc54ea92134df2dc3a5f4f390fcd9493/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/c198feadcc54ea92134df2dc3a5f4f390fcd9493/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/c198feadcc54ea92134df2dc3a5f4f390fcd9493/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/c198feadcc54ea92134df2dc3a5f4f390fcd9493/third_party/WebKit/Source/web/TextFinder.cpp

Project Member Comment 6 by bugdroid1@chromium.org, Apr 6 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ca300989422ff59471397342e4789fb659290cf

commit 3ca300989422ff59471397342e4789fb659290cf
Author: rlanday <rlanday@chromium.org>
Date: Thu Apr 06 01:54:11 2017

Update window.internals.addTextMatchMarker() to take enum instead of bool

Suggested by @yosin here:
https://codereview.chromium.org/2801483002#msg20

We are changing DocumentMarkerController::addTextMatchMarker() to take an enum in https://codereview.chromium.org/2801483002 since it's unclear from looking at a callsite what "true" and "false" mean. This CL handles the window.internals changes separately since a bunch of tests have to be updated.

BUG=707867

Review-Url: https://codereview.chromium.org/2802543002
Cr-Commit-Position: refs/heads/master@{#462319}

[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/fast/dom/documentmarker-add-adjacent-text.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/fast/dom/documentmarker-set-active.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-hittest.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-styled-after-onload.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-styled.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/paint/invalidation/compositing/text-match-highlight.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/paint/invalidation/text-match-document-change.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/paint/invalidation/text-match-pre-wrapped-text.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/paint/invalidation/text-match-svg.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/paint/invalidation/text-match-transparent-text.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/paint/invalidation/text-match.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/paint/text/text-match-highlights-big-line-height.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/LayoutTests/svg/custom/text-match-highlight.html
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/3ca300989422ff59471397342e4789fb659290cf/third_party/WebKit/Source/core/testing/Internals.idl

Project Member Comment 7 by bugdroid1@chromium.org, Apr 11 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/474939fc1d9344c7fdbbb12638d4d318e9c57192

commit 474939fc1d9344c7fdbbb12638d4d318e9c57192
Author: rlanday <rlanday@chromium.org>
Date: Tue Apr 11 00:10:22 2017

Add SplitTextNodeCommandTest

I am planning to modify the behavior of DocumentMarkerController::copyMarkers()
(to move the markers to the new list instead of leaving pointers to the same
markers, which have possibly been modified, in the source list) and decided to
write a test to make sure we don't break SplitTextNodeCommand.

BUG=707867

Review-Url: https://codereview.chromium.org/2802993007
Cr-Commit-Position: refs/heads/master@{#463459}

[modify] https://crrev.com/474939fc1d9344c7fdbbb12638d4d318e9c57192/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/474939fc1d9344c7fdbbb12638d4d318e9c57192/third_party/WebKit/Source/core/editing/commands/SplitTextNodeCommand.h
[add] https://crrev.com/474939fc1d9344c7fdbbb12638d4d318e9c57192/third_party/WebKit/Source/core/editing/commands/SplitTextNodeCommandTest.cpp

Project Member Comment 8 by bugdroid1@chromium.org, Apr 11 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0dea0097ea24907ed4c0f30fb4180191f378f00e

commit 0dea0097ea24907ed4c0f30fb4180191f378f00e
Author: rlanday <rlanday@chromium.org>
Date: Tue Apr 11 02:58:57 2017

Clean up DocumentMarkerController::copyMarkers() (now it's moveMarkers())

This method is kind of weird since it leaves the markers in the source list but
sometimes changes one of them (if it spans a node boundary). In this CL, I'm
cleaning it up so it removes the markers from the source list after adding them
to the destination list.

BUG=707867

Review-Url: https://codereview.chromium.org/2810463002
Cr-Commit-Position: refs/heads/master@{#463509}

[modify] https://crrev.com/0dea0097ea24907ed4c0f30fb4180191f378f00e/third_party/WebKit/Source/core/editing/commands/SplitTextNodeCommand.cpp
[modify] https://crrev.com/0dea0097ea24907ed4c0f30fb4180191f378f00e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/0dea0097ea24907ed4c0f30fb4180191f378f00e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 9 by bugdroid1@chromium.org, Apr 12 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e71603c48cc0898a933e13d266dbf036d034c42

commit 5e71603c48cc0898a933e13d266dbf036d034c42
Author: rlanday <rlanday@chromium.org>
Date: Wed Apr 12 02:14:46 2017

Add some tests for DocumentMarkerController::removeMarkers()

I'm changing the behavior of removeMarkers() in https://codereview.chromium.org/2806683002 to always remove markers rather than splitting and noticed that the current behavior doesn't seem to be covered by any tests; I'm adding tests for the current behavior to make it clear in that CL how the behavior is being changed.

BUG=707867

Review-Url: https://codereview.chromium.org/2812723002
Cr-Commit-Position: refs/heads/master@{#463899}

[modify] https://crrev.com/5e71603c48cc0898a933e13d266dbf036d034c42/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp

Project Member Comment 10 by bugdroid1@chromium.org, Apr 13 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98f1927737cd1c3fd5d471b74cc94333e9e37972

commit 98f1927737cd1c3fd5d471b74cc94333e9e37972
Author: rlanday <rlanday@chromium.org>
Date: Thu Apr 13 20:22:26 2017

Don't ever split DocumentMarkers on remove

Right now some of the DocumentMarkerController::removeMarkers() methods take an
enum flag, RemovePartiallyOverlappingMarkerOrNot that controls what the method
does with markers that partially intersect the passed-in range but do not fall
entirely within it. If the value DoNotRemovePartiallyOverlappingMarker is
passed, the section in the passed-in range is removed and the remaining piece(s)
of the marker are left in place (otherwise the entire marker is removed). If the
passed-in range contains part of the interior of the marker but neither the
start nor end position, this means the marker is split into two pieces, which
requires copying the marker. yosin thinks copying markers is weird and we should
try to avoid ever having to do it. I believe this is possible if we eliminate
this flag and just never split markers on removal.

I don't think any of the currently-existing marker types (Spelling, Grammar,
Composition, TextMatch) ever make sense to split, and neither do the new types
I'm going to add (Suggestion, SuggestionHighlight). The only test cases I had to
update after this change are ones I added based on the current behavior.

BUG=707867

Review-Url: https://codereview.chromium.org/2806683002
Cr-Commit-Position: refs/heads/master@{#464524}

[modify] https://crrev.com/98f1927737cd1c3fd5d471b74cc94333e9e37972/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/98f1927737cd1c3fd5d471b74cc94333e9e37972/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/98f1927737cd1c3fd5d471b74cc94333e9e37972/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/98f1927737cd1c3fd5d471b74cc94333e9e37972/third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp
[modify] https://crrev.com/98f1927737cd1c3fd5d471b74cc94333e9e37972/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp

Project Member Comment 11 by bugdroid1@chromium.org, Apr 17 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73e6a569acfd3b3b5ecc4f0ec2c0945740e9cd2c

commit 73e6a569acfd3b3b5ecc4f0ec2c0945740e9cd2c
Author: rlanday <rlanday@chromium.org>
Date: Mon Apr 17 08:52:08 2017

Remove unused DocumentMarker comparison operators

It was pointed out in a code review that these are unused:
https://codereview.chromium.org/2780313002/diff/20001/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h#newcode166

BUG=707867

Review-Url: https://codereview.chromium.org/2785023002
Cr-Commit-Position: refs/heads/master@{#464908}

[modify] https://crrev.com/73e6a569acfd3b3b5ecc4f0ec2c0945740e9cd2c/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h

Project Member Comment 12 by bugdroid1@chromium.org, Apr 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b59bf346862b89daee2a4f06f02f5d7df59f9164

commit b59bf346862b89daee2a4f06f02f5d7df59f9164
Author: rlanday <rlanday@chromium.org>
Date: Tue Apr 18 01:50:38 2017

Refactor DocumentMarkerController on top of new DocumentMarkerListEditor class

Based on yosin's comment on a previous version of this CL:
https://codereview.chromium.org/2812423002#msg7

This CL is step 1, "Introduce DMListEditor with in-place changes of DMC: Except for DMListEditor.h, changes are small". In this CL, I'm pulling out some of the
DocumentMarkerController implementation code into a DocumentMarkerListEditor class.
In a future CL, I will move this class into its own .h/.cpp files (I'm leaving it
in place for now to make the patch smaller).

BUG=707867

Review-Url: https://codereview.chromium.org/2812423002
Cr-Commit-Position: refs/heads/master@{#465092}

[modify] https://crrev.com/b59bf346862b89daee2a4f06f02f5d7df59f9164/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/b59bf346862b89daee2a4f06f02f5d7df59f9164/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 13 by bugdroid1@chromium.org, Apr 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a92abb9ad049f637596371997cc125f182ae3cc3

commit a92abb9ad049f637596371997cc125f182ae3cc3
Author: rlanday <rlanday@chromium.org>
Date: Tue Apr 18 03:19:14 2017

Refactor DocumentMarkerController::MoveMarkers()

As we (yosin, xiaochengh, and I) discussed, I'm splitting the implementation
changes for this method I previously was including in
https://codereview.chromium.org/2812423002 into a separate CL.

We want to separate out the implementation changes we need to pull out the main
part of this method into a method on DocumentMarkerListEditor, and actually
adding that method.

This CL needs the AddMarker() method I'm adding in that CL, so I've removed the
MoveMarkers() changes entirely from that one and will upload a third CL that
adds DocumentMarkerListEditor::MoveMarkers().

BUG=707867

Review-Url: https://codereview.chromium.org/2819173002
Cr-Commit-Position: refs/heads/master@{#465124}

[modify] https://crrev.com/a92abb9ad049f637596371997cc125f182ae3cc3/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Project Member Comment 14 by bugdroid1@chromium.org, Apr 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5a0bdbe8b745676efac4996f50c0558bb17b839

commit b5a0bdbe8b745676efac4996f50c0558bb17b839
Author: rlanday <rlanday@chromium.org>
Date: Tue Apr 18 03:38:37 2017

Add DocumentMarkerController::ListForType()

Helper method that makes it slightly cleaner to look up the MarkerList* for a
given type when we're holding the MarkerLists* for a Node. Alternatively, we
could have a method that takes a Node* instead of a MarkerLists*, but then we'd
lose the optimization in the call sites where we can return early if there's no
MarkerLists* for the given Node.

I found two places to use this in the current version of the code, and there
will be more after we rewrite some of the loops to use the DocumentMarkerList
interface being added in https://codereview.chromium.org/2820633002 .

BUG=707867

Review-Url: https://codereview.chromium.org/2822963002
Cr-Commit-Position: refs/heads/master@{#465127}

[modify] https://crrev.com/b5a0bdbe8b745676efac4996f50c0558bb17b839/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/b5a0bdbe8b745676efac4996f50c0558bb17b839/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 15 by bugdroid1@chromium.org, Apr 20 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/954977d31f907b814ca4aef51bbd76fb9a55f1f5

commit 954977d31f907b814ca4aef51bbd76fb9a55f1f5
Author: rlanday <rlanday@chromium.org>
Date: Thu Apr 20 03:56:13 2017

Add DocumentMarkerListEditor::MoveMarkers()

Follow-up to https://codereview.chromium.org/2812423002/ (add
DocumentMarkerListEditor class) and https://codereview.chromium.org/2819173002
(refactor DocumentMarkerController::MoveMarkers()): this CL moves some of the
DocumentMarkerController::MoveMarkers() code into a new
DocumentMarkerListEditor::MoveMarkers() method

BUG=707867

Review-Url: https://codereview.chromium.org/2820983002
Cr-Commit-Position: refs/heads/master@{#465894}

[modify] https://crrev.com/954977d31f907b814ca4aef51bbd76fb9a55f1f5/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/954977d31f907b814ca4aef51bbd76fb9a55f1f5/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 16 by bugdroid1@chromium.org, Apr 20 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74ad4fde9304fc6f71a165462360662c5bfe7924

commit 74ad4fde9304fc6f71a165462360662c5bfe7924
Author: rlanday <rlanday@chromium.org>
Date: Thu Apr 20 09:48:32 2017

Convert static comparator functions in DocumentMarkerController to lambdas

After https://codereview.chromium.org/2818873002, we will need to use
EndsBefore() in more than one file. yosin advised that these functions are
small enough that they should be lambdas rather than trying to add them e.g. as
member functions on DocumentMarker to avoid duplication.

BUG=707867

Review-Url: https://codereview.chromium.org/2828093004
Cr-Commit-Position: refs/heads/master@{#465960}

[modify] https://crrev.com/74ad4fde9304fc6f71a165462360662c5bfe7924/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Project Member Comment 17 by bugdroid1@chromium.org, Apr 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37c5a8829c124800330ab1965fa29615465551e9

commit 37c5a8829c124800330ab1965fa29615465551e9
Author: rlanday <rlanday@chromium.org>
Date: Fri Apr 21 02:00:45 2017

Move DocumentMarkerListEditor to its own .h/.cpp files

Based on Step 2 of yosin's plan:
https://codereview.chromium.org/2812423002#msg7

This is a follow-up to this CL where the DocumentMarkerListEditor class
was created inside DocumentMarkerController:
https://codereview.chromium.org/2812423002

BUG=707867

Review-Url: https://codereview.chromium.org/2818873002
Cr-Commit-Position: refs/heads/master@{#466224}

[modify] https://crrev.com/37c5a8829c124800330ab1965fa29615465551e9/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/37c5a8829c124800330ab1965fa29615465551e9/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/37c5a8829c124800330ab1965fa29615465551e9/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[add] https://crrev.com/37c5a8829c124800330ab1965fa29615465551e9/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[add] https://crrev.com/37c5a8829c124800330ab1965fa29615465551e9/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h

Project Member Comment 18 by bugdroid1@chromium.org, Apr 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d9f45d2abbc726557c6f00ba76e6128c478416e

commit 2d9f45d2abbc726557c6f00ba76e6128c478416e
Author: rlanday <rlanday@chromium.org>
Date: Fri Apr 21 08:48:32 2017

Rewrite loops in DocumentMarkerController to use ListForType()

xiaochengh pointed out in another code review that this method is not being used
consistently:
https://codereview.chromium.org/2820633002#msg26

After this CL, it will be used consistently.

BUG=707867

Review-Url: https://codereview.chromium.org/2830033002
Cr-Commit-Position: refs/heads/master@{#466290}

[modify] https://crrev.com/2d9f45d2abbc726557c6f00ba76e6128c478416e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Project Member Comment 19 by bugdroid1@chromium.org, Apr 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27f9eb47ba02349a8b5e850c597d4778b6c38846

commit 27f9eb47ba02349a8b5e850c597d4778b6c38846
Author: rlanday <rlanday@chromium.org>
Date: Fri Apr 21 10:22:10 2017

Rename DocumentMarkerController::RemoveMarkers() to RemoveMarkersInRange()

yosin pointed out that it's confusing to have so many methods named
RemoveMarkers():
https://codereview.chromium.org/2820633002#msg30

This is the first of several renaming CLs I'm going to upload

BUG=707867

Review-Url: https://codereview.chromium.org/2830213002
Cr-Commit-Position: refs/heads/master@{#466302}

[modify] https://crrev.com/27f9eb47ba02349a8b5e850c597d4778b6c38846/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/27f9eb47ba02349a8b5e850c597d4778b6c38846/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/27f9eb47ba02349a8b5e850c597d4778b6c38846/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/27f9eb47ba02349a8b5e850c597d4778b6c38846/third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp
[modify] https://crrev.com/27f9eb47ba02349a8b5e850c597d4778b6c38846/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.cpp
[modify] https://crrev.com/27f9eb47ba02349a8b5e850c597d4778b6c38846/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp

Project Member Comment 20 by bugdroid1@chromium.org, Apr 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c0f93a9a83297a1046670164881e6bd0ce2b66d6

commit c0f93a9a83297a1046670164881e6bd0ce2b66d6
Author: rlanday <rlanday@chromium.org>
Date: Fri Apr 21 10:58:19 2017

Rename DocumentMarkerController::RemoveMarkers() to RemoveMarkersInternal()

yosin pointed out that it's confusing to have so many methods named
RemoveMarkers():
https://codereview.chromium.org/2820633002#msg30

This is the second of several renaming CLs I'm going to upload

BUG=707867

Review-Url: https://codereview.chromium.org/2829723006
Cr-Commit-Position: refs/heads/master@{#466309}

[modify] https://crrev.com/c0f93a9a83297a1046670164881e6bd0ce2b66d6/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/c0f93a9a83297a1046670164881e6bd0ce2b66d6/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 21 by bugdroid1@chromium.org, Apr 22 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad6b1f378b7706541d97b227638ba93c446fc8f1

commit ad6b1f378b7706541d97b227638ba93c446fc8f1
Author: rlanday <rlanday@chromium.org>
Date: Sat Apr 22 00:57:29 2017

Refactor DocumentMarkerController::RemoveMarkers(const MarkerRemoverPredicate&)

MarkerRemoverPredicate seems to have been added with some amount of generality
in mind, but the way it's written it only serves a single purpose, so I'm
refactoring the RemoveMarkers() method that takes it to just take a vector of
words instead.

BUG=707867

Review-Url: https://codereview.chromium.org/2833753004
Cr-Commit-Position: refs/heads/master@{#466516}

[modify] https://crrev.com/ad6b1f378b7706541d97b227638ba93c446fc8f1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/ad6b1f378b7706541d97b227638ba93c446fc8f1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/ad6b1f378b7706541d97b227638ba93c446fc8f1/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp

Project Member Comment 22 by bugdroid1@chromium.org, Apr 22 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bda3d4871655a1cb5121cde488845d9b968eda6c

commit bda3d4871655a1cb5121cde488845d9b968eda6c
Author: rlanday <rlanday@chromium.org>
Date: Sat Apr 22 03:39:11 2017

Rename DocumentMarkerController::RemoveMarkers() to RemoveMarkersForNode()
yosin pointed out that it's confusing to have so many methods named
RemoveMarkers():
https://codereview.chromium.org/2820633002#msg30

This is the fourth of several renaming CLs I'm going to upload

BUG=707867

Review-Url: https://codereview.chromium.org/2830233002
Cr-Commit-Position: refs/heads/master@{#466534}

[modify] https://crrev.com/bda3d4871655a1cb5121cde488845d9b968eda6c/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/bda3d4871655a1cb5121cde488845d9b968eda6c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/bda3d4871655a1cb5121cde488845d9b968eda6c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/bda3d4871655a1cb5121cde488845d9b968eda6c/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp

Project Member Comment 23 by bugdroid1@chromium.org, Apr 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1945a87fa4e60d4749fc02db1a939e21c3470e63

commit 1945a87fa4e60d4749fc02db1a939e21c3470e63
Author: rlanday <rlanday@chromium.org>
Date: Mon Apr 24 01:14:48 2017

Rename DocumentMarkerController::RemoveMarkers() to RemoveMarkersOfTypes()

yosin pointed out that it's confusing to have so many methods named
RemoveMarkers():
https://codereview.chromium.org/2820633002#msg30

This is the third of several renaming CLs I'm going to upload

BUG=707867

Review-Url: https://codereview.chromium.org/2836563002
Cr-Commit-Position: refs/heads/master@{#466575}

[modify] https://crrev.com/1945a87fa4e60d4749fc02db1a939e21c3470e63/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/1945a87fa4e60d4749fc02db1a939e21c3470e63/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/1945a87fa4e60d4749fc02db1a939e21c3470e63/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/1945a87fa4e60d4749fc02db1a939e21c3470e63/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/1945a87fa4e60d4749fc02db1a939e21c3470e63/third_party/WebKit/Source/web/SpellCheckerClientImpl.cpp
[modify] https://crrev.com/1945a87fa4e60d4749fc02db1a939e21c3470e63/third_party/WebKit/Source/web/TextFinder.cpp

Project Member Comment 24 by bugdroid1@chromium.org, Apr 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ad9f6860b4d6a4ec7f7f975b2c99672e02d5d49

commit 9ad9f6860b4d6a4ec7f7f975b2c99672e02d5d49
Author: rlanday <rlanday@chromium.org>
Date: Mon Apr 24 08:30:44 2017

Make DocumentMarkerController::AddMarker() take a pointer instead of a const ref

Right now this method can take a const reference despite DMC possibly later
needing to edit the marker because the current implementation always copies the
marker (to construct a RenderedDocumentMarker). However, this is not guaranteed
by the API, and will be changed in the future once we no longer construct
RenderedDocumentMarkers for all MarkerTypes. So this method needs to be changed
to take a non-const pointer (could use a ref but I think doing so is discouraged
by the style guide). Also, the methods that call AddMarker() need to pass
pointers to heap-allocated objects.

BUG=707867

Review-Url: https://codereview.chromium.org/2833043002
Cr-Commit-Position: refs/heads/master@{#466596}

[modify] https://crrev.com/9ad9f6860b4d6a4ec7f7f975b2c99672e02d5d49/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/9ad9f6860b4d6a4ec7f7f975b2c99672e02d5d49/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 25 by bugdroid1@chromium.org, Apr 26 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a487ccbcee73cf847f61aaf0c35b09447dfa3d73

commit a487ccbcee73cf847f61aaf0c35b09447dfa3d73
Author: rlanday <rlanday@chromium.org>
Date: Wed Apr 26 03:58:28 2017

Remove DocumentMarker::SpellCheckClientMarkers()

This is redundant with DocumentMarker::MisspellingMarkers().

Two arguments for keeping the MisspellingMarkers() name instead of this one:

- "MisspellingMarkers" is shorter than "SpellCheckClientMarkers"

- MisspellingMarkers() has nine callsites, SpellCheckClientMarkers() only has
  one

BUG=707867

Review-Url: https://codereview.chromium.org/2836393002
Cr-Commit-Position: refs/heads/master@{#467226}

[modify] https://crrev.com/a487ccbcee73cf847f61aaf0c35b09447dfa3d73/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/a487ccbcee73cf847f61aaf0c35b09447dfa3d73/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.cpp

Project Member Comment 26 by bugdroid1@chromium.org, Apr 27 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c84b714f7a9730eeae4a4ccd84ac1582a6074926

commit c84b714f7a9730eeae4a4ccd84ac1582a6074926
Author: rlanday <rlanday@chromium.org>
Date: Thu Apr 27 02:00:17 2017

Add DocumentMarkerList interface and GenericDocumentMarkerListImpl

Based on step 3 of yosin's plan:
https://codereview.chromium.org/2812423002#msg7

This CL does the following:

- Introduces a DocumentMarkerList interface

- Introduces GenericDocumentMarkerListImpl, an implementation
  of DocumentMarkerList that works for all marker types and is implemented on top
  of DocumentMarkerListEditor

- Refactors DocumentMarkerController to use GenericDocumentMarkerListImpl instead
  of using DocumentMarkerListEditor directly

BUG=707867

Review-Url: https://codereview.chromium.org/2820633002
Cr-Commit-Position: refs/heads/master@{#467549}

[modify] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[add] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.cpp
[add] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[add] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.cpp
[add] https://crrev.com/c84b714f7a9730eeae4a4ccd84ac1582a6074926/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.h

Project Member Comment 27 by bugdroid1@chromium.org, Apr 27 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4dbe8b7c5c9762619c4baf47e4650a9d8139d48f

commit 4dbe8b7c5c9762619c4baf47e4650a9d8139d48f
Author: rlanday <rlanday@chromium.org>
Date: Thu Apr 27 06:50:18 2017

Prepare DocumentMarkerController for using different DocumentMarkerList impls

I'm going to start writing different implementations of DocumentMarkerList for
the different MarkerTypes. This CL prepares us to be able to easily specify the
marker list class we want to use for each MarkerType.

BUG=707867

Review-Url: https://codereview.chromium.org/2826493003
Cr-Commit-Position: refs/heads/master@{#467609}

[modify] https://crrev.com/4dbe8b7c5c9762619c4baf47e4650a9d8139d48f/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Project Member Comment 28 by bugdroid1@chromium.org, Apr 28 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db5d3c0b9ba46e665c77be59be3695df59f4e139

commit db5d3c0b9ba46e665c77be59be3695df59f4e139
Author: rlanday <rlanday@chromium.org>
Date: Fri Apr 28 04:04:42 2017

Split up DocumentMarkerListEditor::AddMarker() into two methods

This change helps us avoid duplication in the MarkerType-specific list
implementations I'm going to add. Instead of having one
DocumentMarkerListEditor::AddMarker() method that works for
GenericDocumentMarkerListImpl, but then requires each of the other list
implementations to implement the method separately, we'll have one method,
AddAndMergeOverlapping(), that works for Spelling/Grammar markers (and can be
moved into SpellCheckMarkerList once we eliminate
GenericDocumentMarkerListImpl), and another method
AddWithoutMergingOverlapping(), that can be used for the other marker types.
GenericDocumentMarkerListImpl will choose which to use based on the type of the
inserted marker.

Right now both of these methods create a RenderedDocumentMarker from the
passed-in DocumentMarker. I will move this piece out of the method once not all
marker list implementations are still creating RenderedDocumentMarkers.

BUG=707867

Review-Url: https://codereview.chromium.org/2842263002
Cr-Commit-Position: refs/heads/master@{#467884}

[modify] https://crrev.com/db5d3c0b9ba46e665c77be59be3695df59f4e139/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/db5d3c0b9ba46e665c77be59be3695df59f4e139/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[modify] https://crrev.com/db5d3c0b9ba46e665c77be59be3695df59f4e139/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.cpp

Project Member Comment 29 by bugdroid1@chromium.org, May 8 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1bdbd3e0b51de1e6327e71d27e8b7425d475e04

commit b1bdbd3e0b51de1e6327e71d27e8b7425d475e04
Author: rlanday <rlanday@chromium.org>
Date: Mon May 08 17:22:06 2017

Make DocumentMarker::AllMarkers() automatically track list of types

Every time I try to add a new DocumentMarker, I get tripped up by this list
needing to be updated. If we write it like this, it will automatically start
including the new type, without any performance penalty.

BUG=707867

Review-Url: https://codereview.chromium.org/2857483002
Cr-Commit-Position: refs/heads/master@{#470029}

[modify] https://crrev.com/b1bdbd3e0b51de1e6327e71d27e8b7425d475e04/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h

Project Member Comment 30 by bugdroid1@chromium.org, May 9 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45abde5e4bc317fc2e91152b1d66feec93673563

commit 45abde5e4bc317fc2e91152b1d66feec93673563
Author: rlanday <rlanday@chromium.org>
Date: Tue May 09 04:08:41 2017

Add CompositionMarkerListImpl

This is the first of the specialized DocumentMarkerList implementations. It is
currently quite similar to GenericDocumentMarkerListImpl, except it doesn't have
the logic for merging Spelling/Grammar markers, and it doesn't implement the
RemoveMarkersUnderWords() method on DocumentMarkerList that will eventually be
moved to SpellCheckMarkerList. Eventually we will stop creating
RenderedDocumentMarkers in non-TextMatchMarkerLists.

BUG=707867

Review-Url: https://codereview.chromium.org/2820343004
Cr-Commit-Position: refs/heads/master@{#470183}

[modify] https://crrev.com/45abde5e4bc317fc2e91152b1d66feec93673563/third_party/WebKit/Source/core/editing/BUILD.gn
[add] https://crrev.com/45abde5e4bc317fc2e91152b1d66feec93673563/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[add] https://crrev.com/45abde5e4bc317fc2e91152b1d66feec93673563/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.h
[add] https://crrev.com/45abde5e4bc317fc2e91152b1d66feec93673563/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImplTest.cpp
[modify] https://crrev.com/45abde5e4bc317fc2e91152b1d66feec93673563/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/45abde5e4bc317fc2e91152b1d66feec93673563/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/45abde5e4bc317fc2e91152b1d66feec93673563/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.cpp

Project Member Comment 31 by bugdroid1@chromium.org, May 12 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01d3e7ab39cb3238de4950601fb92c81ca4e67d7

commit 01d3e7ab39cb3238de4950601fb92c81ca4e67d7
Author: rlanday <rlanday@chromium.org>
Date: Fri May 12 05:35:48 2017

Move SpellCheckMarkerListImpl::Add() implementation from DMLEditor.cpp

Move SpellCheckMarkerListImpl::Add() from DocumentMarkerListEditor.cpp to
SpellCheckMarkerListImpl.cpp. This method implementation was left in
DocumentMarkerListEditor in https://codereview.chromium.org/2829543002
to make that patch smaller. This patch moves it to its final location.

BUG=707867

Review-Url: https://codereview.chromium.org/2851633003
Cr-Commit-Position: refs/heads/master@{#471229}

[modify] https://crrev.com/01d3e7ab39cb3238de4950601fb92c81ca4e67d7/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/01d3e7ab39cb3238de4950601fb92c81ca4e67d7/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp

Project Member Comment 32 by bugdroid1@chromium.org, May 14 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3639139a6216af6f66840f8348622d45ba06d5b9

commit 3639139a6216af6f66840f8348622d45ba06d5b9
Author: rlanday <rlanday@chromium.org>
Date: Sun May 14 23:05:13 2017

Remove DocumentMarkerController::MarkersInRange()

Currently, MarkersInRange() is kind of broken in that it can potentially return
DocumentMarkers from multiple Nodes, and it doesn't return any information about
which Node each DocumentMarker is associated with. This means that callers are
currently resorting to dubious methods to attempt to reconstruct the
EphemeralRange associated with the returned marker(s).

After discussing with xiaochengh@ and yosin@, instead of changing the method to
also return the marker's Node or EphemeralRange, we decided to remove the method
and move the logic directly into the callers.

BUG=707867

Review-Url: https://codereview.chromium.org/2857173003
Cr-Commit-Position: refs/heads/master@{#471642}

[modify] https://crrev.com/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
[modify] https://crrev.com/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/core/editing/SelectionController.cpp
[modify] https://crrev.com/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
[modify] https://crrev.com/3639139a6216af6f66840f8348622d45ba06d5b9/third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Project Member Comment 33 by bugdroid1@chromium.org, May 16 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8b7adbe4b26fedd0a87ffd74aaac141a4694490

commit d8b7adbe4b26fedd0a87ffd74aaac141a4694490
Author: rlanday <rlanday@chromium.org>
Date: Tue May 16 20:36:22 2017

Add DocumentMarkerList::MarkerType()

We need a method on DocumentMarkerList that returns the marker type the list
supports so we know when we can safely cast a DocumentMarkerList* to an impl
type.

This CL adds two subclasses of SpellCheckMarkerListImpl, SpellingMarkerListImpl,
and GrammarMarkerListImpl, so we can implement this method.

Note that adding this method also requires modifying
GenericDocumentMarkerListImpl to take a MarkerType param (we're going to
eliminate GenericDocumentMarkerListImpl once we add TextMatchMarkerListImpl but
we need this for now).

BUG=707867

Review-Url: https://codereview.chromium.org/2868413002
Cr-Commit-Position: refs/heads/master@{#472206}

[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.h
[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.cpp
[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.h
[add] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/GrammarMarkerListImpl.cpp
[add] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/GrammarMarkerListImpl.h
[add] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/GrammarMarkerListImplTest.cpp
[modify] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.h
[add] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/SpellingMarkerListImpl.cpp
[add] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/SpellingMarkerListImpl.h
[rename] https://crrev.com/d8b7adbe4b26fedd0a87ffd74aaac141a4694490/third_party/WebKit/Source/core/editing/markers/SpellingMarkerListImplTest.cpp

Project Member Comment 34 by bugdroid1@chromium.org, May 17 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afa32648e767d5288c0ea85ca18526ac8810fe0d

commit afa32648e767d5288c0ea85ca18526ac8810fe0d
Author: rlanday <rlanday@chromium.org>
Date: Wed May 17 04:27:18 2017

Add type cast for SpellCheckMarkerListImpl

This will be used to cast a DocumentMarkerList* to SpellCheckMarkerListImpl* in
DocumentMarkerController once RemoveMarkersUnderWords() is a method defined on
SpellCheckMarkerListImpl instead of DocumentMarkerList.

BUG=707867

Review-Url: https://codereview.chromium.org/2873483002
Cr-Commit-Position: refs/heads/master@{#472324}

[modify] https://crrev.com/afa32648e767d5288c0ea85ca18526ac8810fe0d/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.h

Project Member Comment 35 by bugdroid1@chromium.org, May 17 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d38becdb2a4dda654d795cb841111778673c0b1d

commit d38becdb2a4dda654d795cb841111778673c0b1d
Author: rlanday <rlanday@chromium.org>
Date: Wed May 17 09:25:47 2017

Move DocumentMarkerList::RemoveMarkersUnderWords() to SpellCheckMarkerListImpl

Currently this is a method on the general DocumentMarkerList interface. This CL
moves it to only be on SpellCheckMarkerListImpl and adds a typecast at the
callsite in DocumentMarkerController to enable this.

I am leaving the method implementation in DocumentMarkerListEditor.cpp in this CL
to make this CL smaller; I will move it to SpellCheckMarkerListImpl.cpp in a future
CL.

BUG=707867

Review-Url: https://codereview.chromium.org/2871823002
Cr-Commit-Position: refs/heads/master@{#472395}

[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.h
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.cpp
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.h
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp
[modify] https://crrev.com/d38becdb2a4dda654d795cb841111778673c0b1d/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.h

Project Member Comment 36 by bugdroid1@chromium.org, May 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a7b21235feafd7a1c7364df445ea523443882af

commit 8a7b21235feafd7a1c7364df445ea523443882af
Author: rlanday <rlanday@chromium.org>
Date: Thu May 18 05:56:23 2017

Move SpellCheckMarkerListImpl::RemoveMarkersUnderWords() impl to correct place

This method was moved from DocumentMarkerListEditor to SpellCheckMarkerListImpl
in https://codereview.chromium.org/2871823002, but the was left in
DocumentMarkerListEditor.cpp to make that CL smaller. This CL moves this method
to its final place in SpellCheckMarkerListImpl.cpp.

BUG=707867

Review-Url: https://codereview.chromium.org/2864243003
Cr-Commit-Position: refs/heads/master@{#472692}

[modify] https://crrev.com/8a7b21235feafd7a1c7364df445ea523443882af/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/8a7b21235feafd7a1c7364df445ea523443882af/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp

Project Member Comment 37 by bugdroid1@chromium.org, May 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c8452ad5808e8466b92449340e199c334e3b991

commit 2c8452ad5808e8466b92449340e199c334e3b991
Author: rlanday <rlanday@chromium.org>
Date: Thu May 18 09:58:43 2017

Add TextMatchMarkerListImpl

This is the last of the MarkerType-specific marker list implementations for now.
I'm also removing GenericDocumentMarkerListImpl in this CL since it's no longer
needed.

Right now this is very similar to CompositionMarkerList, but eventually this
will be the only marker list impl that creates RenderedDocumentMarkers, and I
will move the implementations of the text match marker-specific methods in
DocumentMarkerController (e.g. everything that has anything to do with rendered
rects) here.

BUG=707867

Review-Url: https://codereview.chromium.org/2836183002
Cr-Commit-Position: refs/heads/master@{#472757}

[modify] https://crrev.com/2c8452ad5808e8466b92449340e199c334e3b991/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/2c8452ad5808e8466b92449340e199c334e3b991/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[delete] https://crrev.com/a98edc8510b39155ec0e0efa1db29b195d487d7e/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.cpp
[delete] https://crrev.com/a98edc8510b39155ec0e0efa1db29b195d487d7e/third_party/WebKit/Source/core/editing/markers/GenericDocumentMarkerListImpl.h
[add] https://crrev.com/2c8452ad5808e8466b92449340e199c334e3b991/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[add] https://crrev.com/2c8452ad5808e8466b92449340e199c334e3b991/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h
[add] https://crrev.com/2c8452ad5808e8466b92449340e199c334e3b991/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImplTest.cpp

Project Member Comment 38 by bugdroid1@chromium.org, May 19 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97cf10ffd89f897b67f489c4b072886e186c9944

commit 97cf10ffd89f897b67f489c4b072886e186c9944
Author: rlanday <rlanday@chromium.org>
Date: Fri May 19 05:39:40 2017

Rename DMC::SetMarkersActive() to SetTextMatchMarkersActive()

Rename DocumentMarkerController::SetMarkersActive() to
SetTextMatchMarkersActive(). The current name of this method is confusing since
only TextMatch markers can be active or inactive.

I'm also fixing a PossiblyHasMarkers() check to be more specific (checking for
TextMatch markers instead of all types).

BUG=707867

Review-Url: https://codereview.chromium.org/2897493002
Cr-Commit-Position: refs/heads/master@{#473099}

[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/LayoutTests/fast/dom/documentmarker-set-active.html
[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/Source/core/testing/Internals.idl
[modify] https://crrev.com/97cf10ffd89f897b67f489c4b072886e186c9944/third_party/WebKit/Source/web/TextFinder.cpp

Project Member Comment 39 by bugdroid1@chromium.org, May 19 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34da61835c4da3f85eb33448a87a5dc530d6dc61

commit 34da61835c4da3f85eb33448a87a5dc530d6dc61
Author: rlanday <rlanday@chromium.org>
Date: Fri May 19 07:03:53 2017

Simplify DMC::InvalidateRectsForAllMarkers()

Implement DocumentMarkerController::InvalidateRectsForAllMarkers() on top of
InvalidateRectsForMarkersInNode(). This makes the code easier to understand and
simplifies maintenance.

BUG=707867

Review-Url: https://codereview.chromium.org/2896483002
Cr-Commit-Position: refs/heads/master@{#473114}

[modify] https://crrev.com/34da61835c4da3f85eb33448a87a5dc530d6dc61/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Accidentally landed this CL with the wrong bug number:

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ee9280525895b4dbb74d3e6117547f79b96d89e

commit 8ee9280525895b4dbb74d3e6117547f79b96d89e
Author: rlanday <rlanday@chromium.org>
Date: Fri May 19 21:53:51 2017

Only create RenderedDocumentMarkers for TextMatchMarkerListImpl

This sets us up to start creating a polymorphic class hierarchy for
DocumentMarker. RenderedDocumentMarker will become TextMatchMarker, and we will
also add subclasses of DocumentMarker for the other MarkerTypes as well.

BUG=715365

Review-Url: https://codereview.chromium.org/2883503004
Cr-Commit-Position: refs/heads/master@{#473345}

[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h

Project Member Comment 41 by bugdroid1@chromium.org, May 20 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/399db8861f0eb1247b1ebec062ac3fe411ff04d1

commit 399db8861f0eb1247b1ebec062ac3fe411ff04d1
Author: rlanday <rlanday@chromium.org>
Date: Sat May 20 04:35:43 2017

Rename DocumentMarkerController::RenderedRectsForMarkers()

Rename DocumentMarkerController::RenderedRectsForMarkers() to
RenderedRectsForTextMatchMarkers() and remove the marker_type param.
After https://codereview.chromium.org/2883503004, we only create
RenderedDocumentMarkers for TextMatch markers, and the type param is
no longer needed.

BUG=707867

Review-Url: https://codereview.chromium.org/2886893008
Cr-Commit-Position: refs/heads/master@{#473411}

[modify] https://crrev.com/399db8861f0eb1247b1ebec062ac3fe411ff04d1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/399db8861f0eb1247b1ebec062ac3fe411ff04d1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/399db8861f0eb1247b1ebec062ac3fe411ff04d1/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/399db8861f0eb1247b1ebec062ac3fe411ff04d1/third_party/WebKit/Source/core/frame/FrameView.cpp

Project Member Comment 42 by bugdroid1@chromium.org, May 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a55089594a4bb75655e016b48c15dc368e159fda

commit a55089594a4bb75655e016b48c15dc368e159fda
Author: rlanday <rlanday@chromium.org>
Date: Sun May 21 21:24:30 2017

Rename DocumentMarkerController::InvalidateRects*() methods

Renaming InvalidateRectsForAllMarkers() to InvalidateRectsForTextMatchMarkers()
and InvalidateRectsForMarkersInNode() to
InvalidateRectsForTextMatchMarkersInNode() to reflect that we're only creating
RenderedDocumentMarkers (which hold the rects) for TextMatch markers after
https://codereview.chromium.org/2886893008.

BUG=707867

Review-Url: https://codereview.chromium.org/2891173002
Cr-Commit-Position: refs/heads/master@{#473483}

[modify] https://crrev.com/a55089594a4bb75655e016b48c15dc368e159fda/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/a55089594a4bb75655e016b48c15dc368e159fda/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/a55089594a4bb75655e016b48c15dc368e159fda/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 43 by bugdroid1@chromium.org, May 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a590e1184af37fc63ff311976de04dda8d48eb02

commit a590e1184af37fc63ff311976de04dda8d48eb02
Author: rlanday <rlanday@chromium.org>
Date: Wed May 24 09:49:23 2017

Add type cast for TextMatchMarkerListImpl

This will be used in DocumentMarkerController to cast a DocumentMarkerList* to
TextMatchMarkerListImpl* to call some rendered rect-related methods I'm going to
add to TextMatchMarkerListImpl.

BUG=707867

Review-Url: https://codereview.chromium.org/2894393002
Cr-Commit-Position: refs/heads/master@{#474230}

[modify] https://crrev.com/a590e1184af37fc63ff311976de04dda8d48eb02/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h
[modify] https://crrev.com/a590e1184af37fc63ff311976de04dda8d48eb02/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImplTest.cpp

Project Member Comment 44 by bugdroid1@chromium.org, May 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25c86433d672606b4daaceecc589664365d63ddf

commit 25c86433d672606b4daaceecc589664365d63ddf
Author: rlanday <rlanday@chromium.org>
Date: Wed May 24 10:54:37 2017

Split DMLEditor::ShiftMarkers() into content-dependent and -independent versions

I modified the logic that updates DocumentMarker in response to edit operations
in https://codereview.chromium.org/2755013004. However, it introduced a bug when
doing spellcheck replace (the old behavior removed the marker when the marked
text was changed and the new behavior doesn't).

This CL renames DocumentMarkerListEditor::ShiftMarkers() to
ShiftMarkersContentIndependent() and adds a new method that implements the old
behavior as ShiftMarkersContentDependent(). Spelling, Grammar, and TextMatch
markers should be removed when the text they mark changes, so those marker list
impls are being changed to use the old behavior. Composition markers don't
depend on the text they mark, so they're still using the new behavior (although
in practice it doesn't matter much since Composition markers are cleared by most
editing operations). Where we really need the new behavior is to implement
Android SuggestionSpan support (see crbug.com/672259).

As all non-Composition markers currently use the ContentDependent behavior, I
had to remove/change the expected behavior for a lot of InputMethodController
tests that were testing the new update behavior.

BUG=707867,722721

Review-Url: https://codereview.chromium.org/2895353003
Cr-Commit-Position: refs/heads/master@{#474245}

[modify] https://crrev.com/25c86433d672606b4daaceecc589664365d63ddf/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
[modify] https://crrev.com/25c86433d672606b4daaceecc589664365d63ddf/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[modify] https://crrev.com/25c86433d672606b4daaceecc589664365d63ddf/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/25c86433d672606b4daaceecc589664365d63ddf/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[modify] https://crrev.com/25c86433d672606b4daaceecc589664365d63ddf/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp
[modify] https://crrev.com/25c86433d672606b4daaceecc589664365d63ddf/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp

Project Member Comment 45 by bugdroid1@chromium.org, May 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d54305bd8f1dc8b62398e229603dfd4165e1447

commit 1d54305bd8f1dc8b62398e229603dfd4165e1447
Author: rlanday <rlanday@chromium.org>
Date: Wed May 24 23:02:17 2017

Fix some callsites of AddMarker() in InputMethodControllerTest

This AddMarker() method should really only be used for Spelling and Grammar
markers (I'm going to split it into AddSpellingMarker() and AddGrammarMarker()
in a later CL to eliminate this problem). This CL fixes some test cases that
create TextMatch markers to use AddTextMatchMarker().

BUG=707867

Review-Url: https://codereview.chromium.org/2897103003
Cr-Commit-Position: refs/heads/master@{#474463}

[modify] https://crrev.com/1d54305bd8f1dc8b62398e229603dfd4165e1447/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp

Project Member Comment 46 by bugdroid1@chromium.org, May 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0115f469e06ebf49f0265ec89eee5e9220f187c

commit f0115f469e06ebf49f0265ec89eee5e9220f187c
Author: rlanday <rlanday@chromium.org>
Date: Wed May 24 23:52:25 2017

Change DMC::AddCompositionMarker() to take EphemeralRange

Change DocumentMarkerController::AddCompositionMarker() from taking two
Positions (start and end) to taking an EphemeralRange. This matches
AddTextMatchMarker() and is how we eventually want all the Add*Marker() methods
to work.

BUG=707867

Review-Url: https://codereview.chromium.org/2899923002
Cr-Commit-Position: refs/heads/master@{#474480}

[modify] https://crrev.com/f0115f469e06ebf49f0265ec89eee5e9220f187c/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/f0115f469e06ebf49f0265ec89eee5e9220f187c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/f0115f469e06ebf49f0265ec89eee5e9220f187c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/f0115f469e06ebf49f0265ec89eee5e9220f187c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/f0115f469e06ebf49f0265ec89eee5e9220f187c/third_party/WebKit/Source/core/testing/Internals.cpp

Project Member Comment 47 by bugdroid1@chromium.org, May 25 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43ffa71e810074eb21a15188cc408ab67039b0a8

commit 43ffa71e810074eb21a15188cc408ab67039b0a8
Author: rlanday <rlanday@chromium.org>
Date: Thu May 25 02:40:18 2017

Split general DMC::AddMarker() method into spelling/grammar versions

Right now DocumentMarker::AddMarker() appears to be usable for any marker type,
when really the params it takes (start/end position and a description) are
really only suitable for spelling/grammar markers. So I'm splitting it into
AddSpellingMarker() and AddGrammarMarker(). After this, we'll have a 1-to-1
mapping between marker types and Add*Marker() methods.

I will change these methods in a follow-up CL to take an EphemeralRange instead
of two Positions.

BUG=707867

Review-Url: https://codereview.chromium.org/2895253003
Cr-Commit-Position: refs/heads/master@{#474532}

[modify] https://crrev.com/43ffa71e810074eb21a15188cc408ab67039b0a8/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/43ffa71e810074eb21a15188cc408ab67039b0a8/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/43ffa71e810074eb21a15188cc408ab67039b0a8/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/43ffa71e810074eb21a15188cc408ab67039b0a8/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/43ffa71e810074eb21a15188cc408ab67039b0a8/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/43ffa71e810074eb21a15188cc408ab67039b0a8/third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Project Member Comment 48 by bugdroid1@chromium.org, May 25 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b8a4e0948600fe3347678918abb63c43e1c2ee3

commit 8b8a4e0948600fe3347678918abb63c43e1c2ee3
Author: rlanday <rlanday@chromium.org>
Date: Thu May 25 07:07:47 2017

Move some rendered rect method impls from DMC To TextMatchMarkerListImpl

In this CL, I'm moving some rendered rect-related logic for TextMatch markers
from DocumentMarkerController into TextMatchMarkerListImpl. I am adding two
methods to TextMatchMarkerListImpl:

AppendRenderedRectsToVector(const Node&, Vector<IntRect>*)
UpdateMarkerRenderedRectIfNeeded(const Node&, RenderedDocumentMarker&)

In a later CL, I will move the implementations from DocumentMarkerController.cpp
to TextMatchMarkerListImpl.cpp, and make UpdateMarkerRenderedRectIfNeeded a
private method of TextMatchMarkerListImpl.

BUG=707867

Review-Url: https://codereview.chromium.org/2896703003
Cr-Commit-Position: refs/heads/master@{#474596}

[modify] https://crrev.com/8b8a4e0948600fe3347678918abb63c43e1c2ee3/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/8b8a4e0948600fe3347678918abb63c43e1c2ee3/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/8b8a4e0948600fe3347678918abb63c43e1c2ee3/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h

Project Member Comment 49 by bugdroid1@chromium.org, May 25 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/746e2408f9f35f9acc8d9ecb991e1de9be729db8

commit 746e2408f9f35f9acc8d9ecb991e1de9be729db8
Author: rlanday <rlanday@chromium.org>
Date: Thu May 25 07:17:06 2017

Move some method impls from DocumentMarkerController.cpp to final place

In https://codereview.chromium.org/2896703003, I'm splitting off some code in
some DocumentMarkerController methods into new methods in
TextMatchMarkerListImpl, but leaving the implementations of the new methods in
DocumentMarkerController.cpp to make that CL smaller. In this CL, I'm moving
these implementations to their final location in TextMatchMarkerListImpl.cpp.

I'm also combining the static method UpdateMarkerRenderedRect() in
DocumentMarkerController into
TextMatchMarkerListImpl::UpdateMarkerRenderedRectIfNeeded().

BUG=707867

Review-Url: https://codereview.chromium.org/2900573002
Cr-Commit-Position: refs/heads/master@{#474599}

[modify] https://crrev.com/746e2408f9f35f9acc8d9ecb991e1de9be729db8/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/746e2408f9f35f9acc8d9ecb991e1de9be729db8/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp

Project Member Comment 50 by bugdroid1@chromium.org, May 25 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b005061f90a81b14985fb155be147b5607f5245

commit 4b005061f90a81b14985fb155be147b5607f5245
Author: rlanday <rlanday@chromium.org>
Date: Thu May 25 08:33:47 2017

Don't create RenderedDocumentMarkers in SpellCheckMarkerListImpl

I meant to do this as part of https://codereview.chromium.org/2883503004 but
forgot that we have a custom Add() method in SpellCheckMarkerListImpl. Oops

BUG=707867

Review-Url: https://codereview.chromium.org/2899413003
Cr-Commit-Position: refs/heads/master@{#474607}

[modify] https://crrev.com/4b005061f90a81b14985fb155be147b5607f5245/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp

Project Member Comment 51 by bugdroid1@chromium.org, May 26 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f04ef6cf643d773e4176cbc4142251b18e959b7f

commit f04ef6cf643d773e4176cbc4142251b18e959b7f
Author: rlanday <rlanday@chromium.org>
Date: Fri May 26 05:07:13 2017

Add TextMatchMarkerListImpl::SetTextMatchMarkersActive()

Adding this helper method allows us to simplify the implementation of
DocumentMarkerController::SetTextMatchMarkersActive(), which was previously
depending on details of the implementation of TextMatchMarkerListImpl (whether
or not it stores its markers in sorted order).

BUG=707867

Review-Url: https://codereview.chromium.org/2904973003
Cr-Commit-Position: refs/heads/master@{#474914}

[modify] https://crrev.com/f04ef6cf643d773e4176cbc4142251b18e959b7f/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/f04ef6cf643d773e4176cbc4142251b18e959b7f/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/f04ef6cf643d773e4176cbc4142251b18e959b7f/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h

Project Member Comment 52 by bugdroid1@chromium.org, May 26 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/802826d9bacfad322a4ec415373d1ed3f317a2a2

commit 802826d9bacfad322a4ec415373d1ed3f317a2a2
Author: rlanday <rlanday@chromium.org>
Date: Fri May 26 18:35:35 2017

Rename RenderedDocumentMarker to TextMatchMarker

RenderedDocumentMarker used to be created for all DocumentMarker types. Now it's
only created for TextMatchMarkers. Renaming it to TextMatchMarker sets us up to
move all of the TextMatch-specific functionality out of DocumentMarker into this
class. I eventually want to do this for all the other marker types as well.

BUG=707867

Review-Url: https://codereview.chromium.org/2898953005
Cr-Commit-Position: refs/heads/master@{#475070}

[modify] https://crrev.com/802826d9bacfad322a4ec415373d1ed3f317a2a2/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/802826d9bacfad322a4ec415373d1ed3f317a2a2/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[rename] https://crrev.com/802826d9bacfad322a4ec415373d1ed3f317a2a2/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h
[modify] https://crrev.com/802826d9bacfad322a4ec415373d1ed3f317a2a2/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp

Project Member Comment 53 by bugdroid1@chromium.org, May 26 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c1c503b1d3d6a8f4f7fcfce84796a0877255d1d

commit 4c1c503b1d3d6a8f4f7fcfce84796a0877255d1d
Author: rlanday <rlanday@chromium.org>
Date: Fri May 26 20:36:59 2017

Add check to make TextMatchMarker cast safer

Currently, ToTextMatchMarker() is an unchecked cast. This CL adds a condition
for describing when this cast is safe so this cast can be DCHECKed in debug
mode.

BUG=707867

Review-Url: https://codereview.chromium.org/2904123003
Cr-Commit-Position: refs/heads/master@{#475112}

[modify] https://crrev.com/4c1c503b1d3d6a8f4f7fcfce84796a0877255d1d/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 54 by bugdroid1@chromium.org, May 26 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b73d36ca42ac01b44b8cda903978a0fe79fdc0e

commit 8b73d36ca42ac01b44b8cda903978a0fe79fdc0e
Author: rlanday <rlanday@chromium.org>
Date: Fri May 26 21:19:22 2017

Remove WTF_ALLOW_MOVE_INIT_AND_COMPARE_WITH_MEM_FUNCTIONS from TextMatchMarker.h

This appears to date back to https://codereview.chromium.org/289323003

It does not appear to be necessary anymore.

BUG=707867

Review-Url: https://codereview.chromium.org/2909543002
Cr-Commit-Position: refs/heads/master@{#475132}

[modify] https://crrev.com/8b73d36ca42ac01b44b8cda903978a0fe79fdc0e/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 55 by bugdroid1@chromium.org, May 30 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68bd4c5b8ab7bb2d11adc31754197433825bf1da

commit 68bd4c5b8ab7bb2d11adc31754197433825bf1da
Author: rlanday <rlanday@chromium.org>
Date: Tue May 30 21:06:02 2017

Make DocumentMarkerController::Add[Spelling/Grammar]Marker() take EphemeralRange

Currently DocumentMarkerController::AddSpellingMarker() and AddGrammarMarker()
take two Positions (start and end). This CL changes them to instead take an
EphemeralRange for consistency with AddTextMatchMarker() and
AddCompositionMarker().

BUG=707867

Review-Url: https://codereview.chromium.org/2901213008
Cr-Commit-Position: refs/heads/master@{#475672}

[modify] https://crrev.com/68bd4c5b8ab7bb2d11adc31754197433825bf1da/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/68bd4c5b8ab7bb2d11adc31754197433825bf1da/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/68bd4c5b8ab7bb2d11adc31754197433825bf1da/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/68bd4c5b8ab7bb2d11adc31754197433825bf1da/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/68bd4c5b8ab7bb2d11adc31754197433825bf1da/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/68bd4c5b8ab7bb2d11adc31754197433825bf1da/third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Project Member Comment 56 by bugdroid1@chromium.org, May 31 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12b75fd5be227207ef7382add4c96fa41f6e40e4

commit 12b75fd5be227207ef7382add4c96fa41f6e40e4
Author: rlanday <rlanday@chromium.org>
Date: Wed May 31 00:24:18 2017

Add tests for DocumentMarkerListEditor::ShiftMarkersContent[D/In]ependent

When we split ShiftMarkers() into ShiftMarkersContentDependent()/
ShiftMarkersContentIndependent() in https://codereview.chromium.org/2895353003,
I had to remove some test cases in InputMethodControllerTest which were testing
the (somewhat complex) ContentIndependent behavior because we don't currently
have any marker types that actually use the behavior (other than Composition
markers, which get cleared by many editing operations). So I'm adding a set of
test cases to directly test ShiftMarkersContentDependent() and
ShiftMarkersContentIndependent().

BUG=707867

Review-Url: https://codereview.chromium.org/2903413003
Cr-Commit-Position: refs/heads/master@{#475703}

[modify] https://crrev.com/12b75fd5be227207ef7382add4c96fa41f6e40e4/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/12b75fd5be227207ef7382add4c96fa41f6e40e4/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[add] https://crrev.com/12b75fd5be227207ef7382add4c96fa41f6e40e4/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditorTest.cpp

Project Member Comment 57 by bugdroid1@chromium.org, May 31 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b0a494122ebe2fc48099a9549b0acbf0b28571d

commit 6b0a494122ebe2fc48099a9549b0acbf0b28571d
Author: rlanday <rlanday@chromium.org>
Date: Wed May 31 01:37:56 2017

Make TextMatchMarkers constructible in single step

Currently, we only construct instances of TextMatchMarker (previously
RenderedDocumentMarker) inside TextMatchMarkerListImpl, by calling
TextMatchMarker::Create() on an instance of DocumentMarker. This CL changes this
so callers directly construct instances of TextMatchMarker. This sets us up to
eventually move the storage and accessor methods for whether or not the marker
is active out of DocumentMarker and into TextMatchMarker.

BUG=707867

Review-Url: https://codereview.chromium.org/2905753002
Cr-Commit-Position: refs/heads/master@{#475724}

[modify] https://crrev.com/6b0a494122ebe2fc48099a9549b0acbf0b28571d/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/6b0a494122ebe2fc48099a9549b0acbf0b28571d/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/6b0a494122ebe2fc48099a9549b0acbf0b28571d/third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp
[modify] https://crrev.com/6b0a494122ebe2fc48099a9549b0acbf0b28571d/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h
[modify] https://crrev.com/6b0a494122ebe2fc48099a9549b0acbf0b28571d/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/6b0a494122ebe2fc48099a9549b0acbf0b28571d/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImplTest.cpp

Project Member Comment 58 by bugdroid1@chromium.org, May 31 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/182740235efc11d585d6710485d66fff4fcf84ae

commit 182740235efc11d585d6710485d66fff4fcf84ae
Author: tyoshino <tyoshino@chromium.org>
Date: Wed May 31 01:49:36 2017

Revert of [DMC #17] Make TextMatchMarkers constructible in single step (patchset #4 id:60001 of https://codereview.chromium.org/2905753002/ )

Reason for revert:
This broke the build.

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder%20%28dbg%29/builds/201575

Original issue's description:
> Make TextMatchMarkers constructible in single step
>
> Currently, we only construct instances of TextMatchMarker (previously
> RenderedDocumentMarker) inside TextMatchMarkerListImpl, by calling
> TextMatchMarker::Create() on an instance of DocumentMarker. This CL changes this
> so callers directly construct instances of TextMatchMarker. This sets us up to
> eventually move the storage and accessor methods for whether or not the marker
> is active out of DocumentMarker and into TextMatchMarker.
>
> BUG=707867
>
> Review-Url: https://codereview.chromium.org/2905753002
> Cr-Commit-Position: refs/heads/master@{#475724}
> Committed: https://chromium.googlesource.com/chromium/src/+/6b0a494122ebe2fc48099a9549b0acbf0b28571d

TBR=yosin@chromium.org,xiaochengh@chromium.org,rlanday@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=707867

Review-Url: https://codereview.chromium.org/2912133003
Cr-Commit-Position: refs/heads/master@{#475732}

[modify] https://crrev.com/182740235efc11d585d6710485d66fff4fcf84ae/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/182740235efc11d585d6710485d66fff4fcf84ae/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/182740235efc11d585d6710485d66fff4fcf84ae/third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp
[modify] https://crrev.com/182740235efc11d585d6710485d66fff4fcf84ae/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h
[modify] https://crrev.com/182740235efc11d585d6710485d66fff4fcf84ae/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/182740235efc11d585d6710485d66fff4fcf84ae/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImplTest.cpp

Project Member Comment 59 by bugdroid1@chromium.org, May 31 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529

commit be1de5ecae843ecb1fe73577a5f6fb4e51c3f529
Author: rlanday <rlanday@chromium.org>
Date: Wed May 31 03:47:41 2017

[Reland] Make TextMatchMarkers constructible in single step

Currently, we only construct instances of TextMatchMarker (previously
RenderedDocumentMarker) inside TextMatchMarkerListImpl, by calling
TextMatchMarker::Create() on an instance of DocumentMarker. This CL changes this
so callers directly construct instances of TextMatchMarker. This sets us up to
eventually move the storage and accessor methods for whether or not the marker
is active out of DocumentMarker and into TextMatchMarker.

BUG=707867

Review-Url: https://codereview.chromium.org/2905753002
Cr-Original-Commit-Position: refs/heads/master@{#475724}
Committed: https://chromium.googlesource.com/chromium/src/+/6b0a494122ebe2fc48099a9549b0acbf0b28571d
Review-Url: https://codereview.chromium.org/2905753002
Cr-Commit-Position: refs/heads/master@{#475781}

[modify] https://crrev.com/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditorTest.cpp
[modify] https://crrev.com/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529/third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp
[modify] https://crrev.com/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h
[modify] https://crrev.com/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/be1de5ecae843ecb1fe73577a5f6fb4e51c3f529/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImplTest.cpp

Project Member Comment 60 by bugdroid1@chromium.org, May 31 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa78f83615f6278ffe657bb6119d1b8339920b36

commit fa78f83615f6278ffe657bb6119d1b8339920b36
Author: rlanday <rlanday@chromium.org>
Date: Wed May 31 06:13:30 2017

Eliminate DocumentMarker and TextMatchMarker copy constructors

We decided DocumentMarkers shouldn't ever be copied. After
https://codereview.chromium.org/2905753002, we no longer need to copy
DocumentMarkers to create TextMatchMarkers, so we can remove the copy
constructor. We can also remove the TextMatchMarker constructor that takes a
DocumentMarker.

We need to remove the copy constructor anyway because we're turning
DocumentMarker into a polymorphic class hierarchy, and copying the base class
will no longer produce correct behavior.

BUG=707867

Review-Url: https://codereview.chromium.org/2902393003
Cr-Commit-Position: refs/heads/master@{#475824}

[modify] https://crrev.com/fa78f83615f6278ffe657bb6119d1b8339920b36/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/fa78f83615f6278ffe657bb6119d1b8339920b36/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/fa78f83615f6278ffe657bb6119d1b8339920b36/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/fa78f83615f6278ffe657bb6119d1b8339920b36/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 61 by bugdroid1@chromium.org, May 31 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68adbbbf1a52544dbd52c2248e530f722d638b34

commit 68adbbbf1a52544dbd52c2248e530f722d638b34
Author: rlanday <rlanday@chromium.org>
Date: Wed May 31 07:38:46 2017

Refactor DocumentMarkerController::Add*Marker() methods

Currently there's a lot of duplicated code in all the MarkerType-specific
DocumentMarkerController::Add*Marker() methods. This CL factors out the common
logic into an AddMarkerInternal() method, and renames the existing (private)
AddMarker() method to AddMarkerToNode().

This CL also adds some filtering to avoid constructing DocumentMarkers with
start/end offsets that are the same (the current version of the code constructs
these DocumentMarkers, then filters them out before actually adding them), or
when the range returned by TextIterator is not a text node (we don't currently
do this filtering).

BUG=707867

Review-Url: https://codereview.chromium.org/2916463002
Cr-Commit-Position: refs/heads/master@{#475840}

[modify] https://crrev.com/68adbbbf1a52544dbd52c2248e530f722d638b34/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/68adbbbf1a52544dbd52c2248e530f722d638b34/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h

Project Member Comment 62 by bugdroid1@chromium.org, May 31 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c59068590b02e4c25544b62a5a9a5cf6863252b3

commit c59068590b02e4c25544b62a5a9a5cf6863252b3
Author: rlanday <rlanday@chromium.org>
Date: Wed May 31 21:18:46 2017

Eliminate DocumentMarkerTextMatch (subclass of DocumentMarkerDetails)

I eventually want to eliminate all the DocumentMarkerDetails subclasses and move
the storage for MarkerType-specific information into the type-specific
subclasses of DocumentMarker. This CL does this for TextMatchMarker. In a later
CL, I will remove IsMarkerActive() and SetIsMarkerActive() from DocumentMarker
and force callers to cast to TextMatchMarker to call these methods.

BUG=707867

Review-Url: https://codereview.chromium.org/2904093002
Cr-Commit-Position: refs/heads/master@{#476026}

[modify] https://crrev.com/c59068590b02e4c25544b62a5a9a5cf6863252b3/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/c59068590b02e4c25544b62a5a9a5cf6863252b3/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/c59068590b02e4c25544b62a5a9a5cf6863252b3/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 63 by bugdroid1@chromium.org, Jun 1
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d7032c3fd7b7311e54b49e267f843ab9196279e

commit 6d7032c3fd7b7311e54b49e267f843ab9196279e
Author: rlanday <rlanday@chromium.org>
Date: Thu Jun 01 01:52:02 2017

Remove DocumentMarker::IsActiveMatch() and SetIsActiveMatch() methods

In https://codereview.chromium.org/2904093002 I'm moving the storage for the
MatchStatus flag from a subclass of DocumentMarkerDetails into TextMatchMarker.
This CL follows up by removing the getter/setter methods for this flag from
DocumentMarker, forcing callers to cast to TextMatchMarker. This helps clarify
where callers are making assumptions about the types of certain DocumentMarkers.

BUG=707867
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2909553002
Cr-Commit-Position: refs/heads/master@{#476142}

[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/layout/line/InlineTextBox.cpp
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/layout/line/InlineTextBox.h
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/layout/svg/line/SVGInlineTextBox.cpp
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/layout/svg/line/SVGInlineTextBox.h
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.h
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.h
[modify] https://crrev.com/6d7032c3fd7b7311e54b49e267f843ab9196279e/third_party/WebKit/Source/core/testing/Internals.cpp

Project Member Comment 64 by bugdroid1@chromium.org, Jun 1
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7059102e597d59abf00aa67caa4a7b39266190fa

commit 7059102e597d59abf00aa67caa4a7b39266190fa
Author: rlanday <rlanday@chromium.org>
Date: Thu Jun 01 07:01:18 2017

Add DISALLOW_COPY_AND_ASSIGN to TextMatchMarker

We're adding DISALLOW_COPY_AND_ASSIGN to CompositionMarker in
https://codereview.chromium.org/2908643002 and to SpellingMarker and
GrammarMarker in https://codereview.chromium.org/2911723002, so we should add
it to TextMatchMarker as well for consistency.

BUG=707867

Review-Url: https://codereview.chromium.org/2916753003
Cr-Commit-Position: refs/heads/master@{#476216}

[modify] https://crrev.com/7059102e597d59abf00aa67caa4a7b39266190fa/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 65 by bugdroid1@chromium.org, Jun 1
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2546cdae560f745b5ee91856244bf12f36729e44

commit 2546cdae560f745b5ee91856244bf12f36729e44
Author: rlanday <rlanday@chromium.org>
Date: Thu Jun 01 07:29:18 2017

Add CompositionMarker (subclass of DocumentMarker)

This CL moves the Composition marker-specific functionality of DocumentMarker
into a new CompositionMarker subclass (similar to TextMatchMarker). I will do
the same for Spelling and Grammar markers in future CLs.

BUG=707867
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2908643002
Cr-Commit-Position: refs/heads/master@{#476220}

[modify] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/BUILD.gn
[add] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/markers/CompositionMarker.cpp
[add] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/markers/CompositionMarker.h
[modify] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImplTest.cpp
[add] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/markers/CompositionMarkerTest.cpp
[modify] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/2546cdae560f745b5ee91856244bf12f36729e44/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp

Project Member Comment 66 by bugdroid1@chromium.org, Jun 1
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45ab0de82123f81a41e7af931ba30e180532066f

commit 45ab0de82123f81a41e7af931ba30e180532066f
Author: rlanday <rlanday@chromium.org>
Date: Thu Jun 01 20:32:58 2017

Add SpellingMarker and GrammarMarker (subclasses of DocumentMarker)

In https://codereview.chromium.org/2904093002, I'm eliminating the
DocumentMarkerDetails subclass for TextMatchMarker in favor of storing the
TextMatchMarker-specific data directly on TextMatchMarker. In
https://codereview.chromium.org/2908643002, I'm creaing a similar subclass for
CompositionMarker. In this CL, I'm doing the same for SpellingMarker and
GrammarMarker, which means we can now eliminate DocumentMarkerDetails.

In a follow-up CL, I will make MarkerType() virtual so we won't have to store
the MarkerType as a field in DocumentMarker.

BUG=707867

Review-Url: https://codereview.chromium.org/2911723002
Cr-Commit-Position: refs/heads/master@{#476419}

[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/GrammarMarker.cpp
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/GrammarMarker.h
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/GrammarMarkerTest.cpp
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/SpellCheckMarker.cpp
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/SpellCheckMarker.h
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/SpellingMarker.cpp
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/SpellingMarker.h
[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/SpellingMarkerListImplTest.cpp
[add] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/editing/markers/SpellingMarkerTest.cpp
[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/45ab0de82123f81a41e7af931ba30e180532066f/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp

Project Member Comment 67 by bugdroid1@chromium.org, Jun 1
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/adb3fe930f151b63798729d3e43f37f6097a7875

commit adb3fe930f151b63798729d3e43f37f6097a7875
Author: rlanday <rlanday@chromium.org>
Date: Thu Jun 01 23:12:46 2017

Make DocumentMarker::GetType() virtual

Now that we have subclasses of DocumentMarker for all the different MarkerTypes,
we can make GetType() a virtual method on each of the subtypes so we don't
actually have to store a field in DocumentMarker for this information.

BUG=707867

Review-Url: https://codereview.chromium.org/2911733002
Cr-Commit-Position: refs/heads/master@{#476473}

[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/CompositionMarker.cpp
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/CompositionMarker.h
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/DocumentMarker.cpp
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/GrammarMarker.cpp
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/GrammarMarker.h
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/SpellCheckMarker.cpp
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/SpellCheckMarker.h
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/SpellingMarker.cpp
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/SpellingMarker.h
[add] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.cpp
[modify] https://crrev.com/adb3fe930f151b63798729d3e43f37f6097a7875/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 68 by bugdroid1@chromium.org, Jun 2
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7fd1b84241b9f5fb8f43842317a722a0e8243b46

commit 7fd1b84241b9f5fb8f43842317a722a0e8243b46
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 02 01:10:02 2017

Add CompositionMarker::Thickness enum

This CL changes CompositionMarker to take and store its thickness param as an
enum instead of a bool. This makes code creating CompositionMarkers easier to
understand (and also enables us to add additional thickness values in the future
if necessary).

BUG=707867
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2906953002
Cr-Commit-Position: refs/heads/master@{#476509}

[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/LayoutTests/editing/composition-marker-basic.html
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/LayoutTests/editing/composition-marker-split.html
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/markers/CompositionMarker.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/markers/CompositionMarker.h
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImplTest.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/markers/CompositionMarkerTest.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/7fd1b84241b9f5fb8f43842317a722a0e8243b46/third_party/WebKit/Source/core/testing/Internals.idl

Project Member Comment 69 by bugdroid1@chromium.org, Jun 2
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb2881f8de65ec1c7d64d92ce414a5f108a86270

commit bb2881f8de65ec1c7d64d92ce414a5f108a86270
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 02 03:24:50 2017

Move TextMatchMarker method implementations to TextMatchMarker.cpp

TextMatchMarker used to be RenderedDocumentMarker, which didn't have a .cpp
file, and had all its method implementations in a header file. This CL moves
TextMatchMarker's method implementations to its .cpp file for cleanliness, as
well as consistency with the classes for the other marker types.

BUG=707867

Review-Url: https://codereview.chromium.org/2919603005
Cr-Commit-Position: refs/heads/master@{#476550}

[modify] https://crrev.com/bb2881f8de65ec1c7d64d92ce414a5f108a86270/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.cpp
[modify] https://crrev.com/bb2881f8de65ec1c7d64d92ce414a5f108a86270/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 70 by bugdroid1@chromium.org, Jun 2
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38610ad5a07527b89be3116610c2c3bff822d5e8

commit 38610ad5a07527b89be3116610c2c3bff822d5e8
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 02 03:26:10 2017

Make DocumentMarkerListEditor::RemoveMarkers() more efficient

The current implementation of this method does a binary search to get the first
marker to be removed, then linearly walks the markers, erasing all that fall in
the specified range. This can potentially take time quadratic in the number of
markers to be removed, and always takes at least log N time.

This new implementation finds both the start point and end point of the range to
be erased in log N time, then does a single erase operation that takes time
linear in the number of markers to be removed. So the asymptotic lower bound
stays the same and the asymptotic upper bound is improved from quadratic to
linear.

I've also added a few more test cases to DocumentMarkerListEditor test to verify
the new implementation works correctly.

BUG=707867

Review-Url: https://codereview.chromium.org/2922623002
Cr-Commit-Position: refs/heads/master@{#476551}

[modify] https://crrev.com/38610ad5a07527b89be3116610c2c3bff822d5e8/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/38610ad5a07527b89be3116610c2c3bff822d5e8/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditorTest.cpp

Project Member Comment 71 by bugdroid1@chromium.org, Jun 2
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c8edfacd6a14a7da6d0977548a8009de764993c

commit 7c8edfacd6a14a7da6d0977548a8009de764993c
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 02 16:55:44 2017

Rename TextMatchMarker::State to LayoutStatus

Adding "Layout" to the name since we also have MatchStatus, and change State to
Status for consistency (they're both enums). I'm also reordering the
layout_status_ member variable to be above rendered_rect_ since whether or not
rendered_rect_ is currently valid depends on layout_status_.

BUG=707867

Review-Url: https://codereview.chromium.org/2915193002
Cr-Commit-Position: refs/heads/master@{#476693}

[modify] https://crrev.com/7c8edfacd6a14a7da6d0977548a8009de764993c/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.cpp
[modify] https://crrev.com/7c8edfacd6a14a7da6d0977548a8009de764993c/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h

Project Member Comment 72 by bugdroid1@chromium.org, Jun 5
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9187085c7cdff3a2864af92f566d7afacbb3060c

commit 9187085c7cdff3a2864af92f566d7afacbb3060c
Author: rlanday <rlanday@chromium.org>
Date: Mon Jun 05 17:16:56 2017

Rename TextMatchMarker::rendered_rect_ to layout_rect_

Rename TextMatchMarker::rendered_rect_ to layout_rect_ and TextMatchMarker
methods with "RenderedRect" in name to instead use "LayoutRect". LayoutRect is a
more specific name than RenderedRect (layout is one step in the rendering
pipeline).

BUG=707867

Review-Url: https://codereview.chromium.org/2920913003
Cr-Commit-Position: refs/heads/master@{#477008}

[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.cpp
[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/editing/markers/TextMatchMarker.h
[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h
[modify] https://crrev.com/9187085c7cdff3a2864af92f566d7afacbb3060c/third_party/WebKit/Source/core/frame/LocalFrameView.cpp

Project Member Comment 73 by bugdroid1@chromium.org, Jun 5
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1a451ab5d6f0a8ec7c3e389fb61c34f363768c4

commit f1a451ab5d6f0a8ec7c3e389fb61c34f363768c4
Author: rlanday <rlanday@chromium.org>
Date: Mon Jun 05 19:12:10 2017

Refactor SpellCheckMarkerListImpl::Add()

Currently, the way this method inserts markers (except for the case where we
can insert at the end of the list) is inefficient. Its runtime is quadratic in
the number of markers being merged. We can make this more efficient by binary
searching for both the start and end of the overlapped range, and then erasing
the whole range (minus one marker, which we swap with the newly-inserted marker)
in one go. This method should now always run in time no worse than linear in the
number of markers.

BUG=707867

Review-Url: https://codereview.chromium.org/2915373002
Cr-Commit-Position: refs/heads/master@{#477051}

[modify] https://crrev.com/f1a451ab5d6f0a8ec7c3e389fb61c34f363768c4/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp

Project Member Comment 74 by bugdroid1@chromium.org, Jun 5
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ee691bbd589760fec30ee27dcb408a70feb66d3

commit 6ee691bbd589760fec30ee27dcb408a70feb66d3
Author: rlanday <rlanday@chromium.org>
Date: Mon Jun 05 19:26:44 2017

Refactor DocumentMarkerListEditor::ShiftMarkersContent{Ind,D}ependent

The current implementations of
DocumentMarkerListEditor::ShiftMarkersContentDependent() and
ShiftMarkersContentIndependent() can potentially take quadratic time in the
number of markers if we have to remove them all, since we call Vector::erase()
on them one-by-one. We can avoid this by creating a new MarkerList and swapping
afterward. The best-case runtime should still be linear for both the old and new
implementations.

BUG=707867

Review-Url: https://codereview.chromium.org/2917963003
Cr-Commit-Position: refs/heads/master@{#477055}

[modify] https://crrev.com/6ee691bbd589760fec30ee27dcb408a70feb66d3/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp

Project Member Comment 75 by bugdroid1@chromium.org, Jun 17
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/206f5f0fcdd4809a342fdde6dbcfa24d2dbc306f

commit 206f5f0fcdd4809a342fdde6dbcfa24d2dbc306f
Author: rlanday <rlanday@chromium.org>
Date: Sat Jun 17 02:32:01 2017

Refactor InlineTextBoxPainter to avoid using CompositionUnderline

CompositionUnderline is a class that exists mainly so callers can specify the
information necessary for creating CompositionMarkers (start/end offset,
underline color, thickness, background color). It's currently also serving
double-duty in InlineTextBoxPainter to pass this information around after it's
retrieved from a CompositionMarker. With the introduction of painting support
for ActiveSuggestionMarker in https://codereview.chromium.org/2925543003, this
starts to seem kind of strange, if it didn't already (since we're creating
CompositionUnderlines from non-Composition markers). This CL refactors
InlineTextBoxPainter to pass around references to DocumentMarkers/
StyleableMarkers directly instead of creating CompositionUnderlines.

BUG=707867
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2920373003
Cr-Commit-Position: refs/heads/master@{#480273}

[modify] https://crrev.com/206f5f0fcdd4809a342fdde6dbcfa24d2dbc306f/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
[modify] https://crrev.com/206f5f0fcdd4809a342fdde6dbcfa24d2dbc306f/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.h

Project Member Comment 76 by bugdroid1@chromium.org, Jun 28
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28fc48705db361c2fd93cd302c0d15c2a1ed8da4

commit 28fc48705db361c2fd93cd302c0d15c2a1ed8da4
Author: rlanday <rlanday@chromium.org>
Date: Wed Jun 28 03:29:23 2017

Optimize DocumentMarkerController::MarkerAtPosition()

This method is used in some selection-related operations to look for spellcheck
markers. Now that we have DocumentMarkerList::MarkersIntersectingRange() (added
in https://codereview.chromium.org/2952953002) which uses binary search, we no
longer need to do a linear search over all the markers in the node to implement
this method.

BUG=707867

Review-Url: https://codereview.chromium.org/2949333002
Cr-Commit-Position: refs/heads/master@{#482871}

[modify] https://crrev.com/28fc48705db361c2fd93cd302c0d15c2a1ed8da4/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Project Member Comment 77 by bugdroid1@chromium.org, Jul 7
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f37e7fd6eb873016a023acc58a121161128aeabd

commit f37e7fd6eb873016a023acc58a121161128aeabd
Author: rlanday <rlanday@chromium.org>
Date: Fri Jul 07 20:52:34 2017

Clean up DocumentMarkerController::MarkersFor()

I'm cleaning up two things here:

- Adding a check to PossiblyHasMarkers() (probably doesn't save us very much
  time, but we can at least avoid a hashmap lookup in the "no markers" case)
- Fixing the loop to just loop over the passed-in set of MarkerTypes vs. looping
  over all types and skipping over some of them

BUG=707867

Review-Url: https://codereview.chromium.org/2956453002
Cr-Commit-Position: refs/heads/master@{#485038}

[modify] https://crrev.com/f37e7fd6eb873016a023acc58a121161128aeabd/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Project Member Comment 78 by bugdroid1@chromium.org, Jul 21
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66e56b31988d34ecefab5212c9daa897b31325e2

commit 66e56b31988d34ecefab5212c9daa897b31325e2
Author: rlanday <rlanday@chromium.org>
Date: Fri Jul 21 01:53:28 2017

Add DocumentMarkerList::FirstMarkerIntersectingRange()

We currently have a method DocumentMarkerList::MarkersIntersectingRange() that
can be used to retrieve all the DocumentMarkers in the list intersecting a
specified offset range. This CL adds another method to DocumentMarkerList,
FirstMarkerIntersectingRange(), that can be used to more efficiently get just
one marker when there may be multiple markers intersecting a given range.

This method will be used to add the method
DocumentMarkerController::FirstMarkerIntersectingOffsetRange() in another CL:
https://codereview.chromium.org/2960473002

BUG=707867

Review-Url: https://codereview.chromium.org/2982313002
Cr-Commit-Position: refs/heads/master@{#488546}

[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImpl.cpp
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImpl.h
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.h
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditorTest.cpp
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.h
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/66e56b31988d34ecefab5212c9daa897b31325e2/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h

Project Member Comment 79 by bugdroid1@chromium.org, Jul 21
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a959e15bb4141aaf7041b2bbfd42f2d6d72cfc66

commit a959e15bb4141aaf7041b2bbfd42f2d6d72cfc66
Author: rlanday <rlanday@chromium.org>
Date: Fri Jul 21 01:59:41 2017

Add DocumentMarkerController::FirstMarkerIntersectingOffsetRange()

This is a method that takes a text node, a pair of start/end offsets, and a list
of MarkerTypes, and tries to find a DocumentMarker in the node of one of those
types that intersects the range [start_offset, end_offset].

This is similar to DocumentMarkerController::MarkersIntersectingRange(), which
I'm introducing in https://codereview.chromium.org/2948133004, except that this
method takes a Text node and offsets instead of taking an EphemeralRange/
EphemeralRangeInFlatTree, and only returns at most one DocumentMarker, instead
of returning all of them that match.

BUG=707867

Review-Url: https://codereview.chromium.org/2960473002
Cr-Commit-Position: refs/heads/master@{#488549}

[modify] https://crrev.com/a959e15bb4141aaf7041b2bbfd42f2d6d72cfc66/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/a959e15bb4141aaf7041b2bbfd42f2d6d72cfc66/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/a959e15bb4141aaf7041b2bbfd42f2d6d72cfc66/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp

Project Member Comment 80 by bugdroid1@chromium.org, Jul 25
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfc3b0fc01351c264824c22da6afbe6e64f46c47

commit bfc3b0fc01351c264824c22da6afbe6e64f46c47
Author: rlanday <rlanday@chromium.org>
Date: Tue Jul 25 02:30:14 2017

Comment/test for DocumentMarkerList::MarkersIntersectingRange() with collapsed range

When I added this method, I didn't realize it had useful behavior for the case
where a collapsed range is passed in. It turns out it does (it returns markers that
contain the start/end position in their interior), so I'm adding a comment
explaining this behavior and a test case testing it.

BUG=707867

Review-Url: https://codereview.chromium.org/2953183004
Cr-Commit-Position: refs/heads/master@{#489192}

[modify] https://crrev.com/bfc3b0fc01351c264824c22da6afbe6e64f46c47/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/bfc3b0fc01351c264824c22da6afbe6e64f46c47/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditorTest.cpp

Status: Fixed
Sign in to add a comment