[refactor] - Remove unused WebTextInputType from WebViewImpl.

[refactor] - Remove unused WebTextInputType from WebViewImpl.

WebViewImpl also implements WebWidget::textInputType() and, unfortunately,
until it is fully separated from WebWidget, IME related methods should be
duplicated.

BUG=629721
Committed: https://crrev.com/ad901b8d9ac484364a22be270bfe1cd6ab317b47
Cr-Commit-Position: refs/heads/master@{#417671}

[EhsanK, 2016-09-01 21:09:38]

ekaramad@chromium.org changed reviewers:
+ dglazkov@chromium.org

[EhsanK, 2016-09-01 21:09:39]

dglazkov@ Could you please review this change. It seems like this should have
happened in your earlier CL: https://codereview.chromium.org/2285923002/. Please
ignore if it is irrelevant.

This CL duplicates the removal of those types from
WebFrameWidgetImpl::textInputInfo() in WebViewImpl::textInputInfo(). Sorry for
the duplicated code (that I might have added originally) but this is intended to
be temporary until we refactor all of it into a separate place.

[dglazkov, 2016-09-02 01:11:29]

On 2016/09/01 at 21:09:39, ekaramad wrote:
> dglazkov@ Could you please review this change. It seems like this should have
happened in your earlier CL: https://codereview.chromium.org/2285923002/. Please
ignore if it is irrelevant.
> 
> This CL duplicates the removal of those types from
WebFrameWidgetImpl::textInputInfo() in WebViewImpl::textInputInfo(). Sorry for
the duplicated code (that I might have added originally) but this is intended to
be temporary until we refactor all of it into a separate place.

LGTM.

Can we have a comment somwehere explaining this refactor. Duplicated code is
super-bad. Like, really super-bad. And confusing if there's no info.

[EhsanK, 2016-09-02 04:04:39]

On 2016/09/02 01:11:29, dglazkov wrote:
> On 2016/09/01 at 21:09:39, ekaramad wrote:
> > dglazkov@ Could you please review this change. It seems like this should
have
> happened in your earlier CL: https://codereview.chromium.org/2285923002/.
Please
> ignore if it is irrelevant.
> > 
> > This CL duplicates the removal of those types from
> WebFrameWidgetImpl::textInputInfo() in WebViewImpl::textInputInfo(). Sorry for
> the duplicated code (that I might have added originally) but this is intended
to
> be temporary until we refactor all of it into a separate place.
> 
> LGTM.
> 
> Can we have a comment somwehere explaining this refactor. Duplicated code is
> super-bad. Like, really super-bad. And confusing if there's no info.

I have a bug reported when I landed the blink changes for OOPIF IME. There are
things to clean up here and I will make
the de-duplication my top priority. I will cc you on that bug.

Back to this CL...where would be the ideal location for the comment?

[EhsanK, 2016-09-02 04:09:52]

Description was changed from

==========
[refactor] - Remove unused WebTextInputType from WebViewImpl.

WebViewImpl also implements WebWidget::textInputType() and, unfortunately,
until it is fully separated from WebWidget, IME related methods should be
duplicated.

BUG=NONE
==========

to

==========
[refactor] - Remove unused WebTextInputType from WebViewImpl.

WebViewImpl also implements WebWidget::textInputType() and, unfortunately,
until it is fully separated from WebWidget, IME related methods should be
duplicated.

BUG=629721
==========

[dglazkov, 2016-09-02 15:47:03]

On 2016/09/02 at 04:04:39, ekaramad wrote:
> Back to this CL...where would be the ideal location for the comment?

I think it might be good to put it in places where we expect peeps to stumble.
Maybe a link to a doc that describes the plan. It seems to me that this
refactoring probably should've been discussed somewhere, maybe
platform-architecture-dev@?

[EhsanK, 2016-09-02 16:15:52]

On 2016/09/02 15:47:03, dglazkov wrote:
> On 2016/09/02 at 04:04:39, ekaramad wrote:
> > Back to this CL...where would be the ideal location for the comment?
> 
> I think it might be good to put it in places where we expect peeps to stumble.
> Maybe a link to a doc that describes the plan. It seems to me that this
> refactoring probably should've been discussed somewhere, maybe
> platform-architecture-dev@?
I added TODOs.

Unfortunately, there have not been such discussions AFAIK. the reason for this
duplication was implementing the blocking IME logic for out of process frames.
Since IME is handled
at WebFrameWidgetImpl of the local root, almost the exact code was copied over.

All I have is the bug that you commented on earlier. But I can go ahead and
prepare a doc soon. I just
need to identify what needs to be taken out of WebWidget into the new class.

Generally, the duplicated code is either:
1) IME method calls such as set/confirmComposition
2) IME updates such as textInputInfo(), caretOrSelectionRange(), etc.
It gets trickier for methods in category 2) as they might have other caller
users and not
specific to IME (I am not quite certain yet).

The ultimate goal is that WebViewImpl is no longer a WebWidget, so even for the
main frame we will
have a WebFrameWidgetImpl. If that was happening today then of course we did not
really need to
modify anything. So in some sense, the Text Input refactoring here is a part of
the general WebView
WebWigit split.

[EhsanK, 2016-09-02 16:16:04]

PTAL.

[dglazkov, 2016-09-02 16:19:43]

LGTM, please consider bringing this topic up with at least architecture leads.

[EhsanK, 2016-09-02 17:15:29]

ekaramad@chromium.org changed reviewers:
+ dcheng@chromium.org

[EhsanK, 2016-09-02 17:16:22]

On 2016/09/02 16:19:43, dglazkov wrote:
> LGTM, please consider bringing this topic up with at least architecture leads.

I also added dcheng@ for the record. I will run the doc with him and any
suggested
architecture leads. Thanks!

[EhsanK, 2016-09-02 20:18:09]

The CQ bit was checked by ekaramad@chromium.org

[commit-bot: I haz the power, 2016-09-02 20:18:51]

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-09-02 21:13:08]

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

[commit-bot: I haz the power, 2016-09-02 21:13:09]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dcheng, 2016-09-06 23:15:44]

LGTM

lfg is working on splitting Widget out of WebView: at that point, the
duplication will naturally go away. But it will take us some time to get there.

[EhsanK, 2016-09-08 17:15:19]

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

[EhsanK, 2016-09-08 17:15:33]

The CQ bit was unchecked by ekaramad@chromium.org

[EhsanK, 2016-09-08 17:15:33]

The CQ bit was checked by ekaramad@chromium.org

[commit-bot: I haz the power, 2016-09-08 17:15:51]

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-09-08 17:18:27]

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

[commit-bot: I haz the power, 2016-09-08 17:18:28]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[EhsanK, 2016-09-09 17:56:59]

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

[EhsanK, 2016-09-09 17:57:13]

The CQ bit was unchecked by ekaramad@chromium.org

[EhsanK, 2016-09-09 17:57:13]

The CQ bit was checked by ekaramad@chromium.org

[EhsanK, 2016-09-09 17:57:13]

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

[commit-bot: I haz the power, 2016-09-09 17:57:29]

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, 2016-09-09 19:37:08]

Description was changed from

==========
[refactor] - Remove unused WebTextInputType from WebViewImpl.

WebViewImpl also implements WebWidget::textInputType() and, unfortunately,
until it is fully separated from WebWidget, IME related methods should be
duplicated.

BUG=629721
==========

to

==========
[refactor] - Remove unused WebTextInputType from WebViewImpl.

WebViewImpl also implements WebWidget::textInputType() and, unfortunately,
until it is fully separated from WebWidget, IME related methods should be
duplicated.

BUG=629721
==========

[commit-bot: I haz the power, 2016-09-09 19:37:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-09 19:39:56]

Description was changed from

==========
[refactor] - Remove unused WebTextInputType from WebViewImpl.

WebViewImpl also implements WebWidget::textInputType() and, unfortunately,
until it is fully separated from WebWidget, IME related methods should be
duplicated.

BUG=629721
==========

to

==========
[refactor] - Remove unused WebTextInputType from WebViewImpl.

WebViewImpl also implements WebWidget::textInputType() and, unfortunately,
until it is fully separated from WebWidget, IME related methods should be
duplicated.

BUG=629721

Committed: https://crrev.com/ad901b8d9ac484364a22be270bfe1cd6ab317b47
Cr-Commit-Position: refs/heads/master@{#417671}
==========

[commit-bot: I haz the power, 2016-09-09 19:39:57]

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