Avoid reporting space as NBSP in TextInputState.

Avoid reporting space as NBSP in TextInputState.

This removes a fix done at the top level Android layer and moves it into
TextIterator.  Because the text is stored away in intermediate caches
(mLastText, mCachedTextInputState), it seems more principled to do it at
emission time to avoid potential cache mismatch issues.  This change also
avoids allocating a new Java String for every update.

BUG=663880
Review-Url: https://codereview.chromium.org/2649923012
Cr-Commit-Position: refs/heads/master@{#445972}
Committed: https://chromium.googlesource.com/chromium/src/+/34f2476fb5c0fab47e6d0b99d18b99a1b6fce7be

[aelias_OOO_until_Jul13, 2017-01-24 21:07:30]

The CQ bit was checked by aelias@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 21:08:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[aelias_OOO_until_Jul13, 2017-01-24 21:08:15]

aelias@chromium.org changed reviewers:
+ yosin@chromium.org

[aelias_OOO_until_Jul13, 2017-01-24 21:08:22]

Hi yosin@, PTAL.

[commit-bot: I haz the power, 2017-01-24 21:30:05]

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

[commit-bot: I haz the power, 2017-01-24 21:30:05]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[yosin_UTC9, 2017-01-25 03:41:23]

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
(right):

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp:1178: // In
a contenteditable, multiple spaces or spaces at the edge need to be nbsp
In a contenteditable, multiple spaces or spaces at the edge need to be nbsp
.........................................^^^^^^ "a space"?

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
(right):

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp:34:
TextIteratorTextState::TextIteratorTextState(bool emitsOriginalText,
How about making |TextIteratorTextState| to take |behavor| parameter and
initialize |m_emitsOriginalText| and |m_emitsSpaceForNbsp| in
|TextIteratorTextState| constructor to avoid |bool| parameter?

If so, we also don't need to introduce |TextIterator::emitsSpaceForNbsp()|.

It is better to have another patch to pass |behavior| to |TextIteratorState|
ctor. In this way, we can omit |TextIterator| change from this patch.


Note: original code's |bool| parameter is not good too.

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h
(right):

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h:102:
bool m_emitsSpaceForNbsp;
Let's make this to |const bool| and |m_emitOriginalText| to |const bool|

[aelias_OOO_until_Jul13, 2017-01-25 04:32:42]

The CQ bit was checked by aelias@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 04:33:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[aelias_OOO_until_Jul13, 2017-01-25 04:33:29]

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp
(right):

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.cpp:34:
TextIteratorTextState::TextIteratorTextState(bool emitsOriginalText,
On 2017/01/25 at 03:41:23, Yosi_UTC9 wrote:
> How about making |TextIteratorTextState| to take |behavor| parameter and
initialize |m_emitsOriginalText| and |m_emitsSpaceForNbsp| in
|TextIteratorTextState| constructor to avoid |bool| parameter?

Done.

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h
(right):

https://codereview.chromium.org/2649923012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTextState.h:102:
bool m_emitsSpaceForNbsp;
On 2017/01/25 at 03:41:23, Yosi_UTC9 wrote:
> Let's make this to |const bool| and |m_emitOriginalText| to |const bool|

Done.

[aelias_OOO_until_Jul13, 2017-01-25 04:34:09]

https://codereview.chromium.org/2649923012/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
(right):

https://codereview.chromium.org/2649923012/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp:1178: // In
a contenteditable, multiple spaces or a space at the edge needs to be
Done.

[commit-bot: I haz the power, 2017-01-25 05:58:27]

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

[commit-bot: I haz the power, 2017-01-25 05:58:28]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-01-25 06:42:18]

lgtm

Thanks!

[yosin_UTC9, 2017-01-25 06:42:19]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2017-01-25 06:42:26]

The CQ bit was unchecked by yosin@chromium.org

[commit-bot: I haz the power, 2017-01-25 06:42:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yosin_UTC9, 2017-01-25 06:42:32]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2017-01-25 06:42:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-25 06:46:16]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485326552436640,
"parent_rev": "b739bdcc12b1d4da4e614fe153cb05cd3b9e6104", "commit_rev":
"34f2476fb5c0fab47e6d0b99d18b99a1b6fce7be"}

[commit-bot: I haz the power, 2017-01-25 06:46:46]

Description was changed from

==========
Avoid reporting space as NBSP in TextInputState.

This removes a fix done at the top level Android layer and moves it into
TextIterator.  Because the text is stored away in intermediate caches
(mLastText, mCachedTextInputState), it seems more principled to do it at
emission time to avoid potential cache mismatch issues.  This change also
avoids allocating a new Java String for every update.

BUG=663880
==========

to

==========
Avoid reporting space as NBSP in TextInputState.

This removes a fix done at the top level Android layer and moves it into
TextIterator.  Because the text is stored away in intermediate caches
(mLastText, mCachedTextInputState), it seems more principled to do it at
emission time to avoid potential cache mismatch issues.  This change also
avoids allocating a new Java String for every update.

BUG=663880

Review-Url: https://codereview.chromium.org/2649923012
Cr-Commit-Position: refs/heads/master@{#445972}
Committed:
https://chromium.googlesource.com/chromium/src/+/34f2476fb5c0fab47e6d0b99d18b...
==========

[commit-bot: I haz the power, 2017-01-25 06:46:47]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/34f2476fb5c0fab47e6d0b99d18b...