[Remoting Android] JniGlDisplayHandler calls invalidate() on UI thread

remoting/client/jni/jni_gl_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_gl_display_handler.h"
 
 #include <android/native_window_jni.h>
 #include <array>
 
 #include "base/android/jni_android.h"
@@ skipping 15 matching lines @@
     : runtime_(runtime), weak_factory_(this) {
   weak_ptr_ = weak_factory_.GetWeakPtr();
   java_display_.Reset(Java_GlDisplay_createJavaDisplayObject(
       base::android::AttachCurrentThread(), reinterpret_cast<intptr_t>(this)));
   renderer_.SetDelegate(weak_ptr_);
   ui_task_poster_.reset(new QueuedTaskPoster(runtime->display_task_runner()));
 }
 
 JniGlDisplayHandler::~JniGlDisplayHandler() {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
-  Java_GlDisplay_invalidate(base::android::AttachCurrentThread(),
-                            java_display_);
-  runtime_->ui_task_runner()->DeleteSoon(FROM_HERE, ui_task_poster_.release());
+  DCHECK(!ui_task_poster_) << "Destroy() has not been called.";
 }
 
 void JniGlDisplayHandler::InitializeClient(
     const base::android::JavaRef<jobject>& java_client) {
   return Java_GlDisplay_initializeClient(base::android::AttachCurrentThread(),
                                          java_display_, java_client);
 }
 
 std::unique_ptr<protocol::CursorShapeStub>
 JniGlDisplayHandler::CreateCursorShapeStub() {
   return base::MakeUnique<CursorShapeStubProxy>(
       weak_ptr_, runtime_->display_task_runner());
 }
 
 std::unique_ptr<protocol::VideoRenderer>
 JniGlDisplayHandler::CreateVideoRenderer() {
   DCHECK(runtime_->ui_task_runner()->BelongsToCurrentThread());
   DCHECK(!frame_consumer_);
   std::unique_ptr<DualBufferFrameConsumer> consumer =
       base::MakeUnique<DualBufferFrameConsumer>(
           base::Bind(&GlRenderer::OnFrameReceived, renderer_.GetWeakPtr()),
           runtime_->display_task_runner(),
           protocol::FrameConsumer::PixelFormat::FORMAT_RGBA);
   frame_consumer_ = consumer->GetWeakPtr();
   return base::MakeUnique<SoftwareVideoRenderer>(std::move(consumer));
 }
 
+void JniGlDisplayHandler::Destroy() {
+  DCHECK(runtime_->ui_task_runner()->BelongsToCurrentThread());
+  Java_GlDisplay_invalidate(base::android::AttachCurrentThread(),
+                            java_display_);
+  ui_task_poster_.reset();

[Yuwei, 2016/09/12 21:34:44]

Releasing |ui_task_poster_| here may not be very safe for functions like
OnCursorMoved. I'll move this back to the destructor.

[Yuwei, 2016/09/13 19:06:18]

I think it's better to just null-check before using it.

+  runtime_->display_task_runner()->PostTask(
+      FROM_HERE, base::Bind(&JniGlDisplayHandler::DeleteOnDisplayThread,
+                            weak_ptr_));
+}
+
 // static
 bool JniGlDisplayHandler::RegisterJni(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 void JniGlDisplayHandler::OnSurfaceCreated(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& caller,
     const base::android::JavaParamRef<jobject>& surface) {
   DCHECK(runtime_->ui_task_runner()->BelongsToCurrentThread());
@@ skipping 59 matching lines @@
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& caller,
     float x,
     float y,
     float diameter) {
   DCHECK(runtime_->ui_task_runner()->BelongsToCurrentThread());
   ui_task_poster_->AddTask(base::Bind(&GlRenderer::OnCursorInputFeedback,
                                       renderer_.GetWeakPtr(), x, y, diameter));
 }
 
+void JniGlDisplayHandler::DeleteOnDisplayThread() {
+  delete this;
+}
+
 bool JniGlDisplayHandler::CanRenderFrame() {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
   return egl_context_ && egl_context_->IsWindowBound();
 }
 
 void JniGlDisplayHandler::OnFrameRendered() {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
   egl_context_->SwapBuffers();
   runtime_->ui_task_runner()->PostTask(
       FROM_HERE, base::Bind(&JniGlDisplayHandler::NotifyRenderDoneOnUiThread,
@@ skipping 44 matching lines @@
 // static
 void JniGlDisplayHandler::ChangeCanvasSizeOnUiThread(
     base::android::ScopedJavaGlobalRef<jobject> java_display,
     int width,
     int height) {
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_GlDisplay_changeCanvasSize(env, java_display, width, height);
 }
 
 }  // namespace remoting