[Remoting Android] Refactor GlDesktopView

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

Index: remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java b/remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java
index dedbbb4e0361a6c33121c0b22a15f8cd5dc2cdb8..e7dae04a790e3644e16b3b9f558f0af6eb526bcf 100644
--- a/remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java
+++ b/remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java
@@ -11,18 +11,15 @@ 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.
@@ -30,13 +27,13 @@ import org.chromium.chromoting.SizeChangedEventParameter;
  */
 @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;
 
@@ -90,6 +87,7 @@ public class GlDisplay implements SurfaceHolder.Callback, RenderStub {
 
     @Override
     public void setDesktopView(DesktopView view) {
+        view.getHolder().addCallback(this);
         mFeedbackRadiusMapper = new InputFeedbackRadiusMapper(view);
     }
 
@@ -179,12 +177,7 @@ public class GlDisplay implements SurfaceHolder.Callback, RenderStub {
 
     @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