Add support for persisting CompositionUnderlines in InputMethodController

Add support for persisting CompositionUnderlines in InputMethodController

This is needed to properly support the Android Voice IME. This IME uses
commitText() to underline certain pieces of text, and it expects these
underlines to persist until it replaces the text with a non-underlined string.

My current plan is for the Java ImeAdapter to keep track of all the
SuggestionSpans that the IME has created, and create a corresponding
PersistingCompositionUnderline for each one. The DocumentMarker infrastructure
will update the start/end locations of each span as the text is edited, and I
will add an API that ImeAdapter can call in response to touch events to get a
list of active CompositionUnderlines, which will have been tagged in such a way
that they can be mapped back to the SuggestionSpan. I've written code to do this
and verified that this approach works but I'm not including it in this CL.

An interesting question is whether we need to have an API for removing these
spans (since they otherwise persist). Using DOM ranges as an example, there is
no way to remove a DOM range from the DOM once attached (there used to be
range.detach(), but it has since been turned into a no-op). So I suspect that
adding an API to remove these ranges isn't really necessary. Replacing an entire
underlined span will collapse the span to be of zero-length but will not remove
it. Once we add support for tagging the spans, as mentioned above, we could add
a way to remove spans by tag, but I'm not convinced that that will really solve
any actual problems, at least for my use case. Or we could potentially make
zero-length spans just disappear, but this is not how DOM ranges behave, and
might break some other future use case for these CompositionUnderlines.

Note: this is unlikely to work correctly without this bug fix:
https://codereview.chromium.org/2594123003

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

[rlanday, 2017-01-03 20:48:09]

Description was changed from

==========
Add support for persisting CompositionUnderlines in InputMethodController

This is needed to properly support the Android Voice IME. This IME uses
commitText() to underline certain pieces of text, and it expects these
underlines to persist until it replaces the text with a non-underlined string.

My current plan is for the Java ImeAdapter to keep track of all the
SuggestionSpans that the IME has created, and create a corresponding
PersistingCompositionUnderline for each one. The DocumentMarker infrastructure
will update the start/end locations of each span as the text is edited, and I
will add an API that ImeAdapter can call in response to touch events to get a
list of active CompositionUnderlines, which will have been tagged in such a way
that they can be mapped back to the SuggestionSpan. I've written code to do this
and verified that this approach works but I'm not including it in this CL.

An interesting question is whether we need to have an API for removing these
spans (since they otherwise persist). Using DOM ranges as an example, there is
no way to remove a DOM range from the DOM once attached (there used to be
range.detach(), but it has since been turned into a no-op). So I suspect that
adding an API to remove these ranges isn't really necessary. Replacing an entire
underlined span will collapse the span to be of zero-length but will not remove
it. Once we add support for tagging the spans, as mentioned above, we could add
a way to remove spans by tag, but I'm not convinced that that will really solve
any actual problems, at least for my use case. Or we could potentially make
zero-length spans just disappear, but this is not how DOM ranges behave, and
might break some other future use case for these CompositionUnderlines.

Note: this is unlikely to work correctly without this bug fix:
https://codereview.chromium.org/2594123003

BUG=672259
==========

to

==========
Add support for persisting CompositionUnderlines in InputMethodController

This is needed to properly support the Android Voice IME. This IME uses
commitText() to underline certain pieces of text, and it expects these
underlines to persist until it replaces the text with a non-underlined string.

My current plan is for the Java ImeAdapter to keep track of all the
SuggestionSpans that the IME has created, and create a corresponding
PersistingCompositionUnderline for each one. The DocumentMarker infrastructure
will update the start/end locations of each span as the text is edited, and I
will add an API that ImeAdapter can call in response to touch events to get a
list of active CompositionUnderlines, which will have been tagged in such a way
that they can be mapped back to the SuggestionSpan. I've written code to do this
and verified that this approach works but I'm not including it in this CL.

An interesting question is whether we need to have an API for removing these
spans (since they otherwise persist). Using DOM ranges as an example, there is
no way to remove a DOM range from the DOM once attached (there used to be
range.detach(), but it has since been turned into a no-op). So I suspect that
adding an API to remove these ranges isn't really necessary. Replacing an entire
underlined span will collapse the span to be of zero-length but will not remove
it. Once we add support for tagging the spans, as mentioned above, we could add
a way to remove spans by tag, but I'm not convinced that that will really solve
any actual problems, at least for my use case. Or we could potentially make
zero-length spans just disappear, but this is not how DOM ranges behave, and
might break some other future use case for these CompositionUnderlines.

Note: this is unlikely to work correctly without this bug fix:
https://codereview.chromium.org/2594123003

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

[rlanday, 2017-01-03 21:07:02]

rlanday@chromium.org changed reviewers:
+ dcheng@chromium.org, pfeldman@chromium.org

[rlanday, 2017-01-03 21:07:03]

pfeldman, I need your review of the changes to:

- components/test_runner/text_input_controller.cc
- third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp

dcheng, I need your review of the changes to:
- content/common/content_param_traits_macros.h

It looks like you both own all the other files involved

https://codereview.chromium.org/2611813002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp
(right):

https://codereview.chromium.org/2611813002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp:648: //
Insert must happen before delete to preserve markers correctly
This is something that came up when I added a test case. There's some piece of
logic that sometimes replaced spaces with &nbsps; and the marker range was no
longer including the space because it was deleting the old space and adding the
new one (and ending up outside the marker range) instead of inserting first (so
the new space stays in the marker range).

https://codereview.chromium.org/2611813002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
(left):

https://codereview.chromium.org/2611813002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:755:
(*marker)->shiftOffsets(delta);
The existing code here seems to not be able to deal with the amount of
generality I need for my use case. There was some sequence of editing operations
I could do that would cause the beginning of a marker range to be deleted, and
this code would try to shift the offsets so the range had a negative start
value, which was not correct.

I don't think I'm changing the behavior for anything currently supported, only
better supporting this case where a range is partially deleted/replaced.

[rlanday, 2017-01-03 22:33:04]

Actually I may take back the comment about not needing to be able to remove the
PersistingCompositionUnderlines by tag...I will make a decision on this before I
put up the CL that adds support for the tags

[rlanday, 2017-01-05 02:01:28]

I just talked to aelias, we think we're going to go with a different approach
(having a CompositionUnderline type specifically for SuggestionSpan)