Remove methods of TextIterator that take Range objects

Remove methods of TextIterator that take Range objects

This patch removes some methods of TextIterator which take Range objects.

A Range object assumes that two positions are in the same DOM tree and
this doesn't match 'selection for WebComponents' which will allow
positions in different DOM trees. To use TextIterator in both the DOM
tree and the composed tree, we need to eliminate methods which take
Range objects.

Now TextIterator works on a DOM tree. After commiting this patch, we are
going to templatize TextIterator to accept another traversal strategy
(ComposedTreeTraversal).

This patch is result of collaboration work with yosin@chromium.org
for selection of web components. We are going to add some features to
this class for the composed tree.

BUG=275851
TEST=n/a; No behavior changes.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192746

[hajimehoshi, 2015-03-30 06:29:11]

hajimehoshi@chromium.org changed reviewers:
+ haraken@chromium.org

[hajimehoshi, 2015-03-30 06:29:12]

PTAL

[haraken, 2015-03-30 07:34:28]

LGTM

https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
File Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
Source/core/editing/iterators/TextIterator.cpp:108: if (start.isNull() ||
end.isNull())

Why do we need this check?

It seems the old code was calling initialize without having the check.

[hajimehoshi, 2015-03-30 07:40:56]

Thank you!

https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
File Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
Source/core/editing/iterators/TextIterator.cpp:108: if (start.isNull() ||
end.isNull())
On 2015/03/30 07:34:28, haraken wrote:
> 
> Why do we need this check?
> 
> It seems the old code was calling initialize without having the check.

It is because there are some cases in which |range| is null and actually some
tests fail without this check (see line 108 of the original file). I this this
doesn't change the behavior and is needed for backward compatibility.

[hajimehoshi, 2015-03-30 07:41:32]

On 2015/03/30 07:40:56, hajimehoshi wrote:
> Thank you!
> 
>
https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
> File Source/core/editing/iterators/TextIterator.cpp (right):
> 
>
https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
> Source/core/editing/iterators/TextIterator.cpp:108: if (start.isNull() ||
> end.isNull())
> On 2015/03/30 07:34:28, haraken wrote:
> > 
> > Why do we need this check?
> > 
> > It seems the old code was calling initialize without having the check.
> 
> It is because there are some cases in which |range| is null and actually some
> tests fail without this check (see line 108 of the original file). I this this
> doesn't change the behavior and is needed for backward compatibility.

s/I this/I think/

[hajimehoshi, 2015-03-30 07:44:36]

Let me check what tests fail and if I can fix those.

[hajimehoshi, 2015-03-30 07:51:15]

https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
File Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/1041463003/diff/1/Source/core/editing/iterato...
Source/core/editing/iterators/TextIterator.cpp:108: if (start.isNull() ||
end.isNull())
On 2015/03/30 07:40:56, hajimehoshi wrote:
> On 2015/03/30 07:34:28, haraken wrote:
> > 
> > Why do we need this check?
> > 
> > It seems the old code was calling initialize without having the check.
> 
> It is because there are some cases in which |range| is null and actually some
> tests fail without this check (see line 108 of the original file). I this this
> doesn't change the behavior and is needed for backward compatibility.

That was my misunderstanding: All editing tests passed without this check. :-(
Removed

[hajimehoshi, 2015-03-30 07:51:21]

The CQ bit was checked by hajimehoshi@chromium.org

[hajimehoshi, 2015-03-30 07:51:22]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1041463003/#ps20001
(title: "Remove null checks for positions")

[commit-bot: I haz the power, 2015-03-30 07:51:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1041463003/20001

[commit-bot: I haz the power, 2015-03-30 08:04:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-30 08:04:41]

Try jobs failed on following builders:
  mac_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)

[hajimehoshi, 2015-03-30 08:10:58]

The CQ bit was checked by hajimehoshi@chromium.org

[hajimehoshi, 2015-03-30 08:10:58]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1041463003/#ps40001
(title: "Fix WebSubstringUtil.mm")

[commit-bot: I haz the power, 2015-03-30 08:11:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1041463003/40001

[haraken, 2015-03-30 08:47:31]

It would be a good idea to add ASSERT(!start.isNull()) and
ASSERT(!start.isNull()).

[hajimehoshi, 2015-03-30 08:51:50]

The CQ bit was unchecked by hajimehoshi@chromium.org

[hajimehoshi, 2015-03-30 08:59:25]

The CQ bit was checked by hajimehoshi@chromium.org

[hajimehoshi, 2015-03-30 08:59:25]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1041463003/#ps60001
(title: "Add assertions")

[commit-bot: I haz the power, 2015-03-30 08:59:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1041463003/60001

[commit-bot: I haz the power, 2015-03-30 10:10:47]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192746