Let ImeAdapterAndroid have the same lifecycle as its Java peer

content/browser/android/content_view_core_impl.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/android/content_view_core_impl.h"
 
 #include <stddef.h>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
@@ skipping 11 matching lines @@
 #include "cc/output/begin_frame_args.h"
 #include "content/browser/accessibility/browser_accessibility_manager_android.h"
 #include "content/browser/accessibility/browser_accessibility_state_impl.h"
 #include "content/browser/android/gesture_event_type.h"
 #include "content/browser/android/interstitial_page_delegate_android.h"
 #include "content/browser/android/java/gin_java_bridge_dispatcher_host.h"
 #include "content/browser/android/load_url_params.h"
 #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"
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
 #include "content/browser/web_contents/web_contents_view_android.h"
 #include "content/common/frame_messages.h"
 #include "content/common/input_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/android/compositor.h"
 #include "content/public/browser/browser_context.h"
@@ skipping 160 matching lines @@
 // static
 ContentViewCore* ContentViewCore::FromWebContents(
     content::WebContents* web_contents) {
   return ContentViewCoreImpl::FromWebContents(web_contents);
 }
 
 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());
   gfx::Size physical_size(
       Java_ContentViewCore_getPhysicalBackingWidthPix(env, obj),
       Java_ContentViewCore_getPhysicalBackingHeightPix(env, obj));
   GetViewAndroid()->GetLayer()->SetBounds(physical_size);
 
   // Currently, the only use case we have for overriding a user agent involves
   // spoofing a desktop Linux user agent for "Request desktop site".
@@ skipping 91 matching lines @@
   DCHECK(web_contents_);
   static_cast<WebContentsViewAndroid*>(
       static_cast<WebContentsImpl*>(web_contents_)->GetView())->
           SetContentViewCore(this);
   DCHECK(!web_contents_->GetUserData(kContentViewUserDataKey));
   web_contents_->SetUserData(kContentViewUserDataKey,
                              new ContentViewUserData(this));
 }
 
 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())
     Java_ContentViewCore_onRenderProcessChange(env, obj);
 
   if (device_orientation_ != 0)
     SendOrientationChangeEventInternal();
 }
 
 void ContentViewCoreImpl::RenderViewHostChanged(RenderViewHost* old_host,
@@ skipping 21 matching lines @@
     ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
     if (!obj.is_null()) {
       Java_ContentViewCore_onRenderProcessChange(env, obj);
     }
   }
 
   SetFocusInternal(HasFocus());
   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;
   if (web_contents_) {
     rwhv = web_contents_->GetRenderWidgetHostView();
     if (web_contents_->ShowingInterstitialPage()) {
       rwhv = web_contents_->GetInterstitialPage()
                  ->GetMainFrame()
                  ->GetRenderViewHost()
                  ->GetWidget()
@@ skipping 744 matching lines @@
 void ContentViewCoreImpl::WasResized(JNIEnv* env,
                                      const JavaParamRef<jobject>& obj) {
   gfx::Size physical_size(
       Java_ContentViewCore_getPhysicalBackingWidthPix(env, obj),
       Java_ContentViewCore_getPhysicalBackingHeightPix(env, obj));
   GetViewAndroid()->GetLayer()->SetBounds(physical_size);
 
   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,
                                            int selection_start,
                                            int selection_end,
                                            int composition_start,
                                            int composition_end,
                                            bool show_ime_if_needed,
                                            bool reply_to_request) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (obj.is_null())
     return;
 
   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(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     bool enabled) {
   SetAccessibilityEnabledInternal(enabled);
 }
 
 void ContentViewCoreImpl::SetTextTrackSettings(
@@ skipping 186 matching lines @@
   DCHECK(wcva);
   wcva->SetContentViewCore(NULL);
 }
 
 // This is called for each ContentView.
 jlong Init(JNIEnv* env,
            const JavaParamRef<jobject>& obj,
            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));
   CHECK(web_contents) <<
       "A ContentViewCoreImpl should be created with a valid WebContents.";
   ui::ViewAndroid* view_android = web_contents->GetView()->GetNativeView();
   view_android->SetDelegate(jview_android_delegate);
   view_android->SetLayout(ui::ViewAndroid::LayoutParams::MatchParent());
 
   ui::WindowAndroid* window_android =
         reinterpret_cast<ui::WindowAndroid*>(jwindow_android);
   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);
 }
 
 static ScopedJavaLocalRef<jobject> FromWebContentsAndroid(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz,
     const JavaParamRef<jobject>& jweb_contents) {
   WebContents* web_contents = WebContents::FromJavaWebContents(jweb_contents);
   if (!web_contents)
     return ScopedJavaLocalRef<jobject>();
 
   ContentViewCore* view = ContentViewCore::FromWebContents(web_contents);
   if (!view)
     return ScopedJavaLocalRef<jobject>();
 
   return view->GetJavaObject();
 }
 
 bool RegisterContentViewCore(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace content