From 943002b47346ea82b4e69dca033cc03086969dd1 Mon Sep 17 00:00:00 2001 From: Wale Ogunwale Date: Wed, 15 Feb 2017 19:34:01 -0800 Subject: [PATCH] Set importance for processes displaying app-overlays based on visibility We were previously setting this based on when the window was added and removed. This can lead to issues where an app isn't allowed to show alert windows so window manager hides the window after the permission is revoked, but the window still exist in the system so the process importance will still be high. We now set the process importance based on if it has visible window surfaces or not. Bug: 33256752 Test: cts/.../run-test CtsAppTestCases android.app.cts.AlertWindowsTests Change-Id: I4e406a94683ec0eecc0911d0195e641c693e1c4a --- .../java/com/android/server/wm/Session.java | 73 +++++++++++++++---- .../server/wm/WindowManagerService.java | 8 +- .../com/android/server/wm/WindowState.java | 4 +- .../server/wm/WindowSurfaceController.java | 48 +++++++----- 4 files changed, 90 insertions(+), 43 deletions(-) diff --git a/services/core/java/com/android/server/wm/Session.java b/services/core/java/com/android/server/wm/Session.java index 782f9f2f6611b..e6fd0abf21727 100644 --- a/services/core/java/com/android/server/wm/Session.java +++ b/services/core/java/com/android/server/wm/Session.java @@ -16,7 +16,10 @@ package com.android.server.wm; +import static android.Manifest.permission.INTERNAL_SYSTEM_WINDOW; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_OVERLAY; +import static android.view.WindowManager.LayoutParams.isSystemAlertWindowType; import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_DRAG; import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_TASK_POSITIONING; import static com.android.server.wm.WindowManagerDebugConfig.SHOW_LIGHT_TRANSACTIONS; @@ -54,6 +57,8 @@ import com.android.internal.view.IInputMethodManager; import com.android.server.wm.WindowManagerService.H; import java.io.PrintWriter; +import java.util.HashSet; +import java.util.Set; /** * This class represents an active client session. There is generally one @@ -70,7 +75,11 @@ public class Session extends IWindowSession.Stub private final String mStringName; SurfaceSession mSurfaceSession; private int mNumWindow = 0; - private int mNumOverlayWindow = 0; + // Set of visible application overlay window surfaces connected to this session. + private final Set mAppOverlaySurfaces = new HashSet<>(); + // Set of visible alert window surfaces connected to this session. + private final Set mAlertWindowSurfaces = new HashSet<>(); + final boolean mCanAddInternalSystemWindow; private boolean mClientDead = false; private float mLastReportedAnimatorScale; @@ -82,6 +91,8 @@ public class Session extends IWindowSession.Stub mUid = Binder.getCallingUid(); mPid = Binder.getCallingPid(); mLastReportedAnimatorScale = service.getCurrentAnimatorScale(); + mCanAddInternalSystemWindow = service.mContext.checkCallingPermission( + INTERNAL_SYSTEM_WINDOW) == PERMISSION_GRANTED; StringBuilder sb = new StringBuilder(); sb.append("Session{"); sb.append(Integer.toHexString(System.identityHashCode(this))); @@ -544,7 +555,7 @@ public class Session extends IWindowSession.Stub } } - void windowAddedLocked(int type) { + void windowAddedLocked() { if (mSurfaceSession == null) { if (WindowManagerService.localLOGV) Slog.v( TAG_WM, "First window added to " + this + ", creating SurfaceSession"); @@ -557,24 +568,52 @@ public class Session extends IWindowSession.Stub } } mNumWindow++; - if (type == TYPE_APPLICATION_OVERLAY) { - mNumOverlayWindow++; - setHasOverlayUi(true); - } } - void windowRemovedLocked(int type) { + void windowRemovedLocked() { mNumWindow--; - if (type == TYPE_APPLICATION_OVERLAY) { - mNumOverlayWindow--; - if (mNumOverlayWindow == 0) { - setHasOverlayUi(false); - } else if (mNumOverlayWindow < 0) { - throw new IllegalStateException("mNumOverlayWindow=" + mNumOverlayWindow - + " less than 0 for session=" + this); + killSessionLocked(); + } + + + void onWindowSurfaceVisibilityChanged(WindowSurfaceController surfaceController, + boolean visible, int type) { + + if (!isSystemAlertWindowType(type)) { + return; + } + + boolean changed; + + if (!mCanAddInternalSystemWindow) { + // We want to track non-system signature apps adding alert windows so we can post an + // on-going notification for the user to control their visibility. + if (visible) { + changed = mAlertWindowSurfaces.add(surfaceController); + } else { + changed = mAlertWindowSurfaces.remove(surfaceController); + } + + if (changed) { + // TODO: Update notification. } } - killSessionLocked(); + + if (type != TYPE_APPLICATION_OVERLAY) { + return; + } + + if (visible) { + changed = mAppOverlaySurfaces.add(surfaceController); + } else { + changed = mAppOverlaySurfaces.remove(surfaceController); + } + + if (changed) { + // Notify activity manager of changes to app overlay windows so it can adjust the + // importance score for the process. + setHasOverlayUi(!mAppOverlaySurfaces.isEmpty()); + } } private void killSessionLocked() { @@ -598,6 +637,7 @@ public class Session extends IWindowSession.Stub } mSurfaceSession = null; setHasOverlayUi(false); + // TODO: Update notification } private void setHasOverlayUi(boolean hasOverlayUi) { @@ -606,7 +646,8 @@ public class Session extends IWindowSession.Stub void dump(PrintWriter pw, String prefix) { pw.print(prefix); pw.print("mNumWindow="); pw.print(mNumWindow); - pw.print(" mNumOverlayWindow="); pw.print(mNumOverlayWindow); + pw.print(" mAppOverlaySurfaces="); pw.print(mAppOverlaySurfaces); + pw.print(" mAlertWindowSurfaces="); pw.print(mAlertWindowSurfaces); pw.print(" mClientDead="); pw.print(mClientDead); pw.print(" mSurfaceSession="); pw.println(mSurfaceSession); } diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 5653113e2cbd2..1df9e433bb1ed 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -1111,8 +1111,6 @@ public class WindowManagerService extends IWindowManager.Stub long origId; final int callingUid = Binder.getCallingUid(); final int type = attrs.type; - final boolean ownerCanAddInternalSystemWindow = - mContext.checkCallingPermission(INTERNAL_SYSTEM_WINDOW) == PERMISSION_GRANTED; synchronized(mWindowMap) { if (!mDisplayReady) { @@ -1215,7 +1213,7 @@ public class WindowManagerService extends IWindowManager.Stub } } token = new WindowToken(this, attrs.token, type, false, displayContent, - ownerCanAddInternalSystemWindow); + session.mCanAddInternalSystemWindow); } else if (rootType >= FIRST_APPLICATION_WINDOW && rootType <= LAST_APPLICATION_WINDOW) { atoken = token.asAppWindowToken(); if (atoken == null) { @@ -1286,12 +1284,12 @@ public class WindowManagerService extends IWindowManager.Stub // instead make a new token for it (as if null had been passed in for the token). attrs.token = null; token = new WindowToken(this, null, type, false, displayContent, - ownerCanAddInternalSystemWindow); + session.mCanAddInternalSystemWindow); } final WindowState win = new WindowState(this, session, client, token, parentWindow, appOp[0], seq, attrs, viewVisibility, session.mUid, - ownerCanAddInternalSystemWindow); + session.mCanAddInternalSystemWindow); if (win.mDeathRecipient == null) { // Client has apparently died, so there is no reason to // continue. diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 65e1f84eae357..867080ed2450a 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -665,7 +665,7 @@ class WindowState extends WindowContainer implements WindowManagerP void attach() { if (localLOGV) Slog.v(TAG, "Attaching " + this + " token=" + mToken); - mSession.windowAddedLocked(mAttrs.type); + mSession.windowAddedLocked(); } @Override @@ -1738,7 +1738,7 @@ class WindowState extends WindowContainer implements WindowManagerP mWinAnimator.destroyDeferredSurfaceLocked(); mWinAnimator.destroySurfaceLocked(); - mSession.windowRemovedLocked(mAttrs.type); + mSession.windowRemovedLocked(); try { mClient.asBinder().unlinkToDeath(mDeathRecipient, 0); } catch (RuntimeException e) { diff --git a/services/core/java/com/android/server/wm/WindowSurfaceController.java b/services/core/java/com/android/server/wm/WindowSurfaceController.java index 1096ede963167..f8e74284fafd1 100644 --- a/services/core/java/com/android/server/wm/WindowSurfaceController.java +++ b/services/core/java/com/android/server/wm/WindowSurfaceController.java @@ -52,6 +52,7 @@ class WindowSurfaceController { private SurfaceControl mSurfaceControl; + // Should only be set from within setShown(). private boolean mSurfaceShown = false; private float mSurfaceX = 0; private float mSurfaceY = 0; @@ -79,6 +80,9 @@ class WindowSurfaceController { private final WindowManagerService mService; + private final int mWindowType; + private final Session mWindowSession; + public WindowSurfaceController(SurfaceSession s, String name, int w, int h, int format, int flags, WindowStateAnimator animator, int windowType, int ownerUid) { mAnimator = animator; @@ -89,16 +93,16 @@ class WindowSurfaceController { title = name; mService = animator.mService; + final WindowState win = animator.mWin; + mWindowType = windowType; + mWindowSession = win.mSession; - // For opaque child windows placed under parent windows, - // we use a special SurfaceControl which mirrors commands - // to a black-out layer placed one Z-layer below the surface. + // For opaque child windows placed under parent windows, we use a special SurfaceControl + // which mirrors commands to a black-out layer placed one Z-layer below the surface. // This prevents holes to whatever app/wallpaper is underneath. - if (animator.mWin.isChildWindow() && - animator.mWin.mSubLayer < 0 && - animator.mWin.mAppToken != null) { - mSurfaceControl = new SurfaceControlWithBackground(s, - name, w, h, format, flags, animator.mWin.mAppToken, windowType, ownerUid); + if (win.isChildWindow() && win.mSubLayer < 0 && win.mAppToken != null) { + mSurfaceControl = new SurfaceControlWithBackground( + s, name, w, h, format, flags, win.mAppToken, windowType, ownerUid); } else if (DEBUG_SURFACE_TRACE) { mSurfaceControl = new SurfaceTrace( s, name, w, h, format, flags, windowType, ownerUid); @@ -109,8 +113,7 @@ class WindowSurfaceController { if (mService.mRoot.mSurfaceTraceEnabled) { mSurfaceControl = new RemoteSurfaceTrace( - mService.mRoot.mSurfaceTraceFd.getFileDescriptor(), - mSurfaceControl, animator.mWin); + mService.mRoot.mSurfaceTraceFd.getFileDescriptor(), mSurfaceControl, win); } } @@ -141,13 +144,14 @@ class WindowSurfaceController { } private void hideSurface() { - if (mSurfaceControl != null) { - mSurfaceShown = false; - try { - mSurfaceControl.hide(); - } catch (RuntimeException e) { - Slog.w(TAG, "Exception hiding surface in " + this); - } + if (mSurfaceControl == null) { + return; + } + setShown(false); + try { + mSurfaceControl.hide(); + } catch (RuntimeException e) { + Slog.w(TAG, "Exception hiding surface in " + this); } } @@ -165,7 +169,7 @@ class WindowSurfaceController { mSurfaceControl.setLayer(layer); mSurfaceControl.setAlpha(0); - mSurfaceShown = false; + setShown(false); } catch (RuntimeException e) { Slog.w(TAG, "Error creating surface in " + this, e); mAnimator.reclaimSomeSurfaceMemory("create-init", true); @@ -188,7 +192,7 @@ class WindowSurfaceController { } catch (RuntimeException e) { Slog.w(TAG, "Error destroying surface in: " + this, e); } finally { - mSurfaceShown = false; + setShown(false); mSurfaceControl = null; } } @@ -447,7 +451,7 @@ class WindowSurfaceController { private boolean showSurface() { try { - mSurfaceShown = true; + setShown(true); mSurfaceControl.show(); return true; } catch (RuntimeException e) { @@ -515,6 +519,10 @@ class WindowSurfaceController { void setShown(boolean surfaceShown) { mSurfaceShown = surfaceShown; + + if (mWindowSession != null) { + mWindowSession.onWindowSurfaceVisibilityChanged(this, mSurfaceShown, mWindowType); + } } float getX() {