New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 662908 link

Starred by 1 user
Status: Fixed
Owner:
jinsuk...@chromium.org
Closed: Feb 2018
Cc:
█ aelias@chromium.org
boliu@chromium.org
amaralp@chromium.org
changwan@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 598880



Sign in to add a comment

Move ImeAdapter out of ContentViewCore

Project Member Reported by jinsuk...@chromium.org, Nov 7 2016 
ImeAdapter object access in CVC/CVCImpl is roughly grouped into following:

1) Overall life cycle: resetAndHideKeyboard() at CVC.destroy(), WebContentObserver.renderProgressGone(), ...
2) ContainerView methods: View#attachedToWindow, #onCreateInputConnection, ..
3) Selection/Input: moveCursorToSelectionEnd()
4) Call from native: onUpdateFrameInfo, updateKeyboardVisibility

Possible refactoring approach would be move 2) to ViewAndroidDelegate, 3) to a new controller handling action mode/paste popup(see  Issue 623783 ), 4) to have RWVHA call ImeAdapterAndroid directly for these.

For 1), we need a new observer for WebContents in both Java/native. Maybe ImeAdapterAndroid can be one. cc'ing Changwan as he seems interested - please shed more light for a better direction.

Comment 1 by jinsuk...@chromium.org, Nov 7 2016

Cc: jinsuk...@chromium.org boliu@chromium.org

Comment 2 by jinsuk...@chromium.org, Nov 7 2016

Cc: -changwan@chromium.org
Owner: changwan@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by boliu@chromium.org, Nov 7 2016 

Does ImeAdapterAndroid have the same lifetime as WebContents (eg like a tab), or as RenderWidgetHostViewAndroid (which roughly maps to a particular blink WebViewImpl, and can change on navigation, or interstitials) ?

Comment 4 by jinsuk...@chromium.org, Nov 7 2016 

It appears that the lifecycle coincides with RWHVA, while that of Java object with WebContents, and they get associated with each other by the emthod attachImeAdapter().

Comment 5 by boliu@chromium.org, Nov 7 2016 

o_O sounds like maybe it should be split up to things that have different lifetimes

Comment 6 by jinsuk...@chromium.org, Mar 15 2017

Cc: -jinsuk...@chromium.org changwan@chromium.org
Owner: jinsuk...@chromium.org
Will look into this and come up with more concrete plan. This is also necessary to eliminate ContentViewClient which is now left with ime/window inset-related interface method only.

Comment 8 by jinsuk...@chromium.org, Mar 15 2017

Cc: amaralp@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/153d2811b3cbcf69cda7ab4817cdc35e7c680dee

commit 153d2811b3cbcf69cda7ab4817cdc35e7c680dee
Author: jinsukkim <jinsukkim@chromium.org>
Date: Tue Mar 28 06:22:39 2017

Let ImeAdapterAndroid have the same lifecycle as its Java peer

The native ImeAdapterAndroid(IAA) instance is owned by
RenderWidgetHostViewAndroid(RWHVA) while its Java peer
(ImeAdapter) is owned by CVC. This causes their life
cycles to be different. Attach/detach API are used to get them
linked before talking to each other.

This CL makes this mechanism simpler by having Java ImeAdapter
class create the native together, hence gets their lifetime synced.
The separate attach/detach mechanism is not necessary since
the linking is done as a part of WCVA - RWHVA association job.
i.e. IAA is linked to the current RWHVA when the corresponding
CVCImpl is set to RWHVA (in the future this will be done separately
as RWHVA will be rid of all the references to CVC).

The only situation where the linking should be done "manually"
is when IAA gets (temporarily) linked to RWHVA for an interstitial
page. The link between IAA and the main RWHVA gets restored
once the interstitial page is detached.

BUG= 662908 

Review-Url: https://codereview.chromium.org/2752113005
Cr-Commit-Position: refs/heads/master@{#460028}

[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/BUILD.gn
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/android/browser_jni_registrar.cc
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/android/content_view_core_impl.h
[rename] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/android/ime_adapter_android.cc
[rename] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/153d2811b3cbcf69cda7ab4817cdc35e7c680dee/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/63b01cc47b07f79243a4a90905c7182bcc6672e6

commit 63b01cc47b07f79243a4a90905c7182bcc6672e6
Author: mdjones <mdjones@chromium.org>
Date: Wed Mar 29 00:20:08 2017

Revert of Let ImeAdapterAndroid have the same lifecycle as its Java peer (patchset #12 id:340001 of https://codereview.chromium.org/2752113005/ )

Reason for revert:
Suspected breaking Marshmallow 64 bit tester: https://uberchromegw.corp.google.com/i/chromium.android/builders/Marshmallow%2064%20bit%20Tester/builds/10654

Original issue's description:
> Let ImeAdapterAndroid have the same lifecycle as its Java peer
>
> The native ImeAdapterAndroid(IAA) instance is owned by
> RenderWidgetHostViewAndroid(RWHVA) while its Java peer
> (ImeAdapter) is owned by CVC. This causes their life
> cycles to be different. Attach/detach API are used to get them
> linked before talking to each other.
>
> This CL makes this mechanism simpler by having Java ImeAdapter
> class create the native together, hence gets their lifetime synced.
> The separate attach/detach mechanism is not necessary since
> the linking is done as a part of WCVA - RWHVA association job.
> i.e. IAA is linked to the current RWHVA when the corresponding
> CVCImpl is set to RWHVA (in the future this will be done separately
> as RWHVA will be rid of all the references to CVC).
>
> The only situation where the linking should be done "manually"
> is when IAA gets (temporarily) linked to RWHVA for an interstitial
> page. The link between IAA and the main RWHVA gets restored
> once the interstitial page is detached.
>
> BUG= 662908 
>
> Review-Url: https://codereview.chromium.org/2752113005
> Cr-Commit-Position: refs/heads/master@{#460028}
> Committed: https://chromium.googlesource.com/chromium/src/+/153d2811b3cbcf69cda7ab4817cdc35e7c680dee

TBR=aelias@chromium.org,changwan@chromium.org,boliu@chromium.org,tedchoc@chromium.org,jinsukkim@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 662908 

Review-Url: https://codereview.chromium.org/2785543003
Cr-Commit-Position: refs/heads/master@{#460247}

[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/BUILD.gn
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/android/browser_jni_registrar.cc
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/android/content_view_core_impl.h
[rename] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/renderer_host/ime_adapter_android.cc
[rename] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/renderer_host/ime_adapter_android.h
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/63b01cc47b07f79243a4a90905c7182bcc6672e6/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 31 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d01dcc6a7955b96aff2083461319208afc2bc9ca

commit d01dcc6a7955b96aff2083461319208afc2bc9ca
Author: jinsukkim <jinsukkim@chromium.org>
Date: Fri Mar 31 04:18:54 2017

Let ImeAdapterAndroid have the same lifecycle as its Java peer

The native ImeAdapterAndroid(IAA) instance is owned by
RenderWidgetHostViewAndroid(RWHVA) while its Java peer
(ImeAdapter) is owned by CVC. This causes their life
cycles to be different. Attach/detach API are used to get them
linked before talking to each other.

This CL makes this mechanism simpler by having Java ImeAdapter
class create the native together, hence gets their lifetime synced.
The separate attach/detach mechanism is not necessary since
the native is an WebContentObserver, and connected to (or
disconnected from) RWHVA accordingly when notified of RenderView-
Host update callback. Connecting to interstitial page is also
taken care of by observing the appropriate callbacks.

BUG= 662908 

Review-Url: https://codereview.chromium.org/2752113005
Cr-Original-Commit-Position: refs/heads/master@{#460028}
Committed: https://chromium.googlesource.com/chromium/src/+/153d2811b3cbcf69cda7ab4817cdc35e7c680dee
Review-Url: https://codereview.chromium.org/2752113005
Cr-Commit-Position: refs/heads/master@{#461024}

[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/BUILD.gn
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/android/browser_jni_registrar.cc
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/android/content_view_core_impl.h
[rename] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/android/ime_adapter_android.cc
[rename] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/d01dcc6a7955b96aff2083461319208afc2bc9ca/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 31 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f7d4c6d1d466214316d4425e46363b2b94caa65

commit 3f7d4c6d1d466214316d4425e46363b2b94caa65
Author: jinsukkim <jinsukkim@chromium.org>
Date: Fri Mar 31 14:10:28 2017

Revert of Let ImeAdapterAndroid have the same lifecycle as its Java peer (patchset #13 id:360001 of https://codereview.chromium.org/2752113005/ )

Reason for revert:
Relanded but the test failure persists:

https://build.chromium.org/p/chromium.android/builders/Marshmallow%2064%20bit%20Tester/builds/10766

Original issue's description:
> Let ImeAdapterAndroid have the same lifecycle as its Java peer
>
> The native ImeAdapterAndroid(IAA) instance is owned by
> RenderWidgetHostViewAndroid(RWHVA) while its Java peer
> (ImeAdapter) is owned by CVC. This causes their life
> cycles to be different. Attach/detach API are used to get them
> linked before talking to each other.
>
> This CL makes this mechanism simpler by having Java ImeAdapter
> class create the native together, hence gets their lifetime synced.
> The separate attach/detach mechanism is not necessary since
> the native is an WebContentObserver, and connected to (or
> disconnected from) RWHVA accordingly when notified of RenderView-
> Host update callback. Connecting to interstitial page is also
> taken care of by observing the appropriate callbacks.
>
> BUG= 662908 
>
> Review-Url: https://codereview.chromium.org/2752113005
> Cr-Original-Commit-Position: refs/heads/master@{#460028}
> Committed: https://chromium.googlesource.com/chromium/src/+/153d2811b3cbcf69cda7ab4817cdc35e7c680dee
> Review-Url: https://codereview.chromium.org/2752113005
> Cr-Commit-Position: refs/heads/master@{#461024}
> Committed: https://chromium.googlesource.com/chromium/src/+/d01dcc6a7955b96aff2083461319208afc2bc9ca

TBR=aelias@chromium.org,boliu@chromium.org,changwan@chromium.org,tedchoc@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 662908 

Review-Url: https://codereview.chromium.org/2791603002
Cr-Commit-Position: refs/heads/master@{#461112}

[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/BUILD.gn
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/android/browser_jni_registrar.cc
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/android/content_view_core_impl.h
[rename] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/renderer_host/ime_adapter_android.cc
[rename] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/renderer_host/ime_adapter_android.h
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/3f7d4c6d1d466214316d4425e46363b2b94caa65/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48f82db77c454a66084b056b4b67d103a681a5e8

commit 48f82db77c454a66084b056b4b67d103a681a5e8
Author: jinsukkim <jinsukkim@chromium.org>
Date: Wed Apr 05 04:10:26 2017

Let ImeAdapterAndroid have the same lifecycle as its Java peer

The native ImeAdapterAndroid(IAA) instance is owned by
RenderWidgetHostViewAndroid(RWHVA) while its Java peer
(ImeAdapter) is owned by CVC. This causes their life
cycles to be different. Attach/detach API are used to get them
linked before talking to each other.

This CL makes this mechanism simpler by having Java ImeAdapter
class create the native together, hence gets their lifetime synced.
The separate attach/detach mechanism is not necessary since
the native is an WebContentObserver, and connected to (or
disconnected from) RWHVA accordingly when notified of RenderView-
Host update callback. Connecting to interstitial page is also
taken care of by observing the appropriate callbacks.

BUG= 662908 

Review-Url: https://codereview.chromium.org/2752113005
Cr-Original-Original-Commit-Position: refs/heads/master@{#460028}
Committed: https://chromium.googlesource.com/chromium/src/+/153d2811b3cbcf69cda7ab4817cdc35e7c680dee
Review-Url: https://codereview.chromium.org/2752113005
Cr-Original-Commit-Position: refs/heads/master@{#461024}
Committed: https://chromium.googlesource.com/chromium/src/+/d01dcc6a7955b96aff2083461319208afc2bc9ca
Review-Url: https://codereview.chromium.org/2752113005
Cr-Commit-Position: refs/heads/master@{#461975}

[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/BUILD.gn
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/android/browser_jni_registrar.cc
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/android/content_view_core_impl.h
[rename] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/android/ime_adapter_android.cc
[rename] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/48f82db77c454a66084b056b4b67d103a681a5e8/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc9e77e7d9e05da0f6017f1bdea024f0ce669786

commit cc9e77e7d9e05da0f6017f1bdea024f0ce669786
Author: jinsukkim <jinsukkim@chromium.org>
Date: Thu Apr 06 11:03:39 2017

Migrate IME state update flow

Refactored the flow for IME state update so it bypasses CVCImpl
and go straight from RWHVA -> ImeAdapter native -> Java layer.

Other related changes are:

ImeAdapter provides EventObserver to reduce the dependency on CVC,
based on the suggestion made in https://goo.gl/pdtQCl. It is used
by an embedder (Chrome) and CVC to deal with IME notification.
This replaces IME state update done through CVC. Only the necessary
info (node editability, password attribute) are passed.

ImeAdapter.ImeAdapterDelegate is to delegate some work upon IME
events. But it is used by ContentViewCore only, and there is no
clear benefit of having the interface. It was removed for
simplification. All the stuff can be (and are now) handled inside
ImeAdapter.

BUG= 662908 , 626765 , 620172 

Review-Url: https://codereview.chromium.org/2777223004
Cr-Commit-Position: refs/heads/master@{#462419}

[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/BUILD.gn
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[add] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content/browser/ViewUtils.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[add] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content_public/browser/ImeEventObserver.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[delete] https://crrev.com/e3f5c08d551b9a818dfa63874b5db137694b4889/content/public/android/javatests/src/org/chromium/content/browser/input/TestImeAdapterDelegate.java
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee89226f5c1820a579f50147cbfaf7df9462a0a2

commit ee89226f5c1820a579f50147cbfaf7df9462a0a2
Author: changwan <changwan@chromium.org>
Date: Fri Apr 07 23:42:41 2017

Revert of Migrate IME state update flow (patchset #4 id:100001 of https://codereview.chromium.org/2777223004/ )

Reason for revert:
This patchset caused a regression crbug.com/709349: 'cut' option disappeared from webview selection pop up.

Original issue's description:
> Migrate IME state update flow
>
> Refactored the flow for IME state update so it bypasses CVCImpl
> and go straight from RWHVA -> ImeAdapter native -> Java layer.
>
> Other related changes are:
>
> ImeAdapter provides EventObserver to reduce the dependency on CVC,
> based on the suggestion made in https://goo.gl/pdtQCl. It is used
> by an embedder (Chrome) and CVC to deal with IME notification.
> This replaces IME state update done through CVC. Only the necessary
> info (node editability, password attribute) are passed.
>
> ImeAdapter.ImeAdapterDelegate is to delegate some work upon IME
> events. But it is used by ContentViewCore only, and there is no
> clear benefit of having the interface. It was removed for
> simplification. All the stuff can be (and are now) handled inside
> ImeAdapter.
>
> BUG= 662908 , 626765 , 620172 
>
> Review-Url: https://codereview.chromium.org/2777223004
> Cr-Commit-Position: refs/heads/master@{#462419}
> Committed: https://chromium.googlesource.com/chromium/src/+/cc9e77e7d9e05da0f6017f1bdea024f0ce669786

TBR=boliu@chromium.org,tedchoc@chromium.org,jinsukkim@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 662908 , 626765 , 620172 

Review-Url: https://codereview.chromium.org/2803203003
Cr-Commit-Position: refs/heads/master@{#463053}

[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/BUILD.gn
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[delete] https://crrev.com/e3e4a4e437e837f6696861741dcde15693529b50/content/public/android/java/src/org/chromium/content/browser/ViewUtils.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[delete] https://crrev.com/e3e4a4e437e837f6696861741dcde15693529b50/content/public/android/java/src/org/chromium/content_public/browser/ImeEventObserver.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[add] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/javatests/src/org/chromium/content/browser/input/TestImeAdapterDelegate.java
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a

commit e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a
Author: jinsukkim <jinsukkim@chromium.org>
Date: Tue Apr 11 02:56:46 2017

Migrate IME state update flow

Refactored the flow for IME state update so it bypasses CVCImpl
and go straight from RWHVA -> ImeAdapter native -> Java layer.

Other related changes are:

ImeAdapter provides EventObserver to reduce the dependency on CVC,
based on the suggestion made in https://goo.gl/pdtQCl. It is used
by an embedder (Chrome) and CVC to deal with IME notification.
This replaces IME state update done through CVC. Only the necessary
info (node editability, password attribute) are passed.

ImeAdapter.ImeAdapterDelegate is to delegate some work upon IME
events. But it is used by ContentViewCore only, and there is no
clear benefit of having the interface. It was removed for
simplification. All the stuff can be (and are now) handled inside
ImeAdapter.

BUG= 662908 , 626765 , 620172 , 709349

Review-Url: https://codereview.chromium.org/2777223004
Cr-Original-Commit-Position: refs/heads/master@{#462419}
Committed: https://chromium.googlesource.com/chromium/src/+/cc9e77e7d9e05da0f6017f1bdea024f0ce669786
Review-Url: https://codereview.chromium.org/2777223004
Cr-Commit-Position: refs/heads/master@{#463508}

[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/BUILD.gn
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[add] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content/browser/ViewUtils.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[add] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content_public/browser/ImeEventObserver.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[delete] https://crrev.com/4cede8d39db10321b053c0d9776cf6b23f290310/content/public/android/javatests/src/org/chromium/content/browser/input/TestImeAdapterDelegate.java
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e377438d1c86ec06009e10d24a88ea84342e594

commit 7e377438d1c86ec06009e10d24a88ea84342e594
Author: jinsukkim <jinsukkim@chromium.org>
Date: Tue Apr 18 08:52:52 2017

Factor out RenderWidgetHostConnector

Factored out RenderWidgetHostConnector from ImeAdapter with an intent
to share the functionally with other classes. Upcoming SelectionPopup-
Controller will also inherit it to manage its connection to Render-
WidgetHostView.

BUG= 662908 

Review-Url: https://codereview.chromium.org/2792063003
Cr-Commit-Position: refs/heads/master@{#465180}

[modify] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/BUILD.gn
[modify] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/android/ime_adapter_android.h
[add] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/android/render_widget_host_connector.cc
[add] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/android/render_widget_host_connector.h
[add] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/android/render_widget_host_connector_browsertest.cc
[add] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/android/render_widget_host_connector_browsertest.h
[modify] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/7e377438d1c86ec06009e10d24a88ea84342e594/content/test/BUILD.gn
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 19 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64dafab9bb2755e8866dc4efa778302598dfd6ea

commit 64dafab9bb2755e8866dc4efa778302598dfd6ea
Author: jinsukkim <jinsukkim@chromium.org>
Date: Wed Apr 19 05:47:58 2017

Rename |selection_controller_| to |touch_selection_controller_|

This is a prework for upcoming SelectionPopupController in
RenderWidgetHostViewAndroid, to avoid confusion on names
of the variables.

BUG= 662908 

Review-Url: https://codereview.chromium.org/2826693005
Cr-Commit-Position: refs/heads/master@{#465503}

[modify] https://crrev.com/64dafab9bb2755e8866dc4efa778302598dfd6ea/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/64dafab9bb2755e8866dc4efa778302598dfd6ea/content/browser/renderer_host/render_widget_host_view_android.h
Project Member

Comment 19 by bugdroid1@chromium.org, Apr 26 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea157e49338a21de6168d5d5873480d298294608

commit ea157e49338a21de6168d5d5873480d298294608
Author: jinsukkim <jinsukkim@chromium.org>
Date: Wed Apr 26 07:00:28 2017

Let IME frame update bypass ContentViewCore

Decouples IME frame update (cursor/anchor coordinates) from
ContentViewCore, hence redueces the dependency on CVC. Now
RenderWidgetHostViewAndroid uses native ImeAdapter to route
the frame update info directly to Java layer.

BUG= 662908 

Review-Url: https://codereview.chromium.org/2834343005
Cr-Commit-Position: refs/heads/master@{#467257}

[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/public/android/java/src/org/chromium/content/browser/input/CursorAnchorInfoController.java
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/ea157e49338a21de6168d5d5873480d298294608/content/public/android/javatests/src/org/chromium/content/browser/input/CursorAnchorInfoControllerTest.java
Project Member

Comment 20 by bugdroid1@chromium.org, Jul 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab29b20f116d1f55e9d8ee44935191a03f117a7a

commit ab29b20f116d1f55e9d8ee44935191a03f117a7a
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Tue Jul 18 23:36:37 2017

Remove unused DEPS entry

ime_adapter_android.* is not in this directory any more.
Removing the corresponding deps entry.

BUG= 662908 

Change-Id: Ic0d61b5b0645eb4727dc699f38d2b98ae89aa88f
Reviewed-on: https://chromium-review.googlesource.com/575319
Reviewed-by: Bo Liu <boliu@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487668}
[modify] https://crrev.com/ab29b20f116d1f55e9d8ee44935191a03f117a7a/content/browser/renderer_host/DEPS

Comment 21 by jinsuk...@chromium.org, Feb 5 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment