diff --git a/remoting/android/java/src/org/chromium/chromoting/DesktopView.java b/remoting/android/java/src/org/chromium/chromoting/DesktopView.java index fad53eb8d98eb2..70940ce8f91a9b 100644 --- a/remoting/android/java/src/org/chromium/chromoting/DesktopView.java +++ b/remoting/android/java/src/org/chromium/chromoting/DesktopView.java @@ -33,7 +33,7 @@ * multitouch pan and zoom gestures, and collects and forwards input events. */ /** GUI element that holds the drawing canvas. */ -public class DesktopView extends SurfaceView implements DesktopViewInterface, Runnable, +public class DesktopView extends SurfaceView implements DesktopViewInterface, SurfaceHolder.Callback { private RenderData mRenderData; private TouchInputHandler mInputHandler; @@ -44,6 +44,11 @@ public class DesktopView extends SurfaceView implements DesktopViewInterface, Ru // thread, so the access should be synchronized on |mRenderData|. private boolean mRepaintPending; + // Flag used to ensure that the SurfaceView is only painted between calls to surfaceCreated() + // and surfaceDestroyed(). Accessed on main thread and display thread, so this should be + // synchronized on |mRenderData|. + private boolean mSurfaceCreated = false; + /** Helper class for displaying the long-press feedback animation. This class is thread-safe. */ private static class FeedbackAnimator { /** Total duration of the animation, in milliseconds. */ @@ -149,8 +154,7 @@ public void onScreenConfigurationChanged() { * cause the UI to lag. Specifically, it is currently invoked on the native * graphics thread using a JNI. */ - @Override - public void run() { + public void paint() { long startTimeMs = SystemClock.uptimeMillis(); if (Looper.myLooper() == Looper.getMainLooper()) { @@ -181,10 +185,21 @@ public void run() { mInputHandler.onHostSizeChanged(width, height); } - Canvas canvas = getHolder().lockCanvas(); + Canvas canvas; int x, y; synchronized (mRenderData) { mRepaintPending = false; + // Don't try to lock the canvas before it is ready, as the implementation of + // lockCanvas() may throttle these calls to a slow rate in order to avoid consuming CPU. + // Note that a successful call to lockCanvas() will prevent the framework from + // destroying the Surface until it is unlocked. + if (!mSurfaceCreated) { + return; + } + canvas = getHolder().lockCanvas(); + if (canvas == null) { + return; + } canvas.setMatrix(mRenderData.transform); x = mRenderData.cursorPosition.x; y = mRenderData.cursorPosition.y; @@ -249,15 +264,22 @@ public void surfaceChanged(SurfaceHolder holder, int format, int width, int heig mRenderData.screenHeight = height; } + JniInterface.provideRedrawCallback(new Runnable() { + @Override + public void run() { + paint(); + } + }); mInputHandler.onClientSizeChanged(width, height); - - JniInterface.provideRedrawCallback(this); + requestRepaint(); } /** Called when the canvas is first created. */ @Override public void surfaceCreated(SurfaceHolder holder) { - Log.i("deskview", "DesktopView.surfaceCreated(...)"); + synchronized (mRenderData) { + mSurfaceCreated = true; + } } /** @@ -266,10 +288,12 @@ public void surfaceCreated(SurfaceHolder holder) { */ @Override public void surfaceDestroyed(SurfaceHolder holder) { - Log.i("deskview", "DesktopView.surfaceDestroyed(...)"); - // Stop this canvas from being redrawn. JniInterface.provideRedrawCallback(null); + + synchronized (mRenderData) { + mSurfaceCreated = false; + } } /** Called when a software keyboard is requested, and specifies its options. */ diff --git a/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java b/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java index 8074ad2520b0d9..bb84a85792d791 100644 --- a/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java +++ b/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java @@ -41,50 +41,56 @@ public class JniInterface { /* * Library-loading state machine. */ - /** Whether we've already loaded the library. */ + /** Whether the library has been loaded. Accessed on the UI thread. */ private static boolean sLoaded = false; - /** The application context. */ + /** The application context. Accessed on the UI thread. */ private static Activity sContext = null; /* * Connection-initiating state machine. */ - /** Whether the native code is attempting a connection. */ + /** Whether the native code is attempting a connection. Accessed on the UI thread. */ private static boolean sConnected = false; - /** Callback to signal upon successful connection. */ + /** Callback to signal upon successful connection. Accessed on the UI thread. */ private static Runnable sSuccessCallback = null; - /** Dialog for reporting connection progress. */ + /** Dialog for reporting connection progress. Accessed on the UI thread. */ private static ProgressDialog sProgressIndicator = null; - /** Callback to signal whenever we need to redraw. */ + // Protects access to |sProgressIndicator|. Used only to silence FindBugs warnings - the + // variable it protects is only accessed on a single thread. + // TODO(lambroslambrou): Refactor the ProgressIndicator into a separate class. + private static Object sProgressIndicatorLock = new Object(); + + /** + * Callback invoked on the graphics thread to repaint the desktop. Accessed on the UI and + * graphics threads. + */ private static Runnable sRedrawCallback = null; - /** Bitmap holding a copy of the latest video frame. */ + /** Bitmap holding a copy of the latest video frame. Accessed on the UI and graphics threads. */ private static Bitmap sFrameBitmap = null; - /** Lock to protect the frame bitmap reference. */ + /** Protects access to sFrameBitmap. */ private static final Object sFrameLock = new Object(); - /** Position of cursor hot-spot. */ + /** Position of cursor hot-spot. Accessed on the graphics thread. */ private static Point sCursorHotspot = new Point(); - /** Bitmap holding the cursor shape. */ + /** Bitmap holding the cursor shape. Accessed on the graphics thread. */ private static Bitmap sCursorBitmap = null; /** * To be called once from the main Activity. Any subsequent calls will update the application * context, but not reload the library. This is useful e.g. when the activity is closed and the - * user later wants to return to the application. + * user later wants to return to the application. Called on the UI thread. */ public static void loadLibrary(Activity context) { sContext = context; - synchronized(JniInterface.class) { - if (sLoaded) return; - } + if (sLoaded) return; System.loadLibrary("remoting_client_jni"); @@ -102,16 +108,10 @@ public static void loadLibrary(Activity context) { public static native String nativeGetClientId(); public static native String nativeGetClientSecret(); - /** Attempts to form a connection to the user-selected host. */ + /** Attempts to form a connection to the user-selected host. Called on the UI thread. */ public static void connectToHost(String username, String authToken, String hostJid, String hostId, String hostPubkey, Runnable successCallback) { - synchronized(JniInterface.class) { - if (!sLoaded) return; - - if (sConnected) { - disconnectFromHost(); - } - } + disconnectFromHost(); sSuccessCallback = successCallback; SharedPreferences prefs = sContext.getPreferences(Activity.MODE_PRIVATE); @@ -124,11 +124,11 @@ public static void connectToHost(String username, String authToken, private static native void nativeConnect(String username, String authToken, String hostJid, String hostId, String hostPubkey, String pairId, String pairSecret); - /** Severs the connection and cleans up. */ + /** Severs the connection and cleans up. Called on the UI thread. */ public static void disconnectFromHost() { - synchronized(JniInterface.class) { - if (!sLoaded || !sConnected) return; + if (!sConnected) return; + synchronized (sProgressIndicatorLock) { if (sProgressIndicator != null) { sProgressIndicator.dismiss(); sProgressIndicator = null; @@ -148,12 +148,12 @@ public static void disconnectFromHost() { /** Performs the native portion of the cleanup. */ private static native void nativeDisconnect(); - /** Reports whenever the connection status changes. */ + /** Reports whenever the connection status changes. Called on the UI thread. */ @CalledByNative private static void reportConnectionStatus(int state, int error) { if (state < SUCCESSFUL_CONNECTION && error == 0) { // The connection is still being established, so we'll report the current progress. - synchronized (JniInterface.class) { + synchronized (sProgressIndicatorLock) { if (sProgressIndicator == null) { sProgressIndicator = ProgressDialog.show(sContext, sContext. getString(R.string.progress_title), sContext.getResources(). @@ -165,16 +165,14 @@ public void onCancel(DialogInterface dialog) { disconnectFromHost(); } }); - } - else { + } else { sProgressIndicator.setMessage( sContext.getResources().getStringArray(R.array.protoc_states)[state]); } } - } - else { + } else { // The connection is complete or has failed, so we can lose the progress indicator. - synchronized (JniInterface.class) { + synchronized (sProgressIndicatorLock) { if (sProgressIndicator != null) { sProgressIndicator.dismiss(); sProgressIndicator = null; @@ -196,7 +194,7 @@ public void onCancel(DialogInterface dialog) { } } - /** Prompts the user to enter a PIN. */ + /** Prompts the user to enter a PIN. Called on the UI thread. */ @CalledByNative private static void displayAuthenticationPrompt(boolean pairingSupported) { AlertDialog.Builder pinPrompt = new AlertDialog.Builder(sContext); @@ -265,18 +263,19 @@ public void onCancel(DialogInterface dialog) { /** Performs the native response to the user's PIN. */ private static native void nativeAuthenticationResponse(String pin, boolean createPair); - /** Saves newly-received pairing credentials to permanent storage. */ + /** Saves newly-received pairing credentials to permanent storage. Called on the UI thread. */ @CalledByNative private static void commitPairingCredentials(String host, byte[] id, byte[] secret) { - synchronized (sContext) { - sContext.getPreferences(Activity.MODE_PRIVATE).edit(). - putString(host + "_id", new String(id)). - putString(host + "_secret", new String(secret)). - apply(); - } + sContext.getPreferences(Activity.MODE_PRIVATE).edit(). + putString(host + "_id", new String(id)). + putString(host + "_secret", new String(secret)). + apply(); } - /** Moves the mouse cursor, possibly while clicking the specified (nonnegative) button. */ + /** + * Moves the mouse cursor, possibly while clicking the specified (nonnegative) button. Called + * on the UI thread. + */ public static void mouseAction(int x, int y, int whichButton, boolean buttonDown) { if (!sConnected) { return; @@ -288,7 +287,7 @@ public static void mouseAction(int x, int y, int whichButton, boolean buttonDown /** Passes mouse information to the native handling code. */ private static native void nativeMouseAction(int x, int y, int whichButton, boolean buttonDown); - /** Injects a mouse-wheel event with delta values. */ + /** Injects a mouse-wheel event with delta values. Called on the UI thread. */ public static void mouseWheelDeltaAction(int deltaX, int deltaY) { if (!sConnected) { return; @@ -300,7 +299,7 @@ public static void mouseWheelDeltaAction(int deltaX, int deltaY) { /** Passes mouse-wheel information to the native handling code. */ private static native void nativeMouseWheelDeltaAction(int deltaX, int deltaY); - /** Presses and releases the specified (nonnegative) key. */ + /** Presses and releases the specified (nonnegative) key. Called on the UI thread. */ public static void keyboardAction(int keyCode, boolean keyDown) { if (!sConnected) { return; @@ -314,17 +313,16 @@ public static void keyboardAction(int keyCode, boolean keyDown) { /** * Sets the redraw callback to the provided functor. Provide a value of null whenever the - * window is no longer visible so that we don't continue to draw onto it. + * window is no longer visible so that we don't continue to draw onto it. Called on the UI + * thread. */ public static void provideRedrawCallback(Runnable redrawCallback) { sRedrawCallback = redrawCallback; } - /** Forces the native graphics thread to redraw to the canvas. */ + /** Forces the native graphics thread to redraw to the canvas. Called on the UI thread. */ public static boolean redrawGraphics() { - synchronized(JniInterface.class) { - if (!sConnected || sRedrawCallback == null) return false; - } + if (!sConnected || sRedrawCallback == null) return false; nativeScheduleRedraw(); return true; @@ -333,11 +331,15 @@ public static boolean redrawGraphics() { /** Schedules a redraw on the native graphics thread. */ private static native void nativeScheduleRedraw(); - /** Performs the redrawing callback. This is a no-op if the window isn't visible. */ + /** + * Performs the redrawing callback. This is a no-op if the window isn't visible. Called on the + * graphics thread. + */ @CalledByNative private static void redrawGraphicsInternal() { - if (sRedrawCallback != null) { - sRedrawCallback.run(); + Runnable callback = sRedrawCallback; + if (callback != null) { + callback.run(); } } @@ -350,10 +352,6 @@ public static Bitmap getVideoFrame() { Log.w("jniiface", "Canvas being redrawn on UI thread"); } - if (!sConnected) { - return null; - } - synchronized (sFrameLock) { return sFrameBitmap; } @@ -397,9 +395,9 @@ public static void updateCursorShape(int width, int height, int hotspotX, int ho sCursorBitmap = Bitmap.createBitmap(data, width, height, Bitmap.Config.ARGB_8888); } - /** Position of cursor hotspot within cursor image. */ + /** Position of cursor hotspot within cursor image. Called on the graphics thread. */ public static Point getCursorHotspot() { return sCursorHotspot; } - /** Returns the current cursor shape. */ + /** Returns the current cursor shape. Called on the graphics thread. */ public static Bitmap getCursorBitmap() { return sCursorBitmap; } }