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

remoting/client/jni/jni_gl_display_handler.h

 // 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.
 
 #ifndef REMOTING_CLIENT_JNI_JNI_GL_DISPLAY_HANDLER_H_
 #define REMOTING_CLIENT_JNI_JNI_GL_DISPLAY_HANDLER_H_
 
 #include <EGL/egl.h>
 #include <jni.h>
 
@@ skipping 15 matching lines @@
 // Handles OpenGL display operations. Draws desktop and cursor on the OpenGL
 // surface.
 // JNI functions should all be called on the UI thread. The display handler
 // itself should be deleted on the display thread.
 // Please see GlDisplay.java for documentations.
 class JniGlDisplayHandler : public DisplayUpdaterFactory,
                             public protocol::CursorShapeStub,
                             public GlRendererDelegate {
  public:
   JniGlDisplayHandler(ChromotingJniRuntime* runtime);
-  ~JniGlDisplayHandler() override;
 
   // Sets the DesktopViewFactory for the Java client.
   void InitializeClient(
       const base::android::JavaRef<jobject>& java_client);
 
   // DisplayUpdaterFactory interface.
   std::unique_ptr<protocol::CursorShapeStub> CreateCursorShapeStub() override;
   std::unique_ptr<protocol::VideoRenderer> CreateVideoRenderer() override;
 
+  //  Must be called on the UI thread.
+  void Destroy() override;
+
   static bool RegisterJni(JNIEnv* env);
 
   void OnSurfaceCreated(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& caller,
       const base::android::JavaParamRef<jobject>& surface);
 
   void OnSurfaceChanged(JNIEnv* env,
                         const base::android::JavaParamRef<jobject>& caller,
                         int width,
@@ skipping 21 matching lines @@
       bool visible);
 
   void OnCursorInputFeedback(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& caller,
       float x,
       float y,
       float diameter);
 
  private:
+  ~JniGlDisplayHandler() override;
+
+  // Queues a task running on current call stack. All queued tasks will be

[Lambros, 2016/09/16 01:44:12]

This first sentence is confusing because of the phrase 'call stack'. I'm not
sure how the call stack relates to what this method does.

[Yuwei, 2016/09/19 23:52:10]

Removed.

+  // posted to the display thread after the current task is finished.
+  // Do nothing if |ui_task_poster_| has already been released.
+  void QueueTaskOnDisplayThread(const base::Closure& task);
+
+  // This avoids listing DeleteHelper as friend.

[Lambros, 2016/09/16 01:44:12]

I would rephrase this comment without 'DeleteHelper' which I guess(?) is an
implementation detail of TaskRunner?

I would say what the method does first:
"This simply calls the dtor."
Then say why:
"This is to allow TaskRunner::PostTask() to access this private dtor, instead of
using TaskRunner::DeleteSoon() which would require the dtor to be visible".

[Yuwei, 2016/09/19 23:52:10]

Obsolete.

+  void DeleteOnDisplayThread();
+
   // GlRendererDelegate interface.
   bool CanRenderFrame() override;
   void OnFrameRendered() override;
   void OnSizeChanged(int width, int height) override;
 
   // CursorShapeStub interface.
   void SetCursorShape(const protocol::CursorShapeInfo& cursor_shape) override;
 
   static void NotifyRenderDoneOnUiThread(
       base::android::ScopedJavaGlobalRef<jobject> java_display);
@@ skipping 23 matching lines @@
 
   // Used on display thread.
   base::WeakPtr<JniGlDisplayHandler> weak_ptr_;
   base::WeakPtrFactory<JniGlDisplayHandler> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(JniGlDisplayHandler);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_CLIENT_JNI_JNI_GL_DISPLAY_HANDLER_H_