From 49a2edf92ab9b02762a2c183809fdee55b0fcf40 Mon Sep 17 00:00:00 2001 From: Craig Mautner Date: Thu, 25 Sep 2014 14:49:26 -0700 Subject: [PATCH] Call Surface.release() for starting windows If the window maanger received BinderDied for a starting window before activity manager then it would null AppWindowToken.startingWindow and not go through the PhoneWindowManager.removeStartingWindow call later. That meant that Surface.release() was never called from ViewRootImpl.dispatchDetachedFromWindow(). Which in turn meant that graphics memory was being leaked. This change notifies the PhoneWindowManager to go through the removeStartingWindow path when the starting window gets removed for any reason. This change also ensures that scheduleRemoveStartingWindow is always called with the window manager lock held. Fixes bug 17381033. Change-Id: Ic6860d0e1410c9bb5053d85ae21a08b11f573b6d --- .../server/wm/WindowManagerService.java | 24 ++++++---- .../server/wm/WindowStateAnimator.java | 48 +++++++++++-------- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 999649b02e685..62a5e63417f13 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -2712,9 +2712,10 @@ public class WindowManagerService extends IWindowManager.Stub if (atoken != null) { if (atoken.startingWindow == win) { - if (DEBUG_STARTING_WINDOW) Slog.v(TAG, "Nulling startingWindow " + win); - atoken.startingWindow = null; - } else if (atoken.allAppWindows.size() == 0 && atoken.startingData != null) { + if (DEBUG_STARTING_WINDOW) Slog.v(TAG, "Notify removed startingWindow " + win); + scheduleRemoveStartingWindowLocked(atoken); + } else + if (atoken.allAppWindows.size() == 0 && atoken.startingData != null) { // If this is the last window and we had requested a starting // transition window, well there is no point now. if (DEBUG_STARTING_WINDOW) Slog.v(TAG, "Nulling last startingWindow"); @@ -2722,7 +2723,7 @@ public class WindowManagerService extends IWindowManager.Stub } else if (atoken.allAppWindows.size() == 1 && atoken.startingView != null) { // If this is the last window except for a starting transition // window, we need to get rid of the starting transition. - scheduleRemoveStartingWindow(atoken); + scheduleRemoveStartingWindowLocked(atoken); } } @@ -4329,7 +4330,7 @@ public class WindowManagerService extends IWindowManager.Stub synchronized (mWindowMap) { AppWindowToken wtoken = mTokenMap.get(token).appWindowToken; if (wtoken.startingWindow != null) { - scheduleRemoveStartingWindow(wtoken); + scheduleRemoveStartingWindowLocked(wtoken); } } } @@ -4791,14 +4792,19 @@ public class WindowManagerService extends IWindowManager.Stub if (!delayed && wtoken != null) { wtoken.updateReportedVisibilityLocked(); } + + // Will only remove if startingToken non null. + scheduleRemoveStartingWindowLocked(startingToken); } Binder.restoreCallingIdentity(origId); - // Will only remove if startingToken non null. - scheduleRemoveStartingWindow(startingToken); } - void scheduleRemoveStartingWindow(AppWindowToken wtoken) { + void scheduleRemoveStartingWindowLocked(AppWindowToken wtoken) { + if (mH.hasMessages(H.REMOVE_STARTING, wtoken)) { + // Already scheduled. + return; + } if (wtoken != null && wtoken.startingWindow != null) { if (DEBUG_STARTING_WINDOW) Slog.v(TAG, Debug.getCallers(1) + ": Schedule remove starting " + wtoken + (wtoken != null ? @@ -10236,7 +10242,7 @@ public class WindowManagerService extends IWindowManager.Stub winAnimator.mSurfaceShown = false; winAnimator.mSurfaceControl = null; winAnimator.mWin.mHasSurface = false; - scheduleRemoveStartingWindow(winAnimator.mWin.mAppToken); + scheduleRemoveStartingWindowLocked(winAnimator.mWin.mAppToken); } try { diff --git a/services/core/java/com/android/server/wm/WindowStateAnimator.java b/services/core/java/com/android/server/wm/WindowStateAnimator.java index 5b007d49fcfae..0c727f33172e1 100644 --- a/services/core/java/com/android/server/wm/WindowStateAnimator.java +++ b/services/core/java/com/android/server/wm/WindowStateAnimator.java @@ -515,6 +515,7 @@ class WindowStateAnimator { static class SurfaceTrace extends SurfaceControl { private final static String SURFACE_TAG = "SurfaceTrace"; + private final static boolean logSurfaceTrace = DEBUG_SURFACE_TRACE; final static ArrayList sSurfaces = new ArrayList(); private float mSurfaceTraceAlpha = 0; @@ -534,7 +535,7 @@ class WindowStateAnimator { super(s, name, w, h, format, flags); mName = name != null ? name : "Not named"; mSize.set(w, h); - Slog.v(SURFACE_TAG, "ctor: " + this + ". Called by " + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "ctor: " + this + ". Called by " + Debug.getCallers(3)); synchronized (sSurfaces) { sSurfaces.add(0, this); @@ -544,8 +545,8 @@ class WindowStateAnimator { @Override public void setAlpha(float alpha) { if (mSurfaceTraceAlpha != alpha) { - Slog.v(SURFACE_TAG, "setAlpha(" + alpha + "): OLD:" + this + ". Called by " - + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setAlpha(" + alpha + "): OLD:" + this + + ". Called by " + Debug.getCallers(3)); mSurfaceTraceAlpha = alpha; } super.setAlpha(alpha); @@ -554,8 +555,8 @@ class WindowStateAnimator { @Override public void setLayer(int zorder) { if (zorder != mLayer) { - Slog.v(SURFACE_TAG, "setLayer(" + zorder + "): OLD:" + this + ". Called by " - + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setLayer(" + zorder + "): OLD:" + this + + ". Called by " + Debug.getCallers(3)); mLayer = zorder; } super.setLayer(zorder); @@ -576,8 +577,8 @@ class WindowStateAnimator { @Override public void setPosition(float x, float y) { if (x != mPosition.x || y != mPosition.y) { - Slog.v(SURFACE_TAG, "setPosition(" + x + "," + y + "): OLD:" + this - + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setPosition(" + x + "," + y + "): OLD:" + + this + ". Called by " + Debug.getCallers(3)); mPosition.set(x, y); } super.setPosition(x, y); @@ -586,8 +587,8 @@ class WindowStateAnimator { @Override public void setSize(int w, int h) { if (w != mSize.x || h != mSize.y) { - Slog.v(SURFACE_TAG, "setSize(" + w + "," + h + "): OLD:" + this + ". Called by " - + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setSize(" + w + "," + h + "): OLD:" + + this + ". Called by " + Debug.getCallers(3)); mSize.set(w, h); } super.setSize(w, h); @@ -597,8 +598,9 @@ class WindowStateAnimator { public void setWindowCrop(Rect crop) { if (crop != null) { if (!crop.equals(mWindowCrop)) { - Slog.v(SURFACE_TAG, "setWindowCrop(" + crop.toShortString() + "): OLD:" + this - + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setWindowCrop(" + + crop.toShortString() + "): OLD:" + this + ". Called by " + + Debug.getCallers(3)); mWindowCrop.set(crop); } } @@ -608,8 +610,8 @@ class WindowStateAnimator { @Override public void setLayerStack(int layerStack) { if (layerStack != mLayerStack) { - Slog.v(SURFACE_TAG, "setLayerStack(" + layerStack + "): OLD:" + this - + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setLayerStack(" + layerStack + "): OLD:" + + this + ". Called by " + Debug.getCallers(3)); mLayerStack = layerStack; } super.setLayerStack(layerStack); @@ -618,8 +620,8 @@ class WindowStateAnimator { @Override public void setOpaque(boolean isOpaque) { if (isOpaque != mIsOpaque) { - Slog.v(SURFACE_TAG, "setOpaque(" + isOpaque + "): OLD:" + this - + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setOpaque(" + isOpaque + "): OLD:" + + this + ". Called by " + Debug.getCallers(3)); mIsOpaque = isOpaque; } super.setOpaque(isOpaque); @@ -628,8 +630,9 @@ class WindowStateAnimator { @Override public void setMatrix(float dsdx, float dtdx, float dsdy, float dtdy) { if (dsdx != mDsdx || dtdx != mDtdx || dsdy != mDsdy || dtdy != mDtdy) { - Slog.v(SURFACE_TAG, "setMatrix(" + dsdx + "," + dtdx + "," + dsdy + "," + dtdy + - "): OLD:" + this + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "setMatrix(" + dsdx + "," + dtdx + "," + + dsdy + "," + dtdy + "): OLD:" + this + ". Called by " + + Debug.getCallers(3)); mDsdx = dsdx; mDtdx = dtdx; mDsdy = dsdy; @@ -641,7 +644,8 @@ class WindowStateAnimator { @Override public void hide() { if (mShown) { - Slog.v(SURFACE_TAG, "hide: OLD:" + this + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "hide: OLD:" + this + ". Called by " + + Debug.getCallers(3)); mShown = false; } super.hide(); @@ -650,7 +654,8 @@ class WindowStateAnimator { @Override public void show() { if (!mShown) { - Slog.v(SURFACE_TAG, "show: OLD:" + this + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "show: OLD:" + this + ". Called by " + + Debug.getCallers(3)); mShown = true; } super.show(); @@ -659,7 +664,8 @@ class WindowStateAnimator { @Override public void destroy() { super.destroy(); - Slog.v(SURFACE_TAG, "destroy: " + this + ". Called by " + Debug.getCallers(3)); + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "destroy: " + this + ". Called by " + + Debug.getCallers(3)); synchronized (sSurfaces) { sSurfaces.remove(this); } @@ -668,7 +674,7 @@ class WindowStateAnimator { @Override public void release() { super.release(); - Slog.v(SURFACE_TAG, "release: " + this + ". Called by " + if (logSurfaceTrace) Slog.v(SURFACE_TAG, "release: " + this + ". Called by " + Debug.getCallers(3)); synchronized (sSurfaces) { sSurfaces.remove(this);