Let ImeAdapterAndroid have the same lifecycle as its Java peer

content/browser/android/content_view_core_impl.cc

Index: content/browser/android/content_view_core_impl.cc
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
index 77e7ab21d9e105d10ec7a6a437e25efc27bd1061..7dbc2779dd4069768116331b5d81b572f2d02c8a 100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -29,6 +29,7 @@
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/media/media_web_contents_observer.h"
 #include "content/browser/renderer_host/compositor_impl_android.h"
+#include "content/browser/renderer_host/ime_adapter_android.h"
 #include "content/browser/renderer_host/input/web_input_event_builders_android.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
@@ -209,13 +210,16 @@ ContentViewCoreImpl::ContentViewCoreImpl(
     JNIEnv* env,
     const JavaRef<jobject>& obj,
     WebContents* web_contents,
+    ImeAdapterAndroid* ime_adapter,
     float dpi_scale,
     const JavaRef<jobject>& java_bridge_retained_object_set)
     : WebContentsObserver(web_contents),
       java_ref_(env, obj),
       web_contents_(static_cast<WebContentsImpl*>(web_contents)),
+      ime_adapter_(ime_adapter),

[boliu, 2017/03/24 17:13:16]

this is backwards

If CVC owns ime adapter, then CVC, then CVC should be the thing that creates it
and hands out pointers to it

Actually, maybe should seriously think about pulling ime out of CVC entirely.
Native adapter can be a WebContentsUserData, and native can "own" java.

[Jinsuk Kim, 2017/03/27 03:19:33]

Assuming that 'creator should be owner' should hold true, it is a bit tricky for
the native to create own Java. Java IA needs to be created in java CVC as it is
tightly linked to it (dependent on many CVC internal states).

In the new patch I'm destroying the IAA in
WebContentsObserver::WebContentsDestroyed. See if that looks better. 

       page_scale_(1),
       dpi_scale_(dpi_scale),
+      active_rwhva_(nullptr),
       device_orientation_(0),
       accessibility_enabled_(false) {
   GetViewAndroid()->SetLayer(cc::Layer::Create());
@@ -327,6 +331,7 @@ void ContentViewCoreImpl::InitWebContents() {
 }
 
 void ContentViewCoreImpl::RenderViewReady() {
+  active_rwhva_ = GetRenderWidgetHostViewAndroid();

[boliu, 2017/03/24 17:13:16]

need to update this in RenderViewHostChanged as well

[Jinsuk Kim, 2017/03/27 03:19:33]

Done.

   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (!obj.is_null())
@@ -368,6 +373,19 @@ void ContentViewCoreImpl::RenderViewHostChanged(RenderViewHost* old_host,
   SetAccessibilityEnabledInternal(accessibility_enabled_);
 }
 
+void ContentViewCoreImpl::DidAttachInterstitialPage() {

[boliu, 2017/03/24 17:13:16]

should this stuff live in the ImeAdapter instead? I thought we want to remove
stuff from CVC..

[Jinsuk Kim, 2017/03/27 03:19:33]

Nice. IAA being WebContentsObserver helps decouple itself from CVCImpl. Done.

+  if (active_rwhva_)
+    active_rwhva_->ConnectImeAdapter(nullptr);
+  active_rwhva_ = GetRenderWidgetHostViewAndroid();
+}
+
+void ContentViewCoreImpl::DidDetachInterstitialPage() {
+  RenderWidgetHostViewAndroid* rwhv = GetRenderWidgetHostViewAndroid();
+  if (rwhv)
+    rwhv->ConnectImeAdapter(ime_adapter());
+  active_rwhva_ = rwhv;
+}
+
 RenderWidgetHostViewAndroid*
     ContentViewCoreImpl::GetRenderWidgetHostViewAndroid() const {
   RenderWidgetHostView* rwhv = NULL;
@@ -1132,25 +1150,7 @@ void ContentViewCoreImpl::WasResized(JNIEnv* env,
   SendScreenRectsAndResizeWidget();
 }
 
-long ContentViewCoreImpl::GetNativeImeAdapter(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& obj) {
-  RenderWidgetHostViewAndroid* rwhva = GetRenderWidgetHostViewAndroid();
-  if (!rwhva)
-    return 0;
-  return rwhva->GetNativeImeAdapter();
-}
-
-void ContentViewCoreImpl::ForceUpdateImeAdapter(long native_ime_adapter) {
-  JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
-  if (obj.is_null())
-    return;
-  Java_ContentViewCore_forceUpdateImeAdapter(env, obj, native_ime_adapter);
-}
-
-void ContentViewCoreImpl::UpdateImeAdapter(long native_ime_adapter,
-                                           int text_input_type,
+void ContentViewCoreImpl::UpdateImeAdapter(int text_input_type,
                                            int text_input_flags,
                                            int text_input_mode,
                                            const std::string& text,
@@ -1167,9 +1167,9 @@ void ContentViewCoreImpl::UpdateImeAdapter(long native_ime_adapter,
 
   ScopedJavaLocalRef<jstring> jstring_text = ConvertUTF8ToJavaString(env, text);
   Java_ContentViewCore_updateImeAdapter(
-      env, obj, native_ime_adapter, text_input_type, text_input_flags,
-      text_input_mode, jstring_text, selection_start, selection_end,
-      composition_start, composition_end, show_ime_if_needed, reply_to_request);
+      env, obj, text_input_type, text_input_flags, text_input_mode,
+      jstring_text, selection_start, selection_end, composition_start,
+      composition_end, show_ime_if_needed, reply_to_request);
 }
 
 void ContentViewCoreImpl::SetAccessibilityEnabled(
@@ -1376,7 +1376,8 @@ jlong Init(JNIEnv* env,
            const JavaParamRef<jobject>& jweb_contents,
            const JavaParamRef<jobject>& jview_android_delegate,
            jlong jwindow_android,
-           jfloat dipScale,
+           jlong jime_adapter,
+           jfloat dip_scale,
            const JavaParamRef<jobject>& retained_objects_set) {
   WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(
       WebContents::FromJavaWebContents(jweb_contents));
@@ -1391,9 +1392,10 @@ jlong Init(JNIEnv* env,
   DCHECK(window_android);
   window_android->AddChild(view_android);
 
-  // TODO: pass dipScale.
   ContentViewCoreImpl* view = new ContentViewCoreImpl(
-      env, obj, web_contents, dipScale, retained_objects_set);
+      env, obj, web_contents,
+      reinterpret_cast<ImeAdapterAndroid*>(jime_adapter), dip_scale,
+      retained_objects_set);
   return reinterpret_cast<intptr_t>(view);
 }