[Remoting Android] Refactor GlDesktopView

remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java

 // 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.
 
 package org.chromium.chromoting.jni;
 
 import android.graphics.Matrix;
 import android.graphics.PointF;
 import android.view.Surface;
 import android.view.SurfaceHolder;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
-import org.chromium.chromoting.Desktop;
 import org.chromium.chromoting.DesktopView;
-import org.chromium.chromoting.DesktopViewFactory;
 import org.chromium.chromoting.Event;
-import org.chromium.chromoting.GlDesktopView;
 import org.chromium.chromoting.InputFeedbackRadiusMapper;
 import org.chromium.chromoting.RenderStub;
 import org.chromium.chromoting.SizeChangedEventParameter;
 
 /**
- * This class is the JNI interface class that helps bridging GlDesktopView with the OpenGL renderer
- * in native code. The lifetime of this class is managed by the native JniGlDisplayHandler.
+ * This class is a RenderStub implementation that uses the OpenGL renderer in native code to render
+ * the remote desktop. The lifetime of this class is managed by the native JniGlDisplayHandler.
  *
  * This class works entirely on the UI thread:
  *  Functions should all be called on UI.
  *  Events will only be triggered on UI.
  */
 @JNINamespace("remoting")
 public class GlDisplay implements SurfaceHolder.Callback, RenderStub {
-    private volatile long mNativeJniGlDisplay;
     private final Event.Raisable<SizeChangedEventParameter> mOnClientSizeChanged =
             new Event.Raisable<>();
     private final Event.Raisable<SizeChangedEventParameter> mOnHostSizeChanged =
             new Event.Raisable<>();
-    private final Event.Raisable<Void> mOnCanvasRendered =
-            new Event.Raisable<>();
+    private final Event.Raisable<Void> mOnCanvasRendered = new Event.Raisable<>();
+
+    private volatile long mNativeJniGlDisplay;

[joedow, 2016/09/12 19:55:08]

Can you initialize |mNativeJniGlDisplay| to a reasonable default?

[Yuwei, 2016/09/12 20:57:18]

It used to be 0 but Zijie thought it is unnecessary since long will be
initialized to 0 by default...

[joedow, 2016/09/12 21:01:56]

It isn't necessary but it documents intent.  Probably just a style preference. 
You can ignore this if you want.

[Hzj_jie, 2016/09/12 21:28:04]

Oh, I saw the initialization and definition of most of variables are separately
in our source code. So I added this comment before.
But I do suggest to initialize variables in constructor instead of definitions,
to avoid any potential reinitialization or initialization ordering issues.

I do not have strong opinion on this. Feel free to add or ignore it.

[Yuwei, 2016/09/12 22:17:58]

```
private GlDisplay(long nativeJniGlDisplay) {
    mNativeJniGlDisplay = nativeJniGlDisplay;
}
```

Given this ctor probably we don't need to change anything...

     private InputFeedbackRadiusMapper mFeedbackRadiusMapper;
     private float mScaleFactor = 0;
 
     private GlDisplay(long nativeJniGlDisplay) {
         mNativeJniGlDisplay = nativeJniGlDisplay;
     }
 
     /**
      * Invalidates this object and disconnects from the native display handler. Called on the
      * display thread by the native code.
@@ skipping 33 matching lines @@
      */
     @Override
     public void surfaceDestroyed(SurfaceHolder holder) {
         if (mNativeJniGlDisplay != 0) {
             nativeOnSurfaceDestroyed(mNativeJniGlDisplay);
         }
     }
 
     @Override
     public void setDesktopView(DesktopView view) {
+        view.getHolder().addCallback(this);
         mFeedbackRadiusMapper = new InputFeedbackRadiusMapper(view);
     }
 
     /**
      * Sets the transformation matrix (in pixel coordinates).
      * @param matrix the transformation matrix
      */
     @Override
     public void setTransformation(Matrix matrix) {
         if (mNativeJniGlDisplay != 0) {
@@ skipping 69 matching lines @@
      * Called by native code when a render request has been done by the OpenGL renderer. This
      * will only be called when the render event callback is enabled.
      */
     @CalledByNative
     private void canvasRendered() {
         mOnCanvasRendered.raise(null);
     }
 
     @CalledByNative
     private void initializeClient(Client client) {
-        client.setDesktopViewFactory(new DesktopViewFactory() {
-            @Override
-            public DesktopView createDesktopView(Desktop desktop, Client client) {
-                return new GlDesktopView(GlDisplay.this, desktop, client);
-            }
-        });
+        client.setRenderStub(this);
     }
 
     @CalledByNative
     private static GlDisplay createJavaDisplayObject(long nativeDisplayHandler) {
         return new GlDisplay(nativeDisplayHandler);
     }
 
     private native void nativeOnSurfaceCreated(long nativeJniGlDisplayHandler, Surface surface);
     private native void nativeOnSurfaceChanged(long nativeJniGlDisplayHandler,
                                                int width, int height);
     private native void nativeOnSurfaceDestroyed(long nativeJniGlDisplayHandler);
     private native void nativeOnPixelTransformationChanged(long nativeJniGlDisplayHandler,
                                                            float[] matrix);
     private native void nativeOnCursorPixelPositionChanged(long nativeJniGlDisplayHandler,
                                                            float x, float y);
     private native void nativeOnCursorInputFeedback(long nativeJniGlDisplayHandler,
                                                     float x, float y, float diameter);
     private native void nativeOnCursorVisibilityChanged(long nativeJniGlDisplayHandler,
                                                         boolean visible);
 }