[Remoting Android] DesktopView Placeholder Cleanups

Cleans up CL 2132883002 and resolves zijiehe@'s feedbacks. * Merge AbstractDesktopView.init() into the ctor of DesktopViewFactory * Pass the reference to the Java client to Java display to set up the desktop view factory instead of creating the factory in Java and passing it to Java client through JNI. Review-Url: https://codereview.chromium.org/2140773002 Cr-Commit-Position: refs/heads/master@{#404991}
Denger-Network · Jul 13, 2016 · 0441a61 · 0441a61
1 parent 199cfb0
commit 0441a61
Show file tree

Hide file tree

Showing 12 changed files with 46 additions and 63 deletions.
diff --git a/remoting/android/java/src/org/chromium/chromoting/AbstractDesktopView.java b/remoting/android/java/src/org/chromium/chromoting/AbstractDesktopView.java
@@ -8,8 +8,6 @@
 import android.graphics.Point;
 import android.view.SurfaceView;
 
-import org.chromium.chromoting.jni.Client;
-
 /**
  * Callback interface to allow the TouchInputHandler to request actions on the DesktopView.
  */
@@ -18,12 +16,6 @@ public AbstractDesktopView(Context context) {
         super(context);
     }
 
-    /**
-     * Initializes the instance. Implementations can assume this function will be called exactly
-     * once after constructor but before other functions.
-     */
-    public abstract void init(Desktop desktop, Client client);
-
     /** Triggers a brief animation to indicate the existence and location of an input event. */
     public abstract void showInputFeedback(DesktopView.InputFeedbackType feedbackToShow, Point pos);
 

diff --git a/remoting/android/java/src/org/chromium/chromoting/Desktop.java b/remoting/android/java/src/org/chromium/chromoting/Desktop.java
@@ -105,11 +105,10 @@ public void onCreate(Bundle savedInstanceState) {
         mToolbar = (Toolbar) findViewById(R.id.toolbar);
         setSupportActionBar(mToolbar);
 
-        AbstractDesktopView remoteHostDesktop = mClient.createDesktopView(this);
+        AbstractDesktopView remoteHostDesktop = mClient.createDesktopView(this, mClient);
         remoteHostDesktop.setLayoutParams(new ViewGroup.LayoutParams(
                 ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT));
         ((ViewGroup) findViewById(R.id.desktop_view_placeholder)).addView(remoteHostDesktop);
-        remoteHostDesktop.init(this, mClient);
         mSwitchToCardboardDesktopActivity = false;
 
         getSupportActionBar().setDisplayShowTitleEnabled(false);

diff --git a/remoting/android/java/src/org/chromium/chromoting/DesktopView.java b/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
@@ -39,12 +39,9 @@ public enum InputFeedbackType { NONE, SMALL_ANIMATION, LARGE_ANIMATION }
     private final TouchInputHandler mInputHandler;
 
     /** The parent Desktop activity. */
-    private Desktop mDesktop;
+    private final Desktop mDesktop;
 
-    /** The Client connection, used to inject input and fetch the video frames. */
-    private Client mClient;
-
-    private Display mDisplay;
+    private final Display mDisplay;
 
 
     // Flag to prevent multiple repaint requests from being backed up. Requests for repainting will
@@ -72,17 +69,20 @@ public enum InputFeedbackType { NONE, SMALL_ANIMATION, LARGE_ANIMATION }
     /** Whether the TouchInputHandler has requested animation to be performed. */
     private boolean mInputAnimationRunning = false;
 
-    public DesktopView(Context context, Display display) {
-        super(context);
-
+    public DesktopView(Display display, Desktop desktop, Client client) {
+        super(desktop);
         Preconditions.notNull(display);
+        Preconditions.notNull(desktop);
+        Preconditions.notNull(client);
         mDisplay = display;
+        mDesktop = desktop;
 
         // Give this view keyboard focus, allowing us to customize the soft keyboard's settings.
         setFocusableInTouchMode(true);
 
         mRenderData = new RenderData();
-        mInputHandler = new TouchInputHandler(this, context, mRenderData);
+        mInputHandler = new TouchInputHandler(this, desktop, mRenderData);
+        mInputHandler.init(desktop, new InputEventSender(client));
 
         mRepaintPending = false;
 
@@ -91,17 +91,6 @@ public DesktopView(Context context, Display display) {
         attachRedrawCallback();
     }
 
-    @Override
-    public void init(Desktop desktop, Client client) {
-        Preconditions.isNull(mDesktop);
-        Preconditions.isNull(mClient);
-        Preconditions.notNull(desktop);
-        Preconditions.notNull(client);
-        mDesktop = desktop;
-        mClient = client;
-        mInputHandler.init(desktop, new InputEventSender(client));
-    }
-
     public Event<PaintEventParameter> onPaint() {
         return mOnPaint;
     }

diff --git a/remoting/android/java/src/org/chromium/chromoting/DesktopViewFactory.java b/remoting/android/java/src/org/chromium/chromoting/DesktopViewFactory.java
@@ -4,7 +4,7 @@
 
 package org.chromium.chromoting;
 
-import android.content.Context;
+import org.chromium.chromoting.jni.Client;
 
 /**
  * Interface for creating a implementation specific desktop view.
@@ -14,5 +14,5 @@ public interface DesktopViewFactory {
      * Creates an uninitialized implementation specific desktop view. Initializing and adding the
      * view should done separately.
      */
-    AbstractDesktopView createDesktopView(Context context);
+    AbstractDesktopView createDesktopView(Desktop desktop, Client client);
 }
diff --git a/remoting/android/java/src/org/chromium/chromoting/jni/Client.java b/remoting/android/java/src/org/chromium/chromoting/jni/Client.java
@@ -11,6 +11,7 @@
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.chromoting.AbstractDesktopView;
 import org.chromium.chromoting.CapabilityManager;
+import org.chromium.chromoting.Desktop;
 import org.chromium.chromoting.DesktopViewFactory;
 import org.chromium.chromoting.InputStub;
 import org.chromium.chromoting.Preconditions;
@@ -45,20 +46,20 @@ public Client() {
     }
 
     /**
-     * Sets the desktop view factory. Called by the native code when the connection starts.
+     * Sets the desktop view factory to be used by {@link Client#createDesktopView(Context)}.
      * @param factory The factory to create implementation-dependent desktop view.
      */
-    @CalledByNative
-    private void setDesktopViewFactory(DesktopViewFactory factory) {
+    public void setDesktopViewFactory(DesktopViewFactory factory) {
         mDesktopViewFactory = factory;
     }
 
     /**
-     * Creates an implementation specific desktop view.
+     * Creates an implementation specific {@link AbstractDesktopView} using the
+     * {@link DesktopViewFactory} set by {@link Client#setDesktopViewFactory(DesktopViewFactory)}.
      */
-    public AbstractDesktopView createDesktopView(Context context) {
+    public AbstractDesktopView createDesktopView(Desktop desktop, Client client) {
         Preconditions.notNull(mDesktopViewFactory);
-        return mDesktopViewFactory.createDesktopView(context);
+        return mDesktopViewFactory.createDesktopView(desktop, client);
     }
 
     // Suppress FindBugs warning, since |sClient| is only used on the UI thread.

diff --git a/remoting/android/java/src/org/chromium/chromoting/jni/Display.java b/remoting/android/java/src/org/chromium/chromoting/jni/Display.java
@@ -4,7 +4,6 @@
 
 package org.chromium.chromoting.jni;
 
-import android.content.Context;
 import android.graphics.Bitmap;
 import android.graphics.Point;
 import android.os.Looper;
@@ -13,6 +12,7 @@
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chromoting.AbstractDesktopView;
+import org.chromium.chromoting.Desktop;
 import org.chromium.chromoting.DesktopView;
 import org.chromium.chromoting.DesktopViewFactory;
 
@@ -170,13 +170,13 @@ public Bitmap getCursorBitmap() {
     }
 
     @CalledByNative
-    private DesktopViewFactory createDesktopViewFactory() {
-        return new DesktopViewFactory() {
+    private void initializeClient(Client client) {
+        client.setDesktopViewFactory(new DesktopViewFactory() {
             @Override
-            public AbstractDesktopView createDesktopView(Context context) {
-                return new DesktopView(context, Display.this);
+            public AbstractDesktopView createDesktopView(Desktop desktop, Client client) {
+                return new DesktopView(Display.this, desktop, client);
             }
-        };
+        });
     }
 
     @CalledByNative

diff --git a/remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java b/remoting/android/java/src/org/chromium/chromoting/jni/GlDisplay.java
@@ -4,12 +4,12 @@
 
 package org.chromium.chromoting.jni;
 
-import android.content.Context;
 import android.view.Surface;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chromoting.AbstractDesktopView;
+import org.chromium.chromoting.Desktop;
 import org.chromium.chromoting.DesktopViewFactory;
 import org.chromium.chromoting.Event;
 import org.chromium.chromoting.SizeChangedEventParameter;
@@ -159,14 +159,14 @@ public void showCursorInputFeedback(int x, int y, float diameter) {
     }
 
     @CalledByNative
-    private DesktopViewFactory createDesktopViewFactory() {
-        return new DesktopViewFactory() {
+    private void initializeClient(Client client) {
+        client.setDesktopViewFactory(new DesktopViewFactory() {
             @Override
-            public AbstractDesktopView createDesktopView(Context context) {
+            public AbstractDesktopView createDesktopView(Desktop desktop, Client client) {
                 // UNIMPLEMENTED.
                 return null;
             }
-        };
+        });
     }
 
     @CalledByNative

diff --git a/remoting/client/jni/jni_client.cc b/remoting/client/jni/jni_client.cc
@@ -168,9 +168,7 @@ void JniClient::Connect(
 #else
   JniDisplayHandler* raw_display_handler = new JniDisplayHandler(runtime_);
 #endif  // defined(REMOTING_ANDROID_ENABLE_OPENGL_RENDERER)
-  Java_Client_setDesktopViewFactory(
-      env, java_client_.obj(),
-      raw_display_handler->CreateDesktopViewFactory().obj());
+  raw_display_handler->InitializeClient(java_client_);
   display_handler_.reset(raw_display_handler);
   ConnectToHost(raw_display_handler,
                 ConvertJavaStringToUTF8(env, username),

diff --git a/remoting/client/jni/jni_display_handler.cc b/remoting/client/jni/jni_display_handler.cc
@@ -67,10 +67,10 @@ JniDisplayHandler::~JniDisplayHandler() {
                           java_display_.obj());
 }
 
-base::android::ScopedJavaLocalRef<jobject>
-JniDisplayHandler::CreateDesktopViewFactory() {
-  return Java_Display_createDesktopViewFactory(
-      base::android::AttachCurrentThread(), java_display_.obj());
+void JniDisplayHandler::InitializeClient(
+    const base::android::JavaRef<jobject>& java_client) {
+  return Java_Display_initializeClient(base::android::AttachCurrentThread(),
+                                       java_display_.obj(), java_client.obj());
 }
 
 void JniDisplayHandler::UpdateCursorShape(

diff --git a/remoting/client/jni/jni_display_handler.h b/remoting/client/jni/jni_display_handler.h
@@ -30,7 +30,9 @@ class JniDisplayHandler : public DisplayUpdaterFactory {
   // Must be deleted on the display thread.
   ~JniDisplayHandler() override;
 
-  base::android::ScopedJavaLocalRef<jobject> CreateDesktopViewFactory();
+  // Sets the DesktopViewFactory for the Java client.
+  void InitializeClient(
+      const base::android::JavaRef<jobject>& java_client);
 
   void UpdateCursorShape(const protocol::CursorShapeInfo& cursor_shape);
 

diff --git a/remoting/client/jni/jni_gl_display_handler.cc b/remoting/client/jni/jni_gl_display_handler.cc
@@ -23,10 +23,11 @@ JniGlDisplayHandler::~JniGlDisplayHandler() {
                             java_display_.obj());
 }
 
-base::android::ScopedJavaLocalRef<jobject>
-JniGlDisplayHandler::CreateDesktopViewFactory() {
-  return Java_GlDisplay_createDesktopViewFactory(
-      base::android::AttachCurrentThread(), java_display_.obj());
+void JniGlDisplayHandler::InitializeClient(
+    const base::android::JavaRef<jobject>& java_client) {
+  return Java_GlDisplay_initializeClient(base::android::AttachCurrentThread(),
+                                         java_display_.obj(),
+                                         java_client.obj());
 }
 
 std::unique_ptr<protocol::CursorShapeStub>

diff --git a/remoting/client/jni/jni_gl_display_handler.h b/remoting/client/jni/jni_gl_display_handler.h
@@ -26,8 +26,9 @@ class JniGlDisplayHandler : public DisplayUpdaterFactory {
   JniGlDisplayHandler(ChromotingJniRuntime* runtime);
   ~JniGlDisplayHandler() override;
 
-  base::android::ScopedJavaLocalRef<jobject> CreateDesktopViewFactory();
-
+  // Sets the DesktopViewFactory for the Java client.
+  void InitializeClient(
+      const base::android::JavaRef<jobject>& java_client);
   // DisplayUpdaterFactory overrides.
   std::unique_ptr<protocol::CursorShapeStub> CreateCursorShapeStub() override;
   std::unique_ptr<protocol::VideoRenderer> CreateVideoRenderer() override;