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

Commit 7f071b98 authored by Mark Renouf's avatar Mark Renouf
Browse files

Scroll capture search: reject covered scroll containers

This change ensures Views are visited in drawing order, respecting
custom drawing order and View z-values. Then, when selecting the best
target, any scrolling view is rejected if any other scrolling view
is drawn over it.

This handles the case where a slightly smaller scroll container is
created atop a larger one, such as when user navigation creates a
new set of interface which is added to the top of the window, leaving
the previous UI in place below.

Bug: 365969802
Bug: 189827634
Fix: 365969802
Test: atest ScrollCaptureSearchResultsTest ViewGroupScrollCaptureTest
Flag: android.view.flags.scroll_capture_target_z_order_fix
Change-Id: I2341266a63d46c0bd68bb5325449bc06681816e0
parent c96bfc49
Loading
Loading
Loading
Loading
+74 −3
Original line number Diff line number Diff line
@@ -16,10 +16,16 @@

package android.view;

import static android.view.flags.Flags.scrollCaptureTargetZOrderFix;

import static java.util.Comparator.comparing;
import static java.util.Objects.requireNonNull;
import static java.util.Objects.requireNonNullElse;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.UiThread;
import android.graphics.Point;
import android.graphics.Rect;
import android.os.CancellationSignal;
import android.util.IndentingPrintWriter;
@@ -113,7 +119,9 @@ public final class ScrollCaptureSearchResults {

    private void signalComplete() {
        mComplete = true;
        if (!scrollCaptureTargetZOrderFix()) {
            mTargets.sort(PRIORITY_ORDER);
        }
        if (mOnCompleteListener != null) {
            mOnCompleteListener.run();
            mOnCompleteListener = null;
@@ -125,15 +133,74 @@ public final class ScrollCaptureSearchResults {
        return new ArrayList<>(mTargets);
    }

    private Rect getScrollBoundsInWindow(@Nullable ScrollCaptureTarget target) {
        if (target == null || target.getScrollBounds() == null) {
            return new Rect();
        }
        Rect windowRect = new Rect(target.getScrollBounds());
        Point windowPosition = target.getPositionInWindow();
        windowRect.offset(windowPosition.x, windowPosition.y);
        return windowRect;
    }

    /**
     * Get the top ranked result out of all completed requests.
     *
     * @return the top ranked result
     */
    @Nullable
    public ScrollCaptureTarget getTopResult() {
        if (!scrollCaptureTargetZOrderFix()) {
            ScrollCaptureTarget target = mTargets.isEmpty() ? null : mTargets.get(0);
            return target != null && target.getScrollBounds() != null ? target : null;
        }
        List<ScrollCaptureTarget> filtered = new ArrayList<>();

        mTargets.removeIf(a -> nullOrEmpty(a.getScrollBounds()));

        // Remove scroll targets obscured or covered by other scrolling views.
        nextTarget:
        for (int i = 0; i <  mTargets.size(); i++) {
            ScrollCaptureTarget current = mTargets.get(i);

            View currentView = current.getContainingView();

            // Nested scroll containers:
            // Check if the next view is a child of the current. If so, skip the current.
            if (i + 1 < mTargets.size()) {
                ScrollCaptureTarget next = mTargets.get(i + 1);
                View nextView = next.getContainingView();
                // Honor explicit include hint on parent as escape hatch (unless both have it)
                if (isDescendant(currentView, nextView)
                        && (!hasIncludeHint(currentView) || hasIncludeHint(nextView))) {
                    continue;
                }
            }

            // Check if any views will be drawn partially or fully over this one.
            for (int j = i + 1; j < mTargets.size(); j++) {
                ScrollCaptureTarget above = mTargets.get(j);
                if (Rect.intersects(getScrollBoundsInWindow(current),
                        getScrollBoundsInWindow(above))) {
                    continue nextTarget;
                }
            }

            filtered.add(current);
        }

        // natural order, false->true
        Comparator<ScrollCaptureTarget> byIncludeHintPresence = comparing(
                ScrollCaptureSearchResults::hasIncludeHint);

        // natural order, smallest->largest area
        Comparator<ScrollCaptureTarget> byArea = comparing(
                target -> area(requireNonNullElse(target.getScrollBounds(), new Rect())));

        // The top result is the last one (with include hint if present, then by largest area)
        filtered.sort(byIncludeHintPresence.thenComparing(byArea));
        return filtered.isEmpty() ? null : filtered.getLast();
    }

    private class SearchRequest implements Consumer<Rect> {
        private ScrollCaptureTarget mTarget;
@@ -226,6 +293,10 @@ public final class ScrollCaptureSearchResults {
        return r == null || r.isEmpty();
    }

    private static boolean hasIncludeHint(ScrollCaptureTarget target) {
        return hasIncludeHint(target.getContainingView());
    }

    private static boolean hasIncludeHint(View view) {
        return (view.getScrollCaptureHint() & View.SCROLL_CAPTURE_HINT_INCLUDE) != 0;
    }
+33 −4
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static android.view.WindowInsetsAnimation.Callback.DISPATCH_MODE_CONTINUE
import static android.view.WindowInsetsAnimation.Callback.DISPATCH_MODE_STOP;
import static android.view.flags.Flags.FLAG_TOOLKIT_VIEWGROUP_SET_REQUESTED_FRAME_RATE_API;
import static android.view.flags.Flags.toolkitViewgroupSetRequestedFrameRateApi;
import static android.view.flags.Flags.scrollCaptureTargetZOrderFix;

import android.animation.LayoutTransition;
import android.annotation.CallSuper;
@@ -7657,6 +7658,11 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
            @NonNull Rect localVisibleRect, @NonNull Point windowOffset,
            @NonNull Consumer<ScrollCaptureTarget> targets) {

        // Only visible views can be captured.
        if (getVisibility() != View.VISIBLE) {
            return;
        }

        if (getClipToPadding() && !localVisibleRect.intersect(mPaddingLeft, mPaddingTop,
                    (mRight - mLeft)  - mPaddingRight, (mBottom - mTop) - mPaddingBottom)) {
            return;
@@ -7665,19 +7671,39 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
        // Dispatch to self first.
        super.dispatchScrollCaptureSearch(localVisibleRect, windowOffset, targets);

        final int childrenCount = mChildrenCount;
        if (childrenCount == 0) {
            return;
        }

        // Skip children if descendants excluded.
        if ((getScrollCaptureHint() & SCROLL_CAPTURE_HINT_EXCLUDE_DESCENDANTS) != 0) {
            return;
        }

        final Rect tmpRect = getTempRect();
        final int childCount = getChildCount();
        for (int i = 0; i < childCount; i++) {
            View child = getChildAt(i);

        ArrayList<View> preorderedList = null;
        boolean customOrder = false;
        if (scrollCaptureTargetZOrderFix()) {
            preorderedList = buildOrderedChildList();
            customOrder = preorderedList == null && isChildrenDrawingOrderEnabled();
        }
        final View[] children = mChildren;
        for (int i = 0; i < childrenCount; i++) {
            View child;
            if (scrollCaptureTargetZOrderFix()) {
                // Traverse children in the same order they will be drawn (honors Z if set)
                final int childIndex = getAndVerifyPreorderedIndex(childrenCount, i, customOrder);
                child = getAndVerifyPreorderedView(preorderedList, children, childIndex);
            } else {
                child = children[i];
            }

            // Only visible views can be captured.
            if (child.getVisibility() != View.VISIBLE) {
                continue;
            }

            // Offset the given rectangle (in parent's local coordinates) into child's coordinate
            // space and clip the result to the child View's bounds, padding and clipRect as needed.
            // If the resulting rectangle is not empty, the request is forwarded to the child.
@@ -7706,6 +7732,9 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
                child.dispatchScrollCaptureSearch(tmpRect, childWindowOffset, targets);
            }
        }
        if (preorderedList != null) {
            preorderedList.clear();
        }
    }

    /**
+13 −0
Original line number Diff line number Diff line
package: "android.view.flags"
container: "system"

flag {
    name: "scroll_capture_target_z_order_fix"
    namespace: "system_ui"
    description: "Always prefer targets with higher z-order"
    bug: "365969802"
    metadata {
        purpose: PURPOSE_BUGFIX
    }
}
+88 −33
Original line number Diff line number Diff line
@@ -16,9 +16,9 @@

package android.view;

import static androidx.test.InstrumentationRegistry.getTargetContext;
import static android.view.flags.Flags.FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX;

import static com.google.common.truth.Truth.assertThat;
import static androidx.test.InstrumentationRegistry.getTargetContext;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
@@ -32,13 +32,16 @@ import android.graphics.Point;
import android.graphics.Rect;
import android.os.CancellationSignal;
import android.os.SystemClock;
import android.platform.test.annotations.EnableFlags;
import android.platform.test.annotations.Presubmit;
import android.platform.test.flag.junit.SetFlagsRule;

import androidx.annotation.NonNull;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest;

import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

@@ -56,6 +59,8 @@ import java.util.function.Consumer;
@RunWith(AndroidJUnit4.class)
public class ScrollCaptureSearchResultsTest {

    @Rule
    public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();

    private static final Rect EMPTY_RECT = new Rect();
    private static final String TAG = "Test";
@@ -98,6 +103,45 @@ public class ScrollCaptureSearchResultsTest {
        assertNull("Expected null due to no valid targets", results.getTopResult());
    }

    /**
     * A scrolling target should be excluded even when larger if it will be drawn over by another
     * scrolling target.
     */
    @EnableFlags(FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX)
    @Test
    public void testCoveredTargetsAreExcluded() {
        ScrollCaptureSearchResults results = new ScrollCaptureSearchResults(mDirectExec);

        FakeScrollCaptureCallback callback1 = new FakeScrollCaptureCallback(mDirectExec);
        callback1.setScrollBounds(new Rect(0, 0, 200, 200)); // 200 tall
        View view1 = new FakeView(getTargetContext(), 0, 0, 200, 200, 1);
        ScrollCaptureTarget target1 = createTargetWithView(view1, callback1,
                new Rect(0, 0, 200, 200), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO);

        FakeScrollCaptureCallback callback2 = new FakeScrollCaptureCallback(mDirectExec);
        callback2.setScrollBounds(new Rect(0, 0, 200, 180)); // 180 tall
        View view2 = new FakeView(getTargetContext(), 0, 20, 200, 200, 2);
        ScrollCaptureTarget target2 = createTargetWithView(view2, callback2,
                new Rect(0, 0, 200, 180), new Point(0, 20), View.SCROLL_CAPTURE_HINT_AUTO);

        // Top z-order but smaller, and non-intersecting. (positioned further Y than the first two)
        FakeScrollCaptureCallback callback3 = new FakeScrollCaptureCallback(mDirectExec);
        callback3.setScrollBounds(new Rect(0, 0, 50, 50));
        View view3 = new FakeView(getTargetContext(), 75, 250, 125, 300, 3);
        ScrollCaptureTarget target3 = createTargetWithView(view3, callback3,
                new Rect(0, 0, 50, 50), new Point(75, 250), View.SCROLL_CAPTURE_HINT_AUTO);

        results.addTarget(target1);
        results.addTarget(target2);
        results.addTarget(target3);

        assertTrue(results.isComplete());
        ScrollCaptureTarget result = results.getTopResult();
        assertSame("Expected the second target because of higher z-Index", target2, result);
        assertEquals("result has wrong scroll bounds",
                new Rect(0, 0, 200, 180), result.getScrollBounds());
    }

    @Test
    public void testSingleTarget() {
        ScrollCaptureSearchResults results = new ScrollCaptureSearchResults(mDirectExec);
@@ -152,29 +196,29 @@ public class ScrollCaptureSearchResultsTest {

        // 2 - 10x10 + HINT_INCLUDE
        FakeScrollCaptureCallback callback2 = new FakeScrollCaptureCallback(mDirectExec);
        callback2.setScrollBounds(new Rect(0, 0, 10, 10));
        ViewGroup targetView2 = new FakeView(getTargetContext(), 0, 0, 60, 60, 2);
        callback2.setScrollBounds(new Rect(25, 25, 35, 35)); // 10x10
        ViewGroup targetView2 = new FakeView(getTargetContext(), 0, 60, 60, 120, 2);
        ScrollCaptureTarget target2 = createTargetWithView(targetView2, callback2,
                 new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_INCLUDE);

        // 3 - 20x20 + AUTO
        FakeScrollCaptureCallback callback3 = new FakeScrollCaptureCallback(mDirectExec);
        callback3.setScrollBounds(new Rect(0, 0, 20, 20));
        ViewGroup targetView3 = new FakeView(getTargetContext(), 0, 0, 60, 60, 3);
        ViewGroup targetView3 = new FakeView(getTargetContext(), 0, 120, 60, 180, 3);
        ScrollCaptureTarget target3 = createTargetWithView(targetView3, callback3,
                new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO);

        // 4 - 30x30 + AUTO
        FakeScrollCaptureCallback callback4 = new FakeScrollCaptureCallback(mDirectExec);
        callback4.setScrollBounds(new Rect(0, 0, 10, 10));
        ViewGroup targetView4 = new FakeView(getTargetContext(), 0, 0, 60, 60, 4);
        ViewGroup targetView4 = new FakeView(getTargetContext(), 0, 180, 60, 240, 4);
        ScrollCaptureTarget target4 = createTargetWithView(targetView4, callback4,
                new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO);

        // 5 - 10x10 + child of #4
        FakeScrollCaptureCallback callback5 = new FakeScrollCaptureCallback(mDirectExec);
        callback5.setScrollBounds(new Rect(0, 0, 10, 10));
        ViewGroup targetView5 = new FakeView(getTargetContext(), 0, 0, 60, 60, 5);
        ViewGroup targetView5 = new FakeView(getTargetContext(), 0, 0, 60, 30, 5);
        ScrollCaptureTarget target5 = createTargetWithView(targetView5, callback5,
                new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO);
        targetView4.addView(targetView5);
@@ -182,7 +226,7 @@ public class ScrollCaptureSearchResultsTest {
        // 6 - 20x20 + child of #4
        FakeScrollCaptureCallback callback6 = new FakeScrollCaptureCallback(mDirectExec);
        callback6.setScrollBounds(new Rect(0, 0, 20, 20));
        ViewGroup targetView6 = new FakeView(getTargetContext(), 0, 0, 60, 60, 6);
        ViewGroup targetView6 = new FakeView(getTargetContext(), 0, 30, 30, 60, 6);
        ScrollCaptureTarget target6 = createTargetWithView(targetView6, callback6,
                new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO);
        targetView4.addView(targetView6);
@@ -194,20 +238,10 @@ public class ScrollCaptureSearchResultsTest {
        results.addTarget(target4);
        results.addTarget(target5);
        results.addTarget(target6);
        assertTrue(results.isComplete());
        assertTrue("results.isComplete()", results.isComplete());

        // Verify "top" result
        assertEquals(target2, results.getTopResult());

        // Verify priority ("best" first)
        assertThat(results.getTargets())
                .containsExactly(
                        target2,
                        target6,
                        target5,
                        target4,
                        target3,
                        target1);
        assertEquals("top result", target2, results.getTopResult());
    }

    /**
@@ -291,27 +325,22 @@ public class ScrollCaptureSearchResultsTest {
                new Rect(1, 2, 3, 4), result.getScrollBounds());
    }

    private void setupTargetView(View view, Rect localVisibleRect, int scrollCaptureHint) {
        view.setScrollCaptureHint(scrollCaptureHint);
        view.onVisibilityAggregated(true);
        // Treat any offset as padding, outset localVisibleRect on all sides and use this as
        // child bounds
        Rect bounds = new Rect(localVisibleRect);
        bounds.inset(-bounds.left, -bounds.top, bounds.left, bounds.top);
        view.layout(bounds.left, bounds.top, bounds.right, bounds.bottom);
        view.onVisibilityAggregated(true);
    }

    private ScrollCaptureTarget createTarget(ScrollCaptureCallback callback, Rect localVisibleRect,
            Point positionInWindow, int scrollCaptureHint) {
        View mockView = new View(getTargetContext());
        Rect bounds = new Rect(localVisibleRect);
        // Use localVisibleRect as position, treat left/top offset as padding
        bounds.left = 0;
        bounds.top = 0;
        View mockView = new FakeView(getTargetContext(), bounds.left, bounds.top, bounds.right,
                bounds.bottom, View.NO_ID);
        return createTargetWithView(mockView, callback, localVisibleRect, positionInWindow,
                scrollCaptureHint);
    }

    private ScrollCaptureTarget createTargetWithView(View view, ScrollCaptureCallback callback,
            Rect localVisibleRect, Point positionInWindow, int scrollCaptureHint) {
        setupTargetView(view, localVisibleRect, scrollCaptureHint);
        view.setScrollCaptureHint(scrollCaptureHint);
        view.onVisibilityAggregated(true);
        return new ScrollCaptureTarget(view, localVisibleRect, positionInWindow, callback);
    }

@@ -326,6 +355,32 @@ public class ScrollCaptureSearchResultsTest {
        @Override
        protected void onLayout(boolean changed, int l, int t, int r, int b) {
        }

        /** Ignores window attachment state. The standard impl always returns [0,0] if the view is
         *  not attached. This override allows testing without dealing with AttachInfo.
         */
        @Override
        public void getLocationInWindow(int[] outLocation) {
            outLocation[0] = mLeft;
            outLocation[1] = mTop;
            ViewParent viewParent = getParent();
            while (viewParent instanceof View) {
                final View view = (View) viewParent;

                outLocation[0] -= view.mScrollX;
                outLocation[1] -= view.mScrollY;

                // Explicitly do not handle matrix/transforms, not needed for testing
                if (!view.hasIdentityMatrix()) {
                    throw new IllegalStateException("This mock does not handle transforms!");
                }

                outLocation[0] += view.mLeft;
                outLocation[1] += view.mTop;

                viewParent = view.mParent;
            }
        }
    }

    static class FakeScrollCaptureCallback implements ScrollCaptureCallback {
+60 −1
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

package android.view;

import static android.view.flags.Flags.FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX;

import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;

import static org.junit.Assert.assertEquals;
@@ -30,16 +32,20 @@ import android.content.Context;
import android.graphics.Point;
import android.graphics.Rect;
import android.os.CancellationSignal;
import android.platform.test.annotations.EnableFlags;
import android.platform.test.annotations.Presubmit;
import android.platform.test.flag.junit.SetFlagsRule;

import androidx.annotation.NonNull;
import androidx.test.filters.MediumTest;
import androidx.test.filters.SmallTest;

import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;

import java.util.List;
import java.util.concurrent.Executor;
import java.util.function.Consumer;

@@ -51,6 +57,9 @@ import java.util.function.Consumer;
@RunWith(MockitoJUnitRunner.class)
public class ViewGroupScrollCaptureTest {

    @Rule
    public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();

    private static final Executor DIRECT_EXECUTOR = Runnable::run;

    /** Make sure the hint flags are saved and loaded correctly. */
@@ -239,6 +248,56 @@ public class ViewGroupScrollCaptureTest {
        assertNull(results.getTopResult());
    }

    @EnableFlags(FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX)
    @MediumTest
    @Test
    public void testDispatchScrollCaptureSearch_traversesInDrawingOrder() throws Exception {
        final Context context = getInstrumentation().getContext();
        // Uses childDrawingOrder to reverse drawing order of children.
        final MockViewGroup viewGroup = new MockViewGroup(context, 0, 0, 200, 200);

        // w=200, h=180, z=10, drawn on top
        final MockView view1 = new MockView(context, 0, 20, 200, 200);
        TestScrollCaptureCallback callback1 = new TestScrollCaptureCallback();
        view1.setScrollCaptureCallback(callback1);
        view1.setZ(10f);

        // w=200, h=200, z=0, drawn first, under view1
        final MockView view2 = new MockView(context, 0, 0, 200, 200);
        TestScrollCaptureCallback callback2 = new TestScrollCaptureCallback();
        view2.setScrollCaptureCallback(callback2);

        viewGroup.addView(view1); // test order is dependent on draw order by adding z=10 first
        viewGroup.addView(view2);

        Rect localVisibleRect = new Rect(0, 0, 200, 200);
        Point windowOffset = new Point(0, 0);

        // Where targets are added
        final ScrollCaptureSearchResults results = new ScrollCaptureSearchResults(DIRECT_EXECUTOR);

        viewGroup.dispatchScrollCaptureSearch(localVisibleRect, windowOffset, results::addTarget);
        callback1.completeSearchRequest(new Rect(0, 0, 200, 180));
        callback2.completeSearchRequest(new Rect(0, 0, 200, 200));
        assertTrue(results.isComplete());

        List<ScrollCaptureTarget> targets = results.getTargets();
        List<View> targetViews =
                targets.stream().map(ScrollCaptureTarget::getContainingView).toList();
        assertEquals(List.of(view2,  view1), targetViews);
    }

    static final class ReverseDrawingViewGroup extends MockViewGroup {
        ReverseDrawingViewGroup(Context context, int left, int top, int right, int bottom) {
            super(context, left, top, right, bottom, View.SCROLL_CAPTURE_HINT_AUTO);
        }

        @Override
        protected int getChildDrawingOrder(int childCount, int drawingPosition) {
            return childCount == 0 ? 0 : childCount - (drawingPosition + 1);
        }
    }

    /**
     * Test scroll capture search dispatch to child views.
     * <p>
@@ -511,7 +570,7 @@ public class ViewGroupScrollCaptureTest {
        }
    };

    public static final class MockViewGroup extends ViewGroup {
    public static class MockViewGroup extends ViewGroup {
        private ScrollCaptureCallback mInternalCallback;
        private Rect mOnScrollCaptureSearchLastLocalVisibleRect;
        private Point mOnScrollCaptureSearchLastWindowOffset;