Merge changes I5050e67a,I1099cf76 into tm-qpr-dev

* changes:
  Address thread safety issues in WindowLayoutComponentImpl
  Identify and fix potential thread safety issues in SplitController

libs/WindowManager/Jetpack/src/androidx/window/common/DeviceStateManagerFoldingFeatureProducer.java

@@ -155,6 +155,7 @@ public final class DeviceStateManagerFoldingFeatureProducer
      * Adds the data to the storeFeaturesConsumer when the data is ready.
      * @param storeFeaturesConsumer a consumer to collect the data when it is first available.
      */
+    @Override
     public void getData(Consumer<List<CommonFoldingFeature>> storeFeaturesConsumer) {
         mRawFoldSupplier.getData((String displayFeaturesString) -> {
             if (TextUtils.isEmpty(displayFeaturesString)) {

libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java

@@ -108,6 +108,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     // Currently applied split configuration.
     @GuardedBy("mLock")
     private final List<EmbeddingRule> mSplitRules = new ArrayList<>();
+
     /**
      * A developer-defined {@link SplitAttributes} calculator to compute the current
      * {@link SplitAttributes} with the current device and window states.
@@ -125,6 +126,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     @GuardedBy("mLock")
     @Nullable
     private SplitAttributesCalculator mSplitAttributesCalculator;
+
     /**
      * Map from Task id to {@link TaskContainer} which contains all TaskFragment and split pair info
      * below it.
@@ -230,6 +232,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     }
 
     @NonNull
+    @GuardedBy("mLock")
+    @VisibleForTesting
     List<EmbeddingRule> getSplitRules() {
         return mSplitRules;
     }
@@ -246,7 +250,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     }
 
     /**
-     * Clears the listener set in {@link SplitController#setSplitInfoListener}.
+     * Clears the listener set in {@link SplitController#setSplitInfoCallback(Consumer)}.
      */
     @Override
     public void clearSplitInfoCallback() {
@@ -466,6 +470,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         updateContainersInTask(wct, taskContainer);
     }
 
+    @GuardedBy("mLock")
     void updateContainersInTaskIfVisible(@NonNull WindowContainerTransaction wct, int taskId) {
         final TaskContainer taskContainer = getTaskContainer(taskId);
         if (taskContainer != null && taskContainer.isVisible()) {
@@ -473,6 +478,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         }
     }
 
+    @GuardedBy("mLock")
     private void updateContainersInTask(@NonNull WindowContainerTransaction wct,
             @NonNull TaskContainer taskContainer) {
         // Update all TaskFragments in the Task. Make a copy of the list since some may be
@@ -756,6 +762,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     /**
      * Places the given activity to the top most TaskFragment in the task if there is any.
      */
+    @GuardedBy("mLock")
     @VisibleForTesting
     void placeActivityInTopContainer(@NonNull WindowContainerTransaction wct,
             @NonNull Activity activity) {
@@ -879,6 +886,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     /** Finds the activity below the given activity. */
     @VisibleForTesting
     @Nullable
+    @GuardedBy("mLock")
     Activity findActivityBelow(@NonNull Activity activity) {
         Activity activityBelow = null;
         final TaskFragmentContainer container = getContainerWithActivity(activity);
@@ -1213,6 +1221,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         return null;
     }
 
+    @GuardedBy("mLock")
     TaskFragmentContainer newContainer(@NonNull Activity pendingAppearedActivity, int taskId) {
         return newContainer(pendingAppearedActivity, pendingAppearedActivity, taskId);
     }
@@ -1350,7 +1359,12 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
      * Removes a secondary container for the given primary container if an existing split is
      * already registered.
      */
-    void removeExistingSecondaryContainers(@NonNull WindowContainerTransaction wct,
+    // Suppress GuardedBy warning because lint asks to mark this method as
+    // @GuardedBy(existingSplitContainer.getSecondaryContainer().mController.mLock), which is mLock
+    // itself
+    @SuppressWarnings("GuardedBy")
+    @GuardedBy("mLock")
+    private void removeExistingSecondaryContainers(@NonNull WindowContainerTransaction wct,
             @NonNull TaskFragmentContainer primaryContainer) {
         // If the primary container was already in a split - remove the secondary container that
         // is now covered by the new one that replaced it.
@@ -1368,6 +1382,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     /**
      * Returns the topmost not finished container in Task of given task id.
      */
+    @GuardedBy("mLock")
     @Nullable
     TaskFragmentContainer getTopActiveContainer(int taskId) {
         final TaskContainer taskContainer = mTaskContainers.get(taskId);
@@ -1737,6 +1752,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     }
 
     @Nullable
+    @GuardedBy("mLock")
     TaskFragmentContainer getContainer(@NonNull IBinder fragmentToken) {
         for (int i = mTaskContainers.size() - 1; i >= 0; i--) {
             final List<TaskFragmentContainer> containers = mTaskContainers.valueAt(i).mContainers;
@@ -1750,6 +1766,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     }
 
     @Nullable
+    @GuardedBy("mLock")
     TaskContainer getTaskContainer(int taskId) {
         return mTaskContainers.get(taskId);
     }
@@ -1758,6 +1775,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         return mHandler;
     }
 
+    @GuardedBy("mLock")
     int getTaskId(@NonNull Activity activity) {
         // Prefer to get the taskId from TaskFragmentContainer because Activity.getTaskId() is an
         // IPC call.
@@ -1850,6 +1868,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     /**
      * @see #shouldRetainAssociatedContainer(TaskFragmentContainer, TaskFragmentContainer)
      */
+    @GuardedBy("mLock")
     boolean shouldRetainAssociatedActivity(@NonNull TaskFragmentContainer finishingContainer,
             @NonNull Activity associatedActivity) {
         final TaskFragmentContainer associatedContainer = getContainerWithActivity(
@@ -1970,6 +1989,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     @VisibleForTesting
     class ActivityStartMonitor extends Instrumentation.ActivityMonitor {
         @VisibleForTesting
+        @GuardedBy("mLock")
         Intent mCurrentIntent;
 
         @Override
@@ -2034,8 +2054,10 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         @Override
         public void onStartActivityResult(int result, @NonNull Bundle bOptions) {
             super.onStartActivityResult(result, bOptions);
+            synchronized (mLock) {
                 if (mCurrentIntent != null && result != START_SUCCESS) {
-                // Clear the pending appeared intent if the activity was not started successfully.
+                    // Clear the pending appeared intent if the activity was not started
+                    // successfully.
                     final IBinder token = bOptions.getBinder(
                             ActivityOptions.KEY_LAUNCH_TASK_FRAGMENT_TOKEN);
                     if (token != null) {
@@ -2048,6 +2070,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
                 mCurrentIntent = null;
             }
         }
+    }
 
     /**
      * Checks if an activity is embedded and its presentation is customized by a

libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java

@@ -39,7 +39,6 @@ import android.view.WindowMetrics;
 import android.window.TaskFragmentCreationParams;
 import android.window.WindowContainerTransaction;
 
-import androidx.annotation.GuardedBy;
 import androidx.annotation.IntDef;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -63,7 +62,10 @@ import java.util.concurrent.Executor;
 /**
  * Controls the visual presentation of the splits according to the containers formed by
  * {@link SplitController}.
+ *
+ * Note that all calls into this class must hold the {@link SplitController} internal lock.
  */
+@SuppressWarnings("GuardedBy")
 class SplitPresenter extends JetpackTaskFragmentOrganizer {
     @VisibleForTesting
     static final int POSITION_START = 0;
@@ -163,7 +165,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
      * @return The newly created secondary container.
      */
     @NonNull
-    @GuardedBy("mController.mLock")
     TaskFragmentContainer createNewSplitWithEmptySideContainer(
             @NonNull WindowContainerTransaction wct, @NonNull Activity primaryActivity,
             @NonNull Intent secondaryIntent, @NonNull SplitPairRule rule) {
@@ -210,7 +211,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
      *                          created and the activity will be re-parented to it.
      * @param rule The split rule to be applied to the container.
      */
-    @GuardedBy("mController.mLock")
     void createNewSplitContainer(@NonNull WindowContainerTransaction wct,
             @NonNull Activity primaryActivity, @NonNull Activity secondaryActivity,
             @NonNull SplitPairRule rule) {
@@ -285,7 +285,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
      * @param rule              The split rule to be applied to the container.
      * @param isPlaceholder     Whether the launch is a placeholder.
      */
-    @GuardedBy("mController.mLock")
     void startActivityToSide(@NonNull WindowContainerTransaction wct,
             @NonNull Activity launchingActivity, @NonNull Intent activityIntent,
             @Nullable Bundle activityOptions, @NonNull SplitRule rule,
@@ -328,7 +327,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
      * @param updatedContainer The task fragment that was updated and caused this split update.
      * @param wct WindowContainerTransaction that this update should be performed with.
      */
-    @GuardedBy("mController.mLock")
     void updateSplitContainer(@NonNull SplitContainer splitContainer,
             @NonNull TaskFragmentContainer updatedContainer,
             @NonNull WindowContainerTransaction wct) {
@@ -369,7 +367,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
         updateTaskFragmentWindowingModeIfRegistered(wct, secondaryContainer, windowingMode);
     }
 
-    @GuardedBy("mController.mLock")
     private void setAdjacentTaskFragments(@NonNull WindowContainerTransaction wct,
             @NonNull TaskFragmentContainer primaryContainer,
             @NonNull TaskFragmentContainer secondaryContainer, @NonNull SplitRule splitRule,
@@ -393,7 +390,7 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
      * creation has not been reported from the server yet.
      */
     // TODO(b/190433398): Handle resize if the fragment hasn't appeared yet.
-    void resizeTaskFragmentIfRegistered(@NonNull WindowContainerTransaction wct,
+    private void resizeTaskFragmentIfRegistered(@NonNull WindowContainerTransaction wct,
             @NonNull TaskFragmentContainer container,
             @Nullable Rect bounds) {
         if (container.getInfo() == null) {
@@ -520,7 +517,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
         return !(splitAttributes.getSplitType() instanceof ExpandContainersSplitType);
     }
 
-    @GuardedBy("mController.mLock")
     @NonNull
     SplitAttributes computeSplitAttributes(@NonNull TaskProperties taskProperties,
             @NonNull SplitRule rule, @Nullable Pair<Size, Size> minDimensionsPair) {
@@ -572,8 +568,8 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
     }
 
     @NonNull
-    static Pair<Size, Size> getActivitiesMinDimensionsPair(@NonNull Activity primaryActivity,
+    private static Pair<Size, Size> getActivitiesMinDimensionsPair(
-            @NonNull Activity secondaryActivity) {
+            @NonNull Activity primaryActivity, @NonNull Activity secondaryActivity) {
         return new Pair<>(getMinDimensions(primaryActivity), getMinDimensions(secondaryActivity));
     }
 
@@ -619,7 +615,7 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
         return new Size(windowLayout.minWidth, windowLayout.minHeight);
     }
 
-    static boolean boundsSmallerThanMinDimensions(@NonNull Rect bounds,
+    private static boolean boundsSmallerThanMinDimensions(@NonNull Rect bounds,
             @Nullable Size minDimensions) {
         if (minDimensions == null) {
             return false;

libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java

@@ -253,6 +253,7 @@ class TaskFragmentContainer {
         mPendingAppearedActivities.remove(activityToken);
     }
 
+    @GuardedBy("mController.mLock")
     void clearPendingAppearedActivities() {
         final List<IBinder> cleanupActivities = new ArrayList<>(mPendingAppearedActivities);
         // Clear mPendingAppearedActivities so that #getContainerWithActivity won't return the
@@ -452,6 +453,7 @@ class TaskFragmentContainer {
      * Removes all activities that belong to this process and finishes other containers/activities
      * configured to finish together.
      */
+    @GuardedBy("mController.mLock")
     void finish(boolean shouldFinishDependent, @NonNull SplitPresenter presenter,
             @NonNull WindowContainerTransaction wct, @NonNull SplitController controller) {
         if (!mIsFinished) {
@@ -476,6 +478,7 @@ class TaskFragmentContainer {
         mInfo = null;
     }
 
+    @GuardedBy("mController.mLock")
     private void finishActivities(boolean shouldFinishDependent, @NonNull SplitPresenter presenter,
             @NonNull WindowContainerTransaction wct, @NonNull SplitController controller) {
         // Finish own activities

libs/WindowManager/Jetpack/src/androidx/window/extensions/layout/WindowLayoutComponentImpl.java

@@ -36,6 +36,7 @@ import android.os.Bundle;
 import android.os.IBinder;
 import android.util.ArrayMap;
 
+import androidx.annotation.GuardedBy;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.UiContext;
@@ -63,13 +64,19 @@ import java.util.function.Consumer;
 public class WindowLayoutComponentImpl implements WindowLayoutComponent {
     private static final String TAG = "SampleExtension";
 
+    private final Object mLock = new Object();
+
+    @GuardedBy("mLock")
     private final Map<Context, Consumer<WindowLayoutInfo>> mWindowLayoutChangeListeners =
             new ArrayMap<>();
 
+    @GuardedBy("mLock")
     private final DataProducer<List<CommonFoldingFeature>> mFoldingFeatureProducer;
 
+    @GuardedBy("mLock")
     private final List<CommonFoldingFeature> mLastReportedFoldingFeatures = new ArrayList<>();
 
+    @GuardedBy("mLock")
     private final Map<IBinder, ConfigurationChangeListener> mConfigurationChangeListeners =
             new ArrayMap<>();
 
@@ -84,8 +91,10 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
 
     /** Registers to listen to {@link CommonFoldingFeature} changes */
     public void addFoldingStateChangedCallback(Consumer<List<CommonFoldingFeature>> consumer) {
+        synchronized (mLock) {
             mFoldingFeatureProducer.addDataChangedCallback(consumer);
         }
+    }
 
     /**
      * Adds a listener interested in receiving updates to {@link WindowLayoutInfo}
@@ -113,8 +122,10 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
     @Override
     public void addWindowLayoutInfoListener(@NonNull @UiContext Context context,
             @NonNull Consumer<WindowLayoutInfo> consumer) {
+        synchronized (mLock) {
             if (mWindowLayoutChangeListeners.containsKey(context)
-                // In theory this method can be called on the same consumer with different context.
+                    // In theory this method can be called on the same consumer with different
+                    // context.
                     || mWindowLayoutChangeListeners.containsValue(consumer)) {
                 return;
             }
@@ -130,14 +141,15 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
 
             final IBinder windowContextToken = context.getWindowContextToken();
             if (windowContextToken != null) {
-            // We register component callbacks for window contexts. For activity contexts, they will
+                // We register component callbacks for window contexts. For activity contexts, they
-            // receive callbacks from NotifyOnConfigurationChanged instead.
+                // will receive callbacks from NotifyOnConfigurationChanged instead.
                 final ConfigurationChangeListener listener =
                         new ConfigurationChangeListener(windowContextToken);
                 context.registerComponentCallbacks(listener);
                 mConfigurationChangeListeners.put(windowContextToken, listener);
             }
         }
+    }
 
     /**
      * Removes a listener no longer interested in receiving updates.
@@ -146,6 +158,7 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
      */
     @Override
     public void removeWindowLayoutInfoListener(@NonNull Consumer<WindowLayoutInfo> consumer) {
+        synchronized (mLock) {
             for (Context context : mWindowLayoutChangeListeners.keySet()) {
                 if (!mWindowLayoutChangeListeners.get(context).equals(consumer)) {
                     continue;
@@ -159,12 +172,15 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
             }
             mWindowLayoutChangeListeners.values().remove(consumer);
         }
+    }
 
+    @GuardedBy("mLock")
     @NonNull
-    Set<Context> getContextsListeningForLayoutChanges() {
+    private Set<Context> getContextsListeningForLayoutChanges() {
         return mWindowLayoutChangeListeners.keySet();
     }
 
+    @GuardedBy("mLock")
     private boolean isListeningForLayoutChanges(IBinder token) {
         for (Context context: getContextsListeningForLayoutChanges()) {
             if (token.equals(Context.getToken(context))) {
@@ -174,10 +190,6 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
         return false;
     }
 
-    protected boolean hasListeners() {
-        return !mWindowLayoutChangeListeners.isEmpty();
-    }
-
     /**
      * A convenience method to translate from the common feature state to the extensions feature
      * state.  More specifically, translates from {@link CommonFoldingFeature.State} to
@@ -201,15 +213,19 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
     }
 
     private void onDisplayFeaturesChanged(List<CommonFoldingFeature> storedFeatures) {
+        synchronized (mLock) {
             mLastReportedFoldingFeatures.clear();
             mLastReportedFoldingFeatures.addAll(storedFeatures);
             for (Context context : getContextsListeningForLayoutChanges()) {
-            // Get the WindowLayoutInfo from the activity and pass the value to the layoutConsumer.
+                // Get the WindowLayoutInfo from the activity and pass the value to the
-            Consumer<WindowLayoutInfo> layoutConsumer = mWindowLayoutChangeListeners.get(context);
+                // layoutConsumer.
+                Consumer<WindowLayoutInfo> layoutConsumer = mWindowLayoutChangeListeners.get(
+                        context);
                 WindowLayoutInfo newWindowLayout = getWindowLayoutInfo(context, storedFeatures);
                 layoutConsumer.accept(newWindowLayout);
             }
         }
+    }
 
     /**
      * Translates the {@link DisplayFeature} into a {@link WindowLayoutInfo} when a
@@ -232,7 +248,10 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
     @NonNull
     public WindowLayoutInfo getCurrentWindowLayoutInfo(int displayId,
             @NonNull WindowConfiguration windowConfiguration) {
-        return getWindowLayoutInfo(displayId, windowConfiguration, mLastReportedFoldingFeatures);
+        synchronized (mLock) {
+            return getWindowLayoutInfo(displayId, windowConfiguration,
+                    mLastReportedFoldingFeatures);
+        }
     }
 
     /** @see #getWindowLayoutInfo(Context, List)  */
@@ -330,6 +349,7 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
         return !WindowConfiguration.inMultiWindowMode(windowingMode);
     }
 
+    @GuardedBy("mLock")
     private void onDisplayFeaturesChangedIfListening(@NonNull IBinder token) {
         if (isListeningForLayoutChanges(token)) {
             mFoldingFeatureProducer.getData(
@@ -341,15 +361,19 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
         @Override
         public void onActivityCreated(Activity activity, Bundle savedInstanceState) {
             super.onActivityCreated(activity, savedInstanceState);
+            synchronized (mLock) {
                 onDisplayFeaturesChangedIfListening(activity.getActivityToken());
             }
+        }
 
         @Override
         public void onActivityConfigurationChanged(Activity activity) {
             super.onActivityConfigurationChanged(activity);
+            synchronized (mLock) {
                 onDisplayFeaturesChangedIfListening(activity.getActivityToken());
             }
         }
+    }
 
     private final class ConfigurationChangeListener implements ComponentCallbacks {
         final IBinder mToken;
@@ -360,8 +384,10 @@ public class WindowLayoutComponentImpl implements WindowLayoutComponent {
 
         @Override
         public void onConfigurationChanged(@NonNull Configuration newConfig) {
+            synchronized (mLock) {
                 onDisplayFeaturesChangedIfListening(mToken);
             }
+        }
 
         @Override
         public void onLowMemory() {}