Change InputMethodController#setSelectionOffsets() to use NotUserTriggered parameter

Change InputMethodController#setSelectionOffsets() to use
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side
effect other than changing the selection. But
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text when inserting line break. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840
Committed: https://crrev.com/786070a33bcfebaff78b304401a4137e8dadccd8
Cr-Commit-Position: refs/heads/master@{#410577}

[yabinh, 2016-08-03 06:20:16]

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

[commit-bot: I haz the power, 2016-08-03 06:20:31]

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

[yabinh, 2016-08-03 06:27:00]

yabinh@chromium.org changed reviewers:
+ changwan@chromium.org, yosin@chromium.org

[yabinh, 2016-08-03 06:27:01]

changwan@, can you take a look at this patch?

https://codereview.chromium.org/2206923002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/InputMethodController.cpp (left):

https://codereview.chromium.org/2206923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/InputMethodController.cpp:502: 
It uses |CloseTyping| here.

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

https://codereview.chromium.org/2206923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/InputMethodController.cpp:345: if
(hasComposition()) {
If there is composing text, it will be confirmed.

https://codereview.chromium.org/2206923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/InputMethodController.cpp:352: 
We don't need to change here, because there is no composing text here.

[commit-bot: I haz the power, 2016-08-03 06:28:04]

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

[commit-bot: I haz the power, 2016-08-03 06:28:05]

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

[Changwan Ryu, 2016-08-03 06:50:50]

changwan@chromium.org changed reviewers:
- yosin@chromium.org

[Changwan Ryu, 2016-08-03 06:50:50]

Removing yosin@ until this CL gets stabilized...

yabinh@, as we talked offline, please check if we can just change CloseTyping
back to NotUserTriggered or pass it as a parameter first.

[yabinh, 2016-08-04 03:20:18]

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

[commit-bot: I haz the power, 2016-08-04 03:20:36]

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

[yabinh, 2016-08-04 03:21:37]

On 2016/08/03 06:50:50, Changwan Ryu wrote:
> Removing yosin@ until this CL gets stabilized...
> 
> yabinh@, as we talked offline, please check if we can just change CloseTyping
> back to NotUserTriggered or pass it as a parameter first.

Yes. We can simply change InputMethodController#setSelectionOffsets to use
|NotUserTriggered| without side effect.
Please check patch set 2.

[commit-bot: I haz the power, 2016-08-04 04:45:16]

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

[commit-bot: I haz the power, 2016-08-04 04:45:17]

Dry run: This issue passed the CQ dry run.

[Changwan Ryu, 2016-08-08 00:01:31]

On 2016/08/04 03:21:37, yabinh wrote:
> On 2016/08/03 06:50:50, Changwan Ryu wrote:
> > Removing yosin@ until this CL gets stabilized...
> > 
> > yabinh@, as we talked offline, please check if we can just change
CloseTyping
> > back to NotUserTriggered or pass it as a parameter first.
> 
> Yes. We can simply change InputMethodController#setSelectionOffsets to use
> |NotUserTriggered| without side effect.
> Please check patch set 2.

Hmm... Actually it wasn't changed by your CL, so it's not changing anything
back.
Could you find a better explanation for this change and use it in your commit
message?

[yabinh, 2016-08-08 01:54:40]

Description was changed from

==========
Fix ImeTest#testCommitEnterKeyWhileComposingText failure when ImeThread is
enabled

In InputMethodController#setComposition, when we call
InputMethodController#setComposition, we should use |NotUserTriggered|.
But in the CL (https://codereview.chromium.org/2020973002/), we replaced
it with |CloseTyping|. That's the reason why we couldn't cancel the
composing text. We need to change it back.

BUG=633840
==========

to

==========
Change InputMethodController#setSelectionOffsets() to use NotUserTriggered
parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping can change the editing state, causing
the failure of canceling the composing text in some cases. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

[yabinh, 2016-08-08 01:56:10]

On 2016/08/08 00:01:31, Changwan Ryu wrote:
> Hmm... Actually it wasn't changed by your CL, so it's not changing anything
> back.
> Could you find a better explanation for this change and use it in your commit
> message?

The explanation has been changed. PTAL.

[yabinh, 2016-08-08 03:33:03]

Description was changed from

==========
Change InputMethodController#setSelectionOffsets() to use NotUserTriggered
parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping can change the editing state, causing
the failure of canceling the composing text in some cases. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

to

==========
Change InputMethodController#setSelectionOffsets() to use 
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text in some cases. We should use 
NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

[Changwan Ryu, 2016-08-08 05:00:37]

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

[Changwan Ryu, 2016-08-08 05:00:37]

yosin@, could you take look?

[yosin_UTC9, 2016-08-08 06:08:32]

On 2016/08/08 at 05:00:37, changwan wrote:
> yosin@, could you take look?

Change is reasonable. Could you add a test case for this?

[yabinh, 2016-08-09 02:11:35]

Description was changed from

==========
Change InputMethodController#setSelectionOffsets() to use 
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text in some cases. We should use 
NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

to

==========
Change InputMethodController#setSelectionOffsets() to use 
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text when inserting line break. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

[yabinh, 2016-08-09 02:13:07]

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

[commit-bot: I haz the power, 2016-08-09 02:14:08]

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

[yabinh, 2016-08-09 02:14:42]

> Change is reasonable. Could you add a test case for this?

yosin@, can you take a look at this patch? Thanks.

[commit-bot: I haz the power, 2016-08-09 03:38:46]

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

[commit-bot: I haz the power, 2016-08-09 03:38:47]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-08-09 04:37:53]

lgtm

Thanks for adding tests!

[yabinh, 2016-08-09 04:50:44]

The CQ bit was checked by yabinh@chromium.org

[yabinh, 2016-08-09 04:53:40]

The CQ bit was unchecked by yabinh@chromium.org

[yabinh, 2016-08-09 04:53:40]

The CQ bit was checked by yabinh@chromium.org

[commit-bot: I haz the power, 2016-08-09 04:54:12]

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, 2016-08-09 04:59:18]

Description was changed from

==========
Change InputMethodController#setSelectionOffsets() to use 
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text when inserting line break. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

to

==========
Change InputMethodController#setSelectionOffsets() to use 
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text when inserting line break. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

[commit-bot: I haz the power, 2016-08-09 04:59:19]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-09 05:02:06]

Description was changed from

==========
Change InputMethodController#setSelectionOffsets() to use 
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side 
effect other than changing the selection. But 
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text when inserting line break. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840
==========

to

==========
Change InputMethodController#setSelectionOffsets() to use
NotUserTriggered parameter

InputMethodController#setSelectionOffsets() should have no side
effect other than changing the selection. But
FrameSelection::CloseTyping will close typing, causing the failure of
canceling the composing text when inserting line break. We should
use NotUserTriggered parameter because it has no side effect.

BUG=633840

Committed: https://crrev.com/786070a33bcfebaff78b304401a4137e8dadccd8
Cr-Commit-Position: refs/heads/master@{#410577}
==========

[commit-bot: I haz the power, 2016-08-09 05:02:07]

Patchset 3 (id:??) landed as
https://crrev.com/786070a33bcfebaff78b304401a4137e8dadccd8
Cr-Commit-Position: refs/heads/master@{#410577}

[yabinh, 2016-08-10 00:47:49]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2227333002/ by yabinh@chromium.org.

The reason for reverting is: ImeTest#testFinishComposingText failed.
We need to fix the test before relanding it..