[refactor] Remove WebWidget::applyReplacementRange

Remove WebWidget::applyReplacementRange

The method WebWidget::applyReplacementRange was added to accomodate the
support for IME in OOPIFs. Before, the IME logic in content/renderer used to
live in RenderViewImpl which has access to the frame tree on the renderer side.
To accomodate this transition, some methods were added to the blink
public API.

Specifically, applyReplacement range selects a range to be later replaceed by
the next call to set or commit composition. This simple task could be
part of the implementation of those methods to the newly added
WebInputMethodController instead. Moreover, the method does not do
anything special to be used outside the scope of handling the IME
events so it should not be part of the public API.

Specifically, this CL will:
  1- Remove WebWidget::applyReplacementRange which will also remove it
  from WebFrameWidgetImpl, WebViewFrameWidget, WebViewImpl.

  2- Adds a new argument, replacementRange to
  WebInputMethodController::setComposition and
  WebInputMethodController::commitText.

The benefits of this change are:
  1- Removing a layering violation inside applyReplacementRange.
  2- Further deduplicate the IME code inside WebFrameWidgetImpl and
  WebViewImpl.
  2- Help with the separation of WebWidget and WebViewImpl.
  3- Reducing some lines of code.

BUG=639320, 629721
Review-Url: https://codereview.chromium.org/2674253004
Cr-Commit-Position: refs/heads/master@{#449339}
Committed: https://chromium.googlesource.com/chromium/src/+/ce32ef9fc7230bbba732162c60cdc90f18fbbbc0

[EhsanK, 2017-02-06 19:41:13]

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

[EhsanK, 2017-02-06 19:41:39]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-02-06 19:41:48]

Dry run: 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-02-06 19:54:08]

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

[commit-bot: I haz the power, 2017-02-06 19:54:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[EhsanK, 2017-02-06 19:54:53]

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

[commit-bot: I haz the power, 2017-02-06 19:55:34]

Dry run: 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-02-06 20:05:58]

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

[commit-bot: I haz the power, 2017-02-06 20:05:59]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[EhsanK, 2017-02-06 20:20:02]

Patchset #2 (id:40001) has been deleted

[EhsanK, 2017-02-06 20:20:14]

Patchset #1 (id:20001) has been deleted

[EhsanK, 2017-02-06 20:20:25]

Description was changed from

==========
Remove WebWidget::applyReplacementRange

The method WebWidget::applyReplacementRange was added to accomodate the
support for IME in OOPIFs. Before, the IME logic in content/renderer used to
live in RenderViewImpl which has access to the frame tree on the renderer side.
To accomodate this transition, some methods were added to the blink
public API.

Specifically, applyReplacement range selects a range to be later replaceed by
the next call to set or commit composition. This simple task could be
part of the implementation of those methods to the newly added
WebInputMethodController instead. Moreover, the method does not do
anything special to be used outside the scope of handling the IME
events so it should not be part of the public API.

Specifically, this CL will:
  1- Remove WebWidget::applyReplacementRange which will also remove it
  from WebFrameWidgetImpl, WebViewFrameWidget, WebViewImpl.

  2- Adds a new argument, replacementRange to
  WebInputMethodController::setComposition and
  WebInputMethodController::commitText.

The benefits of this change are:
  1- Removing a layering violation inside applyReplacementRange.
  2- Further deduplicate the IME code inside WebFrameWidgetImpl and
  WebViewImpl.
  2- Help with the separation of WebWidget and WebViewImpl.
  3- Reducing some lines of code.

BUG=639320,629721
==========

to

==========
Remove WebWidget::applyReplacementRange

The method WebWidget::applyReplacementRange was added to accomodate the
support for IME in OOPIFs. Before, the IME logic in content/renderer used to
live in RenderViewImpl which has access to the frame tree on the renderer side.
To accomodate this transition, some methods were added to the blink
public API.

Specifically, applyReplacement range selects a range to be later replaceed by
the next call to set or commit composition. This simple task could be
part of the implementation of those methods to the newly added
WebInputMethodController instead. Moreover, the method does not do
anything special to be used outside the scope of handling the IME
events so it should not be part of the public API.

Specifically, this CL will:
  1- Remove WebWidget::applyReplacementRange which will also remove it
  from WebFrameWidgetImpl, WebViewFrameWidget, WebViewImpl.

  2- Adds a new argument, replacementRange to
  WebInputMethodController::setComposition and
  WebInputMethodController::commitText.

The benefits of this change are:
  1- Removing a layering violation inside applyReplacementRange.
  2- Further deduplicate the IME code inside WebFrameWidgetImpl and
  WebViewImpl.
  2- Help with the separation of WebWidget and WebViewImpl.
  3- Reducing some lines of code.

BUG=639320,629721
==========

[EhsanK, 2017-02-06 20:20:40]

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

[commit-bot: I haz the power, 2017-02-06 20:21:18]

Dry run: 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-02-06 22:23:59]

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

[commit-bot: I haz the power, 2017-02-06 22:23:59]

Dry run: This issue passed the CQ dry run.

[EhsanK, 2017-02-07 07:05:05]

ekaramad@chromium.org changed reviewers:
+ avi@chromium.org, dcheng@chromium.org, dglazkov@chromium.org,
rbyers@chromium.org

[EhsanK, 2017-02-07 07:05:05]

Please review this change.

dglazkov@/dcheng@: Blink changes in general.
rbyres@: component/ test file and public API.
avi@: content/ changes.

Thanks!

[dcheng, 2017-02-07 07:58:42]

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (left):

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:711: if (LocalFrame* frame
= focusedLocalFrameInWidget()) {
Is the reason it's OK to lose this call is because we're relying on the messages
being correctly routed by IPC now?

[Avi (use Gerrit), 2017-02-07 15:42:00]

content lgtm

[dglazkov, 2017-02-07 16:47:45]

LGTM, fingers crossed.

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebInputMethodControllerImpl.h (right):

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebInputMethodControllerImpl.h:34: int
selectionStart,
Would you consider switching selectionStart/selectionEnd to use WebRange as
well? Maybe not in this CL.

[EhsanK, 2017-02-07 17:14:48]

Thanks! PTAL.

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (left):

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:711: if (LocalFrame* frame
= focusedLocalFrameInWidget()) {
On 2017/02/07 07:58:42, dcheng wrote:
> Is the reason it's OK to lose this call is because we're relying on the
messages
> being correctly routed by IPC now?

Yes. The IME IPC from RenderWidget of local root ends up at the focused frame's
WebInputMethodController. So we should be fine.
Please see this:
https://cs.chromium.org/chromium/src/content/renderer/render_widget.cc?rcl=41...

And this:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/WebFrameWi...

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebInputMethodControllerImpl.h (right):

https://codereview.chromium.org/2674253004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebInputMethodControllerImpl.h:34: int
selectionStart,
On 2017/02/07 16:47:45, dglazkov wrote:
> Would you consider switching selectionStart/selectionEnd to use WebRange as
> well? Maybe not in this CL.

Yes definitely. I filed a bug and assigned it to myself.

[dcheng, 2017-02-07 18:51:55]

LGTM

[Rick Byers, 2017-02-09 01:50:12]

test_runner LGTM

[EhsanK, 2017-02-09 05:34:43]

The CQ bit was checked by ekaramad@chromium.org

[commit-bot: I haz the power, 2017-02-09 05:35:34]

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-02-09 05:55:05]

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

[commit-bot: I haz the power, 2017-02-09 05:55:06]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[EhsanK, 2017-02-09 15:25:12]

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

[commit-bot: I haz the power, 2017-02-09 15:25:31]

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

[EhsanK, 2017-02-09 16:07:16]

The CQ bit was unchecked by ekaramad@chromium.org

[EhsanK, 2017-02-09 16:07:17]

The CQ bit was checked by ekaramad@chromium.org

[EhsanK, 2017-02-09 16:07:17]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
dcheng@chromium.org, dglazkov@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/2674253004/#ps100001
(title: "Rebased")

[commit-bot: I haz the power, 2017-02-09 16:07:38]

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-02-09 17:34:00]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1486656437326400,
"parent_rev": "4fdd067a595065465d6d3a6d35a8f37c2b7d4213", "commit_rev":
"ce32ef9fc7230bbba732162c60cdc90f18fbbbc0"}

[commit-bot: I haz the power, 2017-02-09 17:34:31]

Description was changed from

==========
Remove WebWidget::applyReplacementRange

The method WebWidget::applyReplacementRange was added to accomodate the
support for IME in OOPIFs. Before, the IME logic in content/renderer used to
live in RenderViewImpl which has access to the frame tree on the renderer side.
To accomodate this transition, some methods were added to the blink
public API.

Specifically, applyReplacement range selects a range to be later replaceed by
the next call to set or commit composition. This simple task could be
part of the implementation of those methods to the newly added
WebInputMethodController instead. Moreover, the method does not do
anything special to be used outside the scope of handling the IME
events so it should not be part of the public API.

Specifically, this CL will:
  1- Remove WebWidget::applyReplacementRange which will also remove it
  from WebFrameWidgetImpl, WebViewFrameWidget, WebViewImpl.

  2- Adds a new argument, replacementRange to
  WebInputMethodController::setComposition and
  WebInputMethodController::commitText.

The benefits of this change are:
  1- Removing a layering violation inside applyReplacementRange.
  2- Further deduplicate the IME code inside WebFrameWidgetImpl and
  WebViewImpl.
  2- Help with the separation of WebWidget and WebViewImpl.
  3- Reducing some lines of code.

BUG=639320,629721
==========

to

==========
Remove WebWidget::applyReplacementRange

The method WebWidget::applyReplacementRange was added to accomodate the
support for IME in OOPIFs. Before, the IME logic in content/renderer used to
live in RenderViewImpl which has access to the frame tree on the renderer side.
To accomodate this transition, some methods were added to the blink
public API.

Specifically, applyReplacement range selects a range to be later replaceed by
the next call to set or commit composition. This simple task could be
part of the implementation of those methods to the newly added
WebInputMethodController instead. Moreover, the method does not do
anything special to be used outside the scope of handling the IME
events so it should not be part of the public API.

Specifically, this CL will:
  1- Remove WebWidget::applyReplacementRange which will also remove it
  from WebFrameWidgetImpl, WebViewFrameWidget, WebViewImpl.

  2- Adds a new argument, replacementRange to
  WebInputMethodController::setComposition and
  WebInputMethodController::commitText.

The benefits of this change are:
  1- Removing a layering violation inside applyReplacementRange.
  2- Further deduplicate the IME code inside WebFrameWidgetImpl and
  WebViewImpl.
  2- Help with the separation of WebWidget and WebViewImpl.
  3- Reducing some lines of code.

BUG=639320,629721

Review-Url: https://codereview.chromium.org/2674253004
Cr-Commit-Position: refs/heads/master@{#449339}
Committed:
https://chromium.googlesource.com/chromium/src/+/ce32ef9fc7230bbba732162c60cd...
==========

[commit-bot: I haz the power, 2017-02-09 17:34:33]

Committed patchset #3 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/ce32ef9fc7230bbba732162c60cd...