Don't leak ContentViewCores via ImeAdapter

content/browser/renderer_host/ime_adapter_android.cc

 // Copyright (c) 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/renderer_host/ime_adapter_android.h"
 
 #include <android/input.h>
 
 #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"
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "jni/ImeAdapter_jni.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebCompositionUnderline.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 
@@ skipping 59 matching lines @@
       ui::TEXT_INPUT_TYPE_DATE_TIME,
       ui::TEXT_INPUT_TYPE_DATE_TIME_LOCAL,
       ui::TEXT_INPUT_TYPE_MONTH,
       ui::TEXT_INPUT_TYPE_TIME,
       ui::TEXT_INPUT_TYPE_WEEK,
       ui::TEXT_INPUT_TYPE_CONTENT_EDITABLE);
   return true;
 }
 
 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*,
                                               jobject,
                                               int type,
                                               long time_ms,
                                               int key_code,
                                               int text) {
   NativeWebKeyboardEvent event(static_cast<WebKit::WebInputEvent::Type>(type),
                                0 /* modifiers */, time_ms / 1000.0, key_code,
@@ skipping 58 matching lines @@
   RenderWidgetHostImpl* rwhi = RenderWidgetHostImpl::From(
       rwhva_->GetRenderWidgetHost());
   if (!rwhi)
     return;
 
   string16 text16 = ConvertJavaStringToUTF16(env, text);
   rwhi->ImeConfirmComposition(text16);
 }
 
 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 =

[Kristian Monsen, 2013/03/14 22:42:38]

maybe slightly more expressive name than obj? Also above.

[joth, 2013/03/15 02:39:45]

yeah.. obj seems to be a fairly common name for "this object over on the java
side, as a local", see aw_content.cc. Short name is handy as it all gets very
verbose else.
But I'm not opposed to changing the pattern if there's something particularly
better

+      java_ime_adapter_.get(AttachCurrentThread());
+  if (!obj.is_null())
+    Java_ImeAdapter_cancelComposition(AttachCurrentThread(), obj.obj());
 }
 
 void ImeAdapterAndroid::SetEditableSelectionOffsets(JNIEnv*, jobject,
                                                     int start, int end) {
   RenderWidgetHostImpl* rwhi = RenderWidgetHostImpl::From(
       rwhva_->GetRenderWidgetHost());
   if (!rwhi)
     return;
 
   rwhi->Send(new ViewMsg_SetEditableSelectionOffsets(rwhi->GetRoutingID(),
@@ skipping 65 matching lines @@
 void ImeAdapterAndroid::Paste(JNIEnv* env, jobject) {
   RenderWidgetHostImpl* rwhi = RenderWidgetHostImpl::From(
       rwhva_->GetRenderWidgetHost());
   if (!rwhi)
     return;
 
   rwhi->Send(new ViewMsg_Paste(rwhi->GetRoutingID()));
 }
 
 }  // namespace content