[Remoting Android] Make JniClient own JniDisplayHandler

remoting/client/jni/jni_display_handler.cc

 // Copyright 2016 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 "remoting/client/jni/jni_display_handler.h"
 
 #include "base/android/jni_android.h"
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
@@ skipping 35 matching lines @@
 
 void DisplayCursorShapeStub::SetCursorShape(
     const protocol::CursorShapeInfo& cursor_shape) {
   task_runner_->PostTask(FROM_HERE,
                          base::Bind(&JniDisplayHandler::UpdateCursorShape,
                                     display_handler_,
                                     cursor_shape));
 }
 
 // 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(
     const protocol::CursorShapeInfo& cursor_shape) {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
 
   // const_cast<> is safe as long as the Java updateCursorShape() method copies
   // the data out of the buffer without mutating it, and doesn't keep any
   // reference to the buffer afterwards. Unfortunately, there seems to be no way
   // to create a read-only ByteBuffer from a pointer-to-const.
@@ skipping 42 matching lines @@
 
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_Display_redrawGraphicsInternal(env, java_display_.obj());
 }
 
 // static
 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) {
   runtime_->display_task_runner()->PostTask(
       FROM_HERE, base::Bind(&JniDisplayHandler::RedrawCanvas,
                             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