Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit af38551f authored by Kevin Chyn's avatar Kevin Chyn Committed by Android (Google) Code Review
Browse files

Merge changes I5050e67a,I1099cf76 into tm-qpr-dev

* changes:
  Address thread safety issues in WindowLayoutComponentImpl
  Identify and fix potential thread safety issues in SplitController
parents 0c13a67d 6883c9aa
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -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)) {
+34 −11
Original line number Diff line number Diff line
@@ -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
+7 −11
Original line number Diff line number Diff line
@@ -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,
            @NonNull Activity secondaryActivity) {
    private static Pair<Size, Size> getActivitiesMinDimensionsPair(
            @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;
+3 −0
Original line number Diff line number Diff line
@@ -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
+76 −50
Original line number Diff line number Diff line
@@ -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
            // receive callbacks from NotifyOnConfigurationChanged instead.
                // We register component callbacks for window contexts. For activity contexts, they
                // 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.
            Consumer<WindowLayoutInfo> layoutConsumer = mWindowLayoutChangeListeners.get(context);
                // Get the WindowLayoutInfo from the activity and pass the value to the
                // 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() {}