[Remoting Android] Make JniClient own JniDisplayHandler

remoting/client/jni/jni_display_handler.cc

Index: remoting/client/jni/jni_display_handler.cc
diff --git a/remoting/client/jni/jni_display_handler.cc b/remoting/client/jni/jni_display_handler.cc
index b699461e800a7585be9c90817bad05ebff8d9fe4..16a4a46719e99cca5825c3bb70562cd48a8cf6f1 100644
--- a/remoting/client/jni/jni_display_handler.cc
+++ b/remoting/client/jni/jni_display_handler.cc
@@ -53,15 +53,22 @@ void DisplayCursorShapeStub::SetCursorShape(
 }
 
 // JniDisplayHandler definitions.
-JniDisplayHandler::JniDisplayHandler(
-    ChromotingJniRuntime* runtime,
-    base::android::ScopedJavaGlobalRef<jobject> java_display)
+JniDisplayHandler::JniDisplayHandler(ChromotingJniRuntime* runtime)
     : runtime_(runtime),
-      java_display_(java_display),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  java_display_.Reset(Java_Display_createJavaDisplayObject(
+      env, reinterpret_cast<intptr_t>(this)));
+}
 
 JniDisplayHandler::~JniDisplayHandler() {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
+  Java_Display_invalidate(base::android::AttachCurrentThread(),
+                          java_display_.obj());
+}
+
+base::android::ScopedJavaLocalRef<jobject> JniDisplayHandler::GetJavaDisplay() {
+  return base::android::ScopedJavaLocalRef<jobject>(java_display_);

[Lambros, 2016/06/27 23:51:44]

I'm not sure if this is correct? ScopedJavaLocalRef(foo) takes ownership of foo,
if I'm not mistaken? Looking at the implementation of the ctor
ScopedJavaLocalRef(const U& other), it looks like it takes ownership of
other.obj().

[Sergey Ulanov, 2016/06/27 23:59:18]

It's a java object, so it cannot be owned. This line just creates a new
reference to the same object.

I think it can be replaced with 
  return java display_;

The constructor should be called implicitly.

[Yuwei, 2016/06/28 00:03:26]

It will eventually call SetNewLocalRef(env, other.obj()) but that should be
creating a new local reference to other.obj(). I don't think you can take
ownership of a jobject?

What JniDisplayHandler does is just to call Display.invalidate() inside its
destructor. The Java Display will be GC'ed when no reference remains...

 }
 
 void JniDisplayHandler::UpdateCursorShape(
@@ -124,16 +131,6 @@ bool JniDisplayHandler::RegisterJni(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
-void JniDisplayHandler::Destroy(
-    JNIEnv* env,
-    const base::android::JavaParamRef<jobject>& caller) {
-  if (runtime_->display_task_runner()->BelongsToCurrentThread()) {
-    delete this;
-  } else {
-    runtime_->display_task_runner()->DeleteSoon(FROM_HERE, this);
-  }
-}
-
 void JniDisplayHandler::ScheduleRedraw(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& caller) {
@@ -142,10 +139,4 @@ void JniDisplayHandler::ScheduleRedraw(
                             weak_factory_.GetWeakPtr()));
 }
 
-static jlong Init(JNIEnv* env, const JavaParamRef<jobject>& caller) {
-  return reinterpret_cast<intptr_t>(new JniDisplayHandler(
-      ChromotingJniRuntime::GetInstance(),
-      base::android::ScopedJavaGlobalRef<jobject>(env, caller)));
-}
-
 }  // namespace remoting