IWallpaperServiceWrapper keep track of multiple engines

Bug: 242851438
Test: WallpaperManagerTest
Change-Id: I5606cec182bdc4d24eda03a05849d6a2aab39d61
(cherry picked from commit 5822fade22)
Merged-In: I5606cec182bdc4d24eda03a05849d6a2aab39d61

core/java/android/service/wallpaper/IWallpaperService.aidl

@@ -26,5 +26,5 @@ oneway interface IWallpaperService {
     void attach(IWallpaperConnection connection,
             IBinder windowToken, int windowType, boolean isPreview,
             int reqWidth, int reqHeight, in Rect padding, int displayId, int which);
-    void detach();
+    void detach(IBinder windowToken);
 }

core/java/android/service/wallpaper/WallpaperService.java

@@ -68,9 +68,11 @@ import android.os.RemoteException;
 import android.os.SystemClock;
 import android.os.SystemProperties;
 import android.os.Trace;
+import android.util.ArrayMap;
 import android.util.ArraySet;
 import android.util.Log;
 import android.util.MergedConfiguration;
+import android.util.Slog;
 import android.view.Display;
 import android.view.DisplayCutout;
 import android.view.Gravity;
@@ -102,7 +104,6 @@ import com.android.internal.view.BaseSurfaceHolder;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
@@ -177,8 +178,7 @@ public abstract class WallpaperService extends Service {
 
     private static final long DIMMING_ANIMATION_DURATION_MS = 300L;
 
-    private final ArrayList<Engine> mActiveEngines
+    private final ArrayMap<IBinder, IWallpaperEngineWrapper> mActiveEngines = new ArrayMap<>();
-            = new ArrayList<Engine>();
 
     static final class WallpaperCommand {
         String action;
@@ -2231,7 +2231,6 @@ public abstract class WallpaperService extends Service {
         final DisplayManager mDisplayManager;
         final Display mDisplay;
         final WallpaperManager mWallpaperManager;
-        private final AtomicBoolean mDetached = new AtomicBoolean();
 
         Engine mEngine;
         @SetWallpaperFlags int mWhich;
@@ -2346,21 +2345,18 @@ public abstract class WallpaperService extends Service {
             mEngine.removeLocalColorsAreas(regions);
         }
 
-        public void destroy() {
-            Message msg = mCaller.obtainMessage(DO_DETACH);
-            mCaller.sendMessage(msg);
-        }
-
-        public void detach() {
-            mDetached.set(true);
-        }
-
         public void applyDimming(float dimAmount) throws RemoteException {
             Message msg = mCaller.obtainMessageI(MSG_UPDATE_DIMMING,
                     Float.floatToIntBits(dimAmount));
             mCaller.sendMessage(msg);
         }
 
+        public void destroy() {
+            Message msg = mCaller.obtainMessage(DO_DETACH);
+            mCaller.getHandler().removeCallbacksAndMessages(null);
+            mCaller.sendMessage(msg);
+        }
+
         public void resizePreview(Rect position) {
             Message msg = mCaller.obtainMessageO(MSG_RESIZE_PREVIEW, position);
             mCaller.sendMessage(msg);
@@ -2383,25 +2379,27 @@ public abstract class WallpaperService extends Service {
                 engine.detach();
                 Log.w(TAG, "Wallpaper host disappeared", e);
                 return;
+            } catch (IllegalStateException e) {
+                Log.w(TAG, "Connector instance already destroyed, "
+                                + "can't attach engine to non existing connector", e);
+                return;
             } finally {
                 Trace.endSection();
             }
-            mActiveEngines.add(engine);
             Trace.beginSection("WPMS.engine.attach");
             engine.attach(this);
             Trace.endSection();
         }
 
         private void doDetachEngine() {
-            mActiveEngines.remove(mEngine);
-            mEngine.detach();
             // Some wallpapers will not trigger the rendering threads of the remaining engines even
             // if they are visible, so we need to toggle the state to get their attention.
-            if (!mDetached.get()) {
+            if (!mEngine.mDestroyed) {
-                for (Engine eng : mActiveEngines) {
+                mEngine.detach();
-                    if (eng.mVisible) {
+                for (IWallpaperEngineWrapper engineWrapper : mActiveEngines.values()) {
-                        eng.doVisibilityChanged(false);
+                    if (engineWrapper.mEngine != null && engineWrapper.mEngine.mVisible) {
-                        eng.doVisibilityChanged(true);
+                        engineWrapper.mEngine.doVisibilityChanged(false);
+                        engineWrapper.mEngine.doVisibilityChanged(true);
                     }
                 }
             }
@@ -2409,12 +2407,6 @@ public abstract class WallpaperService extends Service {
 
         @Override
         public void executeMessage(Message message) {
-            if (mDetached.get()) {
-                if (mActiveEngines.contains(mEngine)) {
-                    doDetachEngine();
-                }
-                return;
-            }
             switch (message.what) {
                 case DO_ATTACH: {
                     Trace.beginSection("WPMS.DO_ATTACH");
@@ -2525,7 +2517,6 @@ public abstract class WallpaperService extends Service {
      */
     class IWallpaperServiceWrapper extends IWallpaperService.Stub {
         private final WallpaperService mTarget;
-        private IWallpaperEngineWrapper mEngineWrapper;
 
         public IWallpaperServiceWrapper(WallpaperService context) {
             mTarget = context;
@@ -2536,14 +2527,27 @@ public abstract class WallpaperService extends Service {
                 int windowType, boolean isPreview, int reqWidth, int reqHeight, Rect padding,
                 int displayId, @SetWallpaperFlags int which) {
             Trace.beginSection("WPMS.ServiceWrapper.attach");
-            mEngineWrapper = new IWallpaperEngineWrapper(mTarget, conn, windowToken,
+            IWallpaperEngineWrapper engineWrapper =
-                    windowType, isPreview, reqWidth, reqHeight, padding, displayId, which);
+                    new IWallpaperEngineWrapper(mTarget, conn, windowToken, windowType,
+                            isPreview, reqWidth, reqHeight, padding, displayId, which);
+            mActiveEngines.put(windowToken, engineWrapper);
+            if (DEBUG) {
+                Slog.v(TAG, "IWallpaperServiceWrapper Attaching window token " + windowToken);
+            }
             Trace.endSection();
         }
 
         @Override
-        public void detach() {
+        public void detach(IBinder windowToken) {
-            mEngineWrapper.detach();
+            IWallpaperEngineWrapper engineWrapper = mActiveEngines.remove(windowToken);
+            if (engineWrapper == null) {
+                Log.w(TAG, "Engine for window token " + windowToken + " already detached");
+                return;
+            }
+            if (DEBUG) {
+                Slog.v(TAG, "IWallpaperServiceWrapper Detaching window token " + windowToken);
+            }
+            engineWrapper.destroy();
         }
     }
 
@@ -2558,8 +2562,8 @@ public abstract class WallpaperService extends Service {
     public void onDestroy() {
         Trace.beginSection("WPMS.onDestroy");
         super.onDestroy();
-        for (int i=0; i<mActiveEngines.size(); i++) {
+        for (IWallpaperEngineWrapper engineWrapper : mActiveEngines.values()) {
-            mActiveEngines.get(i).detach();
+            engineWrapper.destroy();
         }
         mActiveEngines.clear();
         Trace.endSection();
@@ -2586,8 +2590,12 @@ public abstract class WallpaperService extends Service {
     @Override
     protected void dump(FileDescriptor fd, PrintWriter out, String[] args) {
         out.print("State of wallpaper "); out.print(this); out.println(":");
-        for (int i=0; i<mActiveEngines.size(); i++) {
+        for (IWallpaperEngineWrapper engineWrapper : mActiveEngines.values()) {
-            Engine engine = mActiveEngines.get(i);
+            Engine engine = engineWrapper.mEngine;
+            if (engine == null) {
+                Slog.w(TAG, "Engine for wrapper " + engineWrapper + " not attached");
+                continue;
+            }
             out.print("  Engine "); out.print(engine); out.println(":");
             engine.dump("    ", fd, out, args);
         }

services/core/java/com/android/server/wallpaper/WallpaperManagerService.java

@@ -772,7 +772,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
                     DisplayConnector connector =
                             targetWallpaper.connection.getDisplayConnectorOrCreate(displayId);
                     if (connector == null) return;
-                    connector.disconnectLocked();
+                    connector.disconnectLocked(targetWallpaper.connection);
                     targetWallpaper.connection.removeDisplayConnector(displayId);
                     mWallpaperDisplayHelper.removeDisplayData(displayId);
                 }
@@ -859,7 +859,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
             if (fallbackConnection.mDisplayConnector.size() != 0) {
                 fallbackConnection.forEachDisplayConnector(connector -> {
                     if (connector.mEngine != null) {
-                        connector.disconnectLocked();
+                        connector.disconnectLocked(fallbackConnection);
                     }
                 });
                 fallbackConnection.mDisplayConnector.clear();
@@ -940,16 +940,14 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
             t.traceEnd();
         }
 
-        void disconnectLocked() {
+        void disconnectLocked(WallpaperConnection connection) {
             if (DEBUG) Slog.v(TAG, "Removing window token: " + mToken);
             mWindowManagerInternal.removeWindowToken(mToken, false/* removeWindows */,
                     mDisplayId);
             try {
-                if (mEngine != null) {
+                connection.mService.detach(mToken);
-                    mEngine.destroy();
-                }
             } catch (RemoteException e) {
-                Slog.w(TAG, "Engine.destroy() threw a RemoteException");
+                Slog.w(TAG, "connection.mService.destroy() threw a RemoteException");
             }
             mEngine = null;
         }
@@ -1249,12 +1247,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
             synchronized (mLock) {
                 final DisplayConnector connector = getDisplayConnectorOrCreate(displayId);
                 if (connector == null) {
-                    try {
+                    throw new IllegalStateException("Connector has already been destroyed");
-                        engine.destroy();
-                    } catch (RemoteException e) {
-                        Slog.w(TAG, "Failed to destroy engine", e);
-                    }
-                    return;
                 }
                 connector.mEngine = engine;
                 connector.ensureStatusHandled();
@@ -3261,20 +3254,8 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
                 }
                 wallpaper.connection.mReply = null;
             }
-            try {
+            wallpaper.connection.forEachDisplayConnector(
-                // It can be null if user switching happens before service connection.
+                    connector -> connector.disconnectLocked(wallpaper.connection));
-                if (wallpaper.connection.mService != null) {
-                    wallpaper.connection.mService.detach();
-                }
-            } catch (RemoteException e) {
-                Slog.w(TAG, "Failed detaching wallpaper service ", e);
-            }
-            try {
-                mContext.unbindService(wallpaper.connection);
-            } catch (IllegalArgumentException e) {
-                Slog.w(TAG, "Attempted to unbind unregistered service");
-            }
-            wallpaper.connection.forEachDisplayConnector(DisplayConnector::disconnectLocked);
             wallpaper.connection.mService = null;
             wallpaper.connection.mDisplayConnector.clear();
 
@@ -3284,6 +3265,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
             mContext.getMainThreadHandler().removeCallbacks(
                     wallpaper.connection.mTryToRebindRunnable);
 
+            mContext.unbindService(wallpaper.connection);
             wallpaper.connection = null;
             if (wallpaper == mLastWallpaper) {
                 mLastWallpaper = null;