Surface: Leave object in unlocked state when unlockCanvasAndPost fails

If nativeUnlockCanvasAndPost() throws, Surface was maintaining a
reference to the native Surface, and assuming it was still locked.
That would cause future lockCanvas() calls to throw without even
trying to lock the native Surface, even though in some cases the
native lock was actually released before the exception was thrown.

Now Surface treats the native object as unlocked even if
nativeUnlockCanvasAndPost() throws, so it will attempt the native lock
on lockCanvas() rather than assuming it would fail.

This change also changes an IllegalStateException to
IllegalArgumentException in unlockCanvasAndPost(). That exception was
added in KitKat, and was never documented or added to the throws
declaration. This was essentially a silent public API change. Quite a
bit of code in the framework (and likely in applications) catches
IAE from this method, but didn't attempt to handle ISE. Although ISE
is more correct here, it's not worth breaking code (and it did -- in
this bug it changed a problem that should have been silently and
perfectly recovered from into a fatal exception in system_server.)

Bug: 17684556
Change-Id: Ia8d3e5d33eaa690d16c7d0f557390c7bb4e1e32e

1 files changed, 7 insertions, 4 deletions

diff --git a/core/java/android/view/Surface.java b/core/java/android/view/Surface.java
index 78986d946161..3770b8a31922 100644
--- a/core/java/android/view/Surface.java
+++ b/core/java/android/view/Surface.java
@@ -250,7 +250,7 @@ public class Surface implements Parcelable {
                 // double-lock, but that won't happen if mNativeObject was updated.  We can't
                 // abandon the old mLockedObject because it might still be in use, so instead
                 // we just refuse to re-lock the Surface.
-                throw new IllegalStateException("Surface was already locked");
+                throw new IllegalArgumentException("Surface was already locked");
             }
             mLockedObject = nativeLockCanvas(mNativeObject, mCanvas, inOutDirty);
             return mCanvas;
@@ -279,9 +279,12 @@ public class Surface implements Parcelable {
             if (mLockedObject == 0) {
                 throw new IllegalStateException("Surface was not locked");
             }
-            nativeUnlockCanvasAndPost(mLockedObject, canvas);
-            nativeRelease(mLockedObject);
-            mLockedObject = 0;
+            try {
+                nativeUnlockCanvasAndPost(mLockedObject, canvas);
+            } finally {
+                nativeRelease(mLockedObject);
+                mLockedObject = 0;
+            }
         }
     }