Don't leak ContentViewCores via ImeAdapter

The existing strong ref from native to java forms a GC root which
thwarts GC of ImeAdapter.java or any object referenced by it, which
includes ContentViewCore by dint of ViewEmbedder subclass being a
non-static inner class of CVC and hence binding it.
This is problematic for WebView as it requires reliable finalizer
based cleanup of CVC.

BUG=b/7697692


Review URL: https://chromiumcodereview.appspot.com/12666007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@188584 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 36 insertions, 25 deletions

diff --git a/content/browser/renderer_host/ime_adapter_android.cc b/content/browser/renderer_host/ime_adapter_android.cc
index c5c4ad8..3eab2b0 100644
--- a/content/browser/renderer_host/ime_adapter_android.cc
+++ b/content/browser/renderer_host/ime_adapter_android.cc
@@ -8,6 +8,7 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
@@ -87,16 +88,14 @@ bool RegisterImeAdapter(JNIEnv* env) {
 }
 
 ImeAdapterAndroid::ImeAdapterAndroid(RenderWidgetHostViewAndroid* rwhva)
-    : rwhva_(rwhva),
-      java_ime_adapter_(NULL) {
+    : rwhva_(rwhva) {
 }
 
 ImeAdapterAndroid::~ImeAdapterAndroid() {
-  if (java_ime_adapter_) {
-    JNIEnv* env = base::android::AttachCurrentThread();
-    Java_ImeAdapter_detach(env, java_ime_adapter_);
-    env->DeleteGlobalRef(java_ime_adapter_);
-  }
+  JNIEnv* env = AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> obj = java_ime_adapter_.get(env);
+  if (!obj.is_null())
+    Java_ImeAdapter_detach(env, obj.obj());
 }
 
 bool ImeAdapterAndroid::SendSyntheticKeyEvent(JNIEnv*,
@@ -175,11 +174,14 @@ void ImeAdapterAndroid::CommitText(JNIEnv* env, jobject, jstring text) {
 }
 
 void ImeAdapterAndroid::AttachImeAdapter(JNIEnv* env, jobject java_object) {
-  java_ime_adapter_ = AttachCurrentThread()->NewGlobalRef(java_object);
+  java_ime_adapter_ = JavaObjectWeakGlobalRef(env, java_object);
 }
 
 void ImeAdapterAndroid::CancelComposition() {
-  Java_ImeAdapter_cancelComposition(AttachCurrentThread(), java_ime_adapter_);
+  base::android::ScopedJavaLocalRef<jobject> obj =
+      java_ime_adapter_.get(AttachCurrentThread());
+  if (!obj.is_null())
+    Java_ImeAdapter_cancelComposition(AttachCurrentThread(), obj.obj());
 }
 
 void ImeAdapterAndroid::SetEditableSelectionOffsets(JNIEnv*, jobject,
diff --git a/content/browser/renderer_host/ime_adapter_android.h b/content/browser/renderer_host/ime_adapter_android.h
index a2cd2ec..f990498 100644
--- a/content/browser/renderer_host/ime_adapter_android.h
+++ b/content/browser/renderer_host/ime_adapter_android.h
@@ -7,6 +7,8 @@
 
 #include <jni.h>
 
+#include "base/android/jni_helper.h"
+
 namespace content {
 
 class RenderWidgetHostViewAndroid;
@@ -15,6 +17,8 @@ struct NativeWebKeyboardEvent;
 // This class is in charge of dispatching key events from the java side
 // and forward to renderer along with input method results via
 // corresponding host view.
+// Ownership of these objects remains on the native side (see
+// RenderWidgetHostViewAndroid).
 class ImeAdapterAndroid {
  public:
   explicit ImeAdapterAndroid(RenderWidgetHostViewAndroid* rwhva);
@@ -53,7 +57,7 @@ class ImeAdapterAndroid {
 
  private:
   RenderWidgetHostViewAndroid* rwhva_;
-  jobject java_ime_adapter_;
+  JavaObjectWeakGlobalRef java_ime_adapter_;
 };
 
 bool RegisterImeAdapter(JNIEnv* env);
diff --git a/content/public/android/java/src/org/chromium/content/browser/ImeAdapter.java b/content/public/android/java/src/org/chromium/content/browser/ImeAdapter.java
index 224f070..89bca24 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ImeAdapter.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ImeAdapter.java
@@ -24,21 +24,26 @@ import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 
 /**
- We have to adapt and plumb android IME service and chrome text input API.
- ImeAdapter provides an interface in both ways native <-> java:
- 1. InputConnectionAdapter notifies native code of text composition state and
-    dispatch key events from java -> WebKit.
- 2. Native ImeAdapter notifies java side to clear composition text.
-
- The basic flow is:
- 1. When InputConnectionAdapter gets called with composition or result text:
-    If we receive a composition text or a result text, then we just need to
-    dispatch a synthetic key event with special keycode 229, and then dispatch
-    the composition or result text.
- 2. Intercept dispatchKeyEvent() method for key events not handled by IME, we
-    need to dispatch them to webkit and check webkit's reply. Then inject a
-    new key event for further processing if webkit didn't handle it.
-*/
+ * Adapts and plumbs android IME service onto the chrome text input API.
+ * ImeAdapter provides an interface in both ways native <-> java:
+ * 1. InputConnectionAdapter notifies native code of text composition state and
+ *    dispatch key events from java -> WebKit.
+ * 2. Native ImeAdapter notifies java side to clear composition text.
+ *
+ * The basic flow is:
+ * 1. When InputConnectionAdapter gets called with composition or result text:
+ *    If we receive a composition text or a result text, then we just need to
+ *    dispatch a synthetic key event with special keycode 229, and then dispatch
+ *    the composition or result text.
+ * 2. Intercept dispatchKeyEvent() method for key events not handled by IME, we
+ *   need to dispatch them to webkit and check webkit's reply. Then inject a
+ *   new key event for further processing if webkit didn't handle it.
+ *
+ * Note that the native peer object does not take any strong reference onto the
+ * instance of this java object, hence it is up to the client of this class (e.g.
+ * the ViewEmbedder implementor) to hold a strong reference to it for the required
+ * lifetime of the object.
+ */
 @JNINamespace("content")
 class ImeAdapter {
     interface ViewEmbedder {