Remove some TODOs.

Change-Id: I52f5a8a76593dde177c2e931f656b13134a3bd2b

4 files changed, 45 insertions, 52 deletions

diff --git a/services/java/com/android/server/wm/AppWindowToken.java b/services/java/com/android/server/wm/AppWindowToken.java
index adad09d..0ada604 100644
--- a/services/java/com/android/server/wm/AppWindowToken.java
+++ b/services/java/com/android/server/wm/AppWindowToken.java
@@ -225,7 +225,6 @@ class AppWindowToken extends WindowToken {
 
     boolean isVisible() {
         final int N = allAppWindows.size();
-        // TODO: Consider using allDrawn instead of a single window.
         for (int i=0; i<N; i++) {
             WindowState win = allAppWindows.get(i);
             if (!win.mAppFreezing
diff --git a/services/java/com/android/server/wm/WindowManagerService.java b/services/java/com/android/server/wm/WindowManagerService.java
index 2ca419b..9c3a15d 100644
--- a/services/java/com/android/server/wm/WindowManagerService.java
+++ b/services/java/com/android/server/wm/WindowManagerService.java
@@ -324,6 +324,7 @@ public class WindowManagerService extends IWindowManager.Stub
     /**
      * Mapping from an IWindow IBinder to the server's Window object.
      * This is also used as the lock for all of our state.
+     * NOTE: Never call into methods that lock ActivityManagerService while holding this object.
      */
     final HashMap<IBinder, WindowState> mWindowMap = new HashMap<IBinder, WindowState>();
 
@@ -1113,7 +1114,6 @@ public class WindowManagerService extends IWindowManager.Stub
         }
     }
 
-    /** TODO(cmautner): Is this the same as {@link WindowState#canReceiveKeys()} */
     static boolean canBeImeTarget(WindowState w) {
         final int fl = w.mAttrs.flags
                 & (FLAG_NOT_FOCUSABLE|FLAG_ALT_FOCUSABLE_IM);
@@ -1969,12 +1969,7 @@ public class WindowManagerService extends IWindowManager.Stub
                     winAnimator.computeShownFrameLocked();
                     // No need to lay out the windows - we can just set the wallpaper position
                     // directly.
-                    // TODO(cmautner): Don't move this from here, just lock the WindowAnimator.
-                    if (winAnimator.mSurfaceX != wallpaper.mShownFrame.left
-                            || winAnimator.mSurfaceY != wallpaper.mShownFrame.top) {
-                        winAnimator.setWallpaperOffset((int) wallpaper.mShownFrame.left,
-                                (int) wallpaper.mShownFrame.top);
-                    }
+                    winAnimator.setWallpaperOffset(wallpaper.mShownFrame);
                     // We only want to be synchronous with one wallpaper.
                     sync = false;
                 }
@@ -2478,11 +2473,6 @@ public class WindowManagerService extends IWindowManager.Stub
         }
     }
 
-    // TODO(cmautner): Move to WindowStateAnimator.
-    void setTransparentRegionHint(final WindowStateAnimator winAnimator, final Region region) {
-        mH.sendMessage(mH.obtainMessage(H.SET_TRANSPARENT_REGION,
-                new Pair<WindowStateAnimator, Region>(winAnimator, region)));
-    }
 
     void setTransparentRegionWindow(Session session, IWindow client, Region region) {
         long origId = Binder.clearCallingIdentity();
@@ -2490,7 +2480,9 @@ public class WindowManagerService extends IWindowManager.Stub
             synchronized (mWindowMap) {
                 WindowState w = windowForClientLocked(session, client, false);
                 if ((w != null) && w.mHasSurface) {
-                    setTransparentRegionHint(w.mWinAnimator, region);
+                    final Pair<WindowStateAnimator, Region> pair =
+                            new Pair<WindowStateAnimator, Region>(w.mWinAnimator, region);
+                    mH.sendMessage(mH.obtainMessage(H.SET_TRANSPARENT_REGION, pair));
                 }
             }
         } finally {
@@ -2652,7 +2644,6 @@ public class WindowManagerService extends IWindowManager.Stub
         long origId = Binder.clearCallingIdentity();
 
         synchronized(mWindowMap) {
-            // TODO(cmautner): synchronize on mAnimator or win.mWinAnimator.
             WindowState win = windowForClientLocked(session, client, false);
             if (win == null) {
                 return 0;
@@ -7031,8 +7022,8 @@ public class WindowManagerService extends IWindowManager.Stub
                     break;
                 }
 
-                // Animation messages. Move to Window{State}Animator
                 case SET_TRANSPARENT_REGION: {
+                    @SuppressWarnings("unchecked")
                     Pair<WindowStateAnimator, Region> pair =
                                 (Pair<WindowStateAnimator, Region>) msg.obj;
                     final WindowStateAnimator winAnimator = pair.first;
@@ -7147,13 +7138,13 @@ public class WindowManagerService extends IWindowManager.Stub
     }
 
     public void getInitialDisplaySize(int displayId, Point size) {
-        // TODO(cmautner): Access to DisplayContent should be locked on mWindowMap. Doing that
-        //  could lead to deadlock since this is called from ActivityManager.
-        final DisplayContent displayContent = getDisplayContentLocked(displayId);
-        if (displayContent != null) {
-            synchronized(displayContent.mDisplaySizeLock) {
-                size.x = displayContent.mInitialDisplayWidth;
-                size.y = displayContent.mInitialDisplayHeight;
+        synchronized (mWindowMap) {
+            final DisplayContent displayContent = getDisplayContentLocked(displayId);
+            if (displayContent != null) {
+                synchronized(displayContent.mDisplaySizeLock) {
+                    size.x = displayContent.mInitialDisplayWidth;
+                    size.y = displayContent.mInitialDisplayHeight;
+                }
             }
         }
     }
@@ -8951,8 +8942,6 @@ public class WindowManagerService extends IWindowManager.Stub
         boolean doRequest = false;
 
         final int bulkUpdateParams = mAnimator.mBulkUpdateParams;
-        // TODO(cmautner): As the number of bits grows, use masks of bit groups to
-        //  eliminate unnecessary tests.
         if ((bulkUpdateParams & LayoutFields.SET_UPDATE_ROTATION) != 0) {
             mInnerFields.mUpdateRotation = true;
             doRequest = true;
diff --git a/services/java/com/android/server/wm/WindowState.java b/services/java/com/android/server/wm/WindowState.java
index 5fd42c2..cb11be3 100644
--- a/services/java/com/android/server/wm/WindowState.java
+++ b/services/java/com/android/server/wm/WindowState.java
@@ -945,11 +945,11 @@ final class WindowState implements WindowManagerPolicy.WindowState {
         }
     }
 
-    /** Returns true if this window desires key events.
-     * TODO(cmautner): Is this the same as {@link WindowManagerService#canBeImeTarget}
+    /**
+     * @return true if this window desires key events.
      */
     public final boolean canReceiveKeys() {
-        return     isVisibleOrAdding()
+        return isVisibleOrAdding()
                 && (mViewVisibility == View.VISIBLE)
                 && ((mAttrs.flags & WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE) == 0);
     }
diff --git a/services/java/com/android/server/wm/WindowStateAnimator.java b/services/java/com/android/server/wm/WindowStateAnimator.java
index cc58ad8..bccdd68 100644
--- a/services/java/com/android/server/wm/WindowStateAnimator.java
+++ b/services/java/com/android/server/wm/WindowStateAnimator.java
@@ -12,6 +12,7 @@ import android.graphics.PixelFormat;
 import android.graphics.Point;
 import android.graphics.PointF;
 import android.graphics.Rect;
+import android.graphics.RectF;
 import android.graphics.Region;
 import android.os.Debug;
 import android.util.Slog;
@@ -1337,31 +1338,35 @@ class WindowStateAnimator {
         }
     }
 
-    void setWallpaperOffset(int left, int top) {
-        mSurfaceX = left;
-        mSurfaceY = top;
-        if (mAnimating) {
-            // If this window (or its app token) is animating, then the position
-            // of the surface will be re-computed on the next animation frame.
-            // We can't poke it directly here because it depends on whatever
-            // transformation is being applied by the animation.
-            return;
-        }
-        if (SHOW_LIGHT_TRANSACTIONS) Slog.i(TAG,
-                ">>> OPEN TRANSACTION setWallpaperOffset");
-        Surface.openTransaction();
-        try {
-            if (WindowManagerService.SHOW_TRANSACTIONS) WindowManagerService.logSurface(mWin,
-                    "POS " + left + ", " + top, null);
-            mSurface.setPosition(mWin.mFrame.left + left, mWin.mFrame.top + top);
-            updateSurfaceWindowCrop(false);
-        } catch (RuntimeException e) {
-            Slog.w(TAG, "Error positioning surface of " + mWin
-                    + " pos=(" + left + "," + top + ")", e);
-        } finally {
-            Surface.closeTransaction();
+    void setWallpaperOffset(RectF shownFrame) {
+        final int left = (int) shownFrame.left;
+        final int top = (int) shownFrame.top;
+        if (mSurfaceX != left || mSurfaceY != top) {
+            mSurfaceX = left;
+            mSurfaceY = top;
+            if (mAnimating) {
+                // If this window (or its app token) is animating, then the position
+                // of the surface will be re-computed on the next animation frame.
+                // We can't poke it directly here because it depends on whatever
+                // transformation is being applied by the animation.
+                return;
+            }
             if (SHOW_LIGHT_TRANSACTIONS) Slog.i(TAG,
-                    "<<< CLOSE TRANSACTION setWallpaperOffset");
+                    ">>> OPEN TRANSACTION setWallpaperOffset");
+            Surface.openTransaction();
+            try {
+                if (WindowManagerService.SHOW_TRANSACTIONS) WindowManagerService.logSurface(mWin,
+                        "POS " + left + ", " + top, null);
+                mSurface.setPosition(mWin.mFrame.left + left, mWin.mFrame.top + top);
+                updateSurfaceWindowCrop(false);
+            } catch (RuntimeException e) {
+                Slog.w(TAG, "Error positioning surface of " + mWin
+                        + " pos=(" + left + "," + top + ")", e);
+            } finally {
+                Surface.closeTransaction();
+                if (SHOW_LIGHT_TRANSACTIONS) Slog.i(TAG,
+                        "<<< CLOSE TRANSACTION setWallpaperOffset");
+            }
         }
     }