Merge "Clean up previous DA organizer when registering" into sc-dev

services/core/java/com/android/server/wm/DisplayAreaOrganizerController.java

@@ -26,6 +26,7 @@ import android.content.pm.ParceledListSlice;
 import android.os.Binder;
 import android.os.IBinder;
 import android.os.RemoteException;
+import android.util.Slog;
 import android.view.SurfaceControl;
 import android.window.DisplayAreaAppearedInfo;
 import android.window.IDisplayAreaOrganizer;
@@ -49,7 +50,8 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
 
     final ActivityTaskManagerService mService;
     private final WindowManagerGlobalLock mGlobalLock;
-    private final HashMap<Integer, IDisplayAreaOrganizer> mOrganizersByFeatureIds = new HashMap();
+    private final HashMap<Integer, DisplayAreaOrganizerState> mOrganizersByFeatureIds =
+            new HashMap();
 
     private class DeathRecipient implements IBinder.DeathRecipient {
         int mFeature;
@@ -63,12 +65,41 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
         @Override
         public void binderDied() {
             synchronized (mGlobalLock) {
-                mOrganizersByFeatureIds.remove(mFeature);
+                mOrganizersByFeatureIds.remove(mFeature).destroy();
-                removeOrganizer(mOrganizer);
             }
         }
     }
 
+    private class DisplayAreaOrganizerState {
+        private final IDisplayAreaOrganizer mOrganizer;
+        private final DeathRecipient mDeathRecipient;
+
+        DisplayAreaOrganizerState(IDisplayAreaOrganizer organizer, int feature) {
+            mOrganizer = organizer;
+            mDeathRecipient = new DeathRecipient(organizer, feature);
+            try {
+                organizer.asBinder().linkToDeath(mDeathRecipient, 0);
+            } catch (RemoteException e) {
+                // Oh well...
+            }
+        }
+
+        void destroy() {
+            IBinder organizerBinder = mOrganizer.asBinder();
+            mService.mRootWindowContainer.forAllDisplayAreas((da) -> {
+                if (da.mOrganizer != null && da.mOrganizer.asBinder().equals(organizerBinder)) {
+                    if (da.isTaskDisplayArea() && da.asTaskDisplayArea().mCreatedByOrganizer) {
+                        // Delete the organizer created TDA when unregister.
+                        deleteTaskDisplayArea(da.asTaskDisplayArea());
+                    } else {
+                        da.setOrganizer(null);
+                    }
+                }
+            });
+            organizerBinder.unlinkToDeath(mDeathRecipient, 0);
+        }
+    }
+
     DisplayAreaOrganizerController(ActivityTaskManagerService atm) {
         mService = atm;
         mGlobalLock = atm.mGlobalLock;
@@ -80,7 +111,8 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
 
     @Nullable
     IDisplayAreaOrganizer getOrganizerByFeature(int featureId) {
-        return mOrganizersByFeatureIds.get(featureId);
+        final DisplayAreaOrganizerState state = mOrganizersByFeatureIds.get(featureId);
+        return state != null ? state.mOrganizer : null;
     }
 
     @Override
@@ -94,17 +126,18 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
                 ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER, "Register display organizer=%s uid=%d",
                         organizer.asBinder(), uid);
                 if (mOrganizersByFeatureIds.get(feature) != null) {
-                    throw new IllegalStateException(
+                    if (mOrganizersByFeatureIds.get(feature).mOrganizer.asBinder()
-                            "Replacing existing organizer currently unsupported");
+                            .isBinderAlive()) {
-                }
+                        throw new IllegalStateException(
-
+                                "Replacing existing organizer currently unsupported");
-                final DeathRecipient dr = new DeathRecipient(organizer, feature);
+                    }
-                try {
+
-                    organizer.asBinder().linkToDeath(dr, 0);
+                    mOrganizersByFeatureIds.remove(feature).destroy();
-                } catch (RemoteException e) {
+                    Slog.d(TAG, "Replacing dead organizer for feature=" + feature);
-                    // Oh well...
                 }
 
+                final DisplayAreaOrganizerState state = new DisplayAreaOrganizerState(organizer,
+                        feature);
                 final List<DisplayAreaAppearedInfo> displayAreaInfos = new ArrayList<>();
                 mService.mRootWindowContainer.forAllDisplays(dc -> {
                     if (!dc.isTrusted()) {
@@ -120,7 +153,7 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
                     });
                 });
 
-                mOrganizersByFeatureIds.put(feature, organizer);
+                mOrganizersByFeatureIds.put(feature, state);
                 return new ParceledListSlice<>(displayAreaInfos);
             }
         } finally {
@@ -137,9 +170,14 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
             synchronized (mGlobalLock) {
                 ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER, "Unregister display organizer=%s uid=%d",
                         organizer.asBinder(), uid);
-                mOrganizersByFeatureIds.entrySet().removeIf(
+                mOrganizersByFeatureIds.entrySet().removeIf((entry) -> {
-                        entry -> entry.getValue().asBinder() == organizer.asBinder());
+                    final boolean matches = entry.getValue().mOrganizer.asBinder()
-                removeOrganizer(organizer);
+                            == organizer.asBinder();
+                    if (matches) {
+                        entry.getValue().destroy();
+                    }
+                    return matches;
+                });
             }
         } finally {
             Binder.restoreCallingIdentity(origId);
@@ -190,19 +228,15 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
                 }
 
                 final int taskDisplayAreaFeatureId = mNextTaskDisplayAreaFeatureId++;
-                final DeathRecipient dr = new DeathRecipient(organizer, taskDisplayAreaFeatureId);
+                final DisplayAreaOrganizerState state = new DisplayAreaOrganizerState(organizer,
-                try {
+                        taskDisplayAreaFeatureId);
-                    organizer.asBinder().linkToDeath(dr, 0);
-                } catch (RemoteException e) {
-                    // Oh well...
-                }
 
                 final TaskDisplayArea tda = parentRoot != null
                         ? createTaskDisplayArea(parentRoot, name, taskDisplayAreaFeatureId)
                         : createTaskDisplayArea(parentTda, name, taskDisplayAreaFeatureId);
                 final DisplayAreaAppearedInfo tdaInfo = organizeDisplayArea(organizer, tda,
                         "DisplayAreaOrganizerController.createTaskDisplayArea");
-                mOrganizersByFeatureIds.put(taskDisplayAreaFeatureId, organizer);
+                mOrganizersByFeatureIds.put(taskDisplayAreaFeatureId, state);
                 return tdaInfo;
             }
         } finally {
@@ -230,8 +264,7 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
                                     + "TaskDisplayArea=" + taskDisplayArea);
                 }
 
-                mOrganizersByFeatureIds.remove(taskDisplayArea.mFeatureId);
+                mOrganizersByFeatureIds.remove(taskDisplayArea.mFeatureId).destroy();
-                deleteTaskDisplayArea(taskDisplayArea);
             }
         } finally {
             Binder.restoreCallingIdentity(origId);
@@ -251,6 +284,10 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
 
     void onDisplayAreaVanished(IDisplayAreaOrganizer organizer, DisplayArea da) {
         ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER, "DisplayArea vanished name=%s", da.getName());
+        if (!organizer.asBinder().isBinderAlive()) {
+            Slog.d(TAG, "Organizer died before sending onDisplayAreaVanished");
+            return;
+        }
         try {
             organizer.onDisplayAreaVanished(da.getDisplayAreaInfo());
         } catch (RemoteException e) {
@@ -267,20 +304,6 @@ public class DisplayAreaOrganizerController extends IDisplayAreaOrganizerControl
         }
     }
 
-    private void removeOrganizer(IDisplayAreaOrganizer organizer) {
-        IBinder organizerBinder = organizer.asBinder();
-        mService.mRootWindowContainer.forAllDisplayAreas((da) -> {
-            if (da.mOrganizer != null && da.mOrganizer.asBinder().equals(organizerBinder)) {
-                if (da.isTaskDisplayArea() && da.asTaskDisplayArea().mCreatedByOrganizer) {
-                    // Delete the organizer created TDA when unregister.
-                    deleteTaskDisplayArea(da.asTaskDisplayArea());
-                } else {
-                    da.setOrganizer(null);
-                }
-            }
-        });
-    }
-
     private DisplayAreaAppearedInfo organizeDisplayArea(IDisplayAreaOrganizer organizer,
             DisplayArea displayArea, String callsite) {
         displayArea.setOrganizer(organizer, true /* skipDisplayAreaAppeared */);

services/tests/wmtests/src/com/android/server/wm/DisplayAreaTest.java

@@ -24,6 +24,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_PRESENTATION;
 import static android.view.WindowManager.LayoutParams.TYPE_WALLPAPER;
 import static android.window.DisplayAreaOrganizer.FEATURE_DEFAULT_TASK_CONTAINER;
 import static android.window.DisplayAreaOrganizer.FEATURE_VENDOR_FIRST;
+import static android.window.DisplayAreaOrganizer.FEATURE_VENDOR_LAST;
 
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
@@ -57,6 +58,7 @@ import android.content.pm.ActivityInfo;
 import android.content.res.Configuration;
 import android.graphics.Rect;
 import android.os.Binder;
+import android.os.IBinder;
 import android.platform.test.annotations.Presubmit;
 import android.view.SurfaceControl;
 import android.view.View;
@@ -555,6 +557,7 @@ public class DisplayAreaTest extends WindowTestsBase {
         final DisplayArea<WindowContainer> displayArea = new DisplayArea<>(
                 mWm, BELOW_TASKS, "NewArea", FEATURE_VENDOR_FIRST);
         final IDisplayAreaOrganizer mockDisplayAreaOrganizer = mock(IDisplayAreaOrganizer.class);
+        doReturn(mock(IBinder.class)).when(mockDisplayAreaOrganizer).asBinder();
         displayArea.mOrganizer = mockDisplayAreaOrganizer;
         spyOn(mWm.mAtmService.mWindowOrganizerController.mDisplayAreaOrganizerController);
         mDisplayContent.addChild(displayArea, 0);
@@ -566,6 +569,30 @@ public class DisplayAreaTest extends WindowTestsBase {
                 .onDisplayAreaVanished(mockDisplayAreaOrganizer, displayArea);
     }
 
+    @Test
+    public void testRegisterSameFeatureOrganizer_expectThrowsException() {
+        final IDisplayAreaOrganizer mockDisplayAreaOrganizer = mock(IDisplayAreaOrganizer.class);
+        final IBinder binder = mock(IBinder.class);
+        doReturn(true).when(binder).isBinderAlive();
+        doReturn(binder).when(mockDisplayAreaOrganizer).asBinder();
+        final DisplayAreaOrganizerController controller =
+                mWm.mAtmService.mWindowOrganizerController.mDisplayAreaOrganizerController;
+        controller.registerOrganizer(mockDisplayAreaOrganizer, FEATURE_VENDOR_FIRST);
+        assertThrows(IllegalStateException.class,
+                () -> controller.registerOrganizer(mockDisplayAreaOrganizer, FEATURE_VENDOR_FIRST));
+    }
+
+    @Test
+    public void testRegisterUnregisterOrganizer() {
+        final IDisplayAreaOrganizer mockDisplayAreaOrganizer = mock(IDisplayAreaOrganizer.class);
+        doReturn(mock(IBinder.class)).when(mockDisplayAreaOrganizer).asBinder();
+        final DisplayAreaOrganizerController controller =
+                mWm.mAtmService.mWindowOrganizerController.mDisplayAreaOrganizerController;
+        controller.registerOrganizer(mockDisplayAreaOrganizer, FEATURE_VENDOR_FIRST);
+        controller.unregisterOrganizer(mockDisplayAreaOrganizer);
+        controller.registerOrganizer(mockDisplayAreaOrganizer, FEATURE_VENDOR_FIRST);
+    }
+
     private static class TestDisplayArea<T extends WindowContainer> extends DisplayArea<T> {
         private TestDisplayArea(WindowManagerService wms, Rect bounds) {
             super(wms, ANY, "half display area");