From c14734d86537138ecdb5b05bc76e793e838320e0 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Sun, 11 Apr 2021 11:22:26 -0400 Subject: [PATCH] Account for padding in dispatchScrollCaptureSearch When dispatching to self, ViewGroup is not applying padding to localVisibleRect, only when dispatching to child views. Also when deducting padding from the visible bounds, it must be intersected with bounds, not subtracted. Test: atest ViewGroupScrollCaptureTest Bug: 185153400 Change-Id: Ib92e4c552fbe02fd863123f166878e19b8b1c952 --- core/java/android/view/View.java | 2 +- core/java/android/view/ViewGroup.java | 21 ++--- .../view/ViewGroupScrollCaptureTest.java | 83 ++++++++++++++++++- 3 files changed, 91 insertions(+), 15 deletions(-) diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index dd5c954045101..f8ff001d20dfb 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -1494,7 +1494,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback, public static final int SCROLL_CAPTURE_HINT_AUTO = 0; /** - * Explicitly exclcude this view as a potential scroll capture target. The system will not + * Explicitly exclude this view as a potential scroll capture target. The system will not * consider it. Mutually exclusive with {@link #SCROLL_CAPTURE_HINT_INCLUDE}, which this flag * takes precedence over. * diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java index 5b695f4c425a1..e8610bd3343b7 100644 --- a/core/java/android/view/ViewGroup.java +++ b/core/java/android/view/ViewGroup.java @@ -7479,12 +7479,9 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager @NonNull Rect localVisibleRect, @NonNull Point windowOffset, @NonNull Consumer targets) { - // copy local visible rect for modification and dispatch - final Rect rect = getTempRect(); - rect.set(localVisibleRect); - - if (getClipToPadding()) { - rect.inset(mPaddingLeft, mPaddingTop, mPaddingRight, mPaddingBottom); + if (getClipToPadding() && !localVisibleRect.intersect(mPaddingLeft, mPaddingTop, + (mRight - mLeft) - mPaddingRight, (mBottom - mTop) - mPaddingBottom)) { + return; } // Dispatch to self first. @@ -7495,6 +7492,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager return; } + final Rect tmpRect = getTempRect(); final int childCount = getChildCount(); for (int i = 0; i < childCount; i++) { View child = getChildAt(i); @@ -7507,8 +7505,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager // If the resulting rectangle is not empty, the request is forwarded to the child. // copy local visible rect for modification and dispatch - final Rect childVisibleRect = getTempRect(); - childVisibleRect.set(localVisibleRect); + tmpRect.set(localVisibleRect); // transform to child coords final Point childWindowOffset = getTempPoint(); @@ -7517,20 +7514,18 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager final int dx = child.mLeft - mScrollX; final int dy = child.mTop - mScrollY; - childVisibleRect.offset(-dx, -dy); + tmpRect.offset(-dx, -dy); childWindowOffset.offset(dx, dy); boolean rectIsVisible = true; // Clip to child bounds if (getClipChildren()) { - rectIsVisible = childVisibleRect.intersect(0, 0, child.getWidth(), - child.getHeight()); + rectIsVisible = tmpRect.intersect(0, 0, child.getWidth(), child.getHeight()); } - // Clip to child padding. if (rectIsVisible) { - child.dispatchScrollCaptureSearch(childVisibleRect, childWindowOffset, targets); + child.dispatchScrollCaptureSearch(tmpRect, childWindowOffset, targets); } } } diff --git a/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java b/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java index 2833ea3f9ac07..591dae239e74f 100644 --- a/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java +++ b/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java @@ -33,7 +33,6 @@ import android.os.CancellationSignal; import android.platform.test.annotations.Presubmit; import androidx.annotation.NonNull; -import androidx.test.filters.FlakyTest; import androidx.test.filters.MediumTest; import androidx.test.filters.SmallTest; @@ -343,6 +342,54 @@ public class ViewGroupScrollCaptureTest { target.getContainingView().getScrollCaptureHint()); } + /** + * Tests the effect of padding on scroll capture search dispatch. + *

+ * Verifies computation of child visible bounds with padding. + */ + @MediumTest + @Test + public void testOnScrollCaptureSearch_withPadding() { + final Context context = getInstrumentation().getContext(); + + Rect windowBounds = new Rect(0, 0, 200, 200); + Point windowOffset = new Point(0, 0); + + final MockViewGroup parent = new MockViewGroup(context, 0, 0, 200, 200); + parent.setPadding(25, 50, 25, 50); + parent.setClipToPadding(true); // (default) + + final MockView view1 = new MockView(context, 0, -100, 200, 100); + parent.addView(view1); + + final MockView view2 = new MockView(context, 0, 0, 200, 200); + parent.addView(view2); + + final MockViewGroup view3 = new MockViewGroup(context, 0, 100, 200, 300); + parent.addView(view3); + view3.setPadding(25, 25, 25, 25); + view3.setClipToPadding(true); + + // Where targets are added + final ScrollCaptureSearchResults results = new ScrollCaptureSearchResults(DIRECT_EXECUTOR); + + // Dispatch to the ViewGroup + parent.dispatchScrollCaptureSearch(windowBounds, windowOffset, results::addTarget); + + // Verify padding (with clipToPadding) is subtracted from visibleBounds + parent.assertOnScrollCaptureSearchLastArgs(new Rect(25, 50, 175, 150), new Point(0, 0)); + + view1.assertOnScrollCaptureSearchLastArgs( + new Rect(25, 150, 175, 200), new Point(0, -100)); + + view2.assertOnScrollCaptureSearchLastArgs( + new Rect(25, 50, 175, 150), new Point(0, 0)); + + // Account for padding on view3 as well (top == 25px) + view3.assertOnScrollCaptureSearchLastArgs( + new Rect(25, 25, 175, 50), new Point(0, 100)); + } + public static final class MockView extends View { private ScrollCaptureCallback mInternalCallback; @@ -350,6 +397,8 @@ public class ViewGroupScrollCaptureTest { private Rect mDispatchScrollCaptureSearchLastLocalVisibleRect; private Point mDispatchScrollCaptureSearchLastWindowOffset; private int mCreateScrollCaptureCallbackInternalCount; + private Rect mOnScrollCaptureSearchLastLocalVisibleRect; + private Point mOnScrollCaptureSearchLastWindowOffset; MockView(Context context) { this(context, /* left */ 0, /* top */0, /* right */ 0, /* bottom */0); @@ -394,6 +443,21 @@ public class ViewGroupScrollCaptureTest { } + @Override + public void onScrollCaptureSearch(Rect localVisibleRect, Point windowOffset, + Consumer targets) { + super.onScrollCaptureSearch(localVisibleRect, windowOffset, targets); + mOnScrollCaptureSearchLastLocalVisibleRect = new Rect(localVisibleRect); + mOnScrollCaptureSearchLastWindowOffset = new Point(windowOffset); + } + + void assertOnScrollCaptureSearchLastArgs(Rect localVisibleRect, Point windowOffset) { + assertEquals("arg localVisibleRect was incorrect.", + localVisibleRect, mOnScrollCaptureSearchLastLocalVisibleRect); + assertEquals("arg windowOffset was incorrect.", + windowOffset, mOnScrollCaptureSearchLastWindowOffset); + } + @Override public void dispatchScrollCaptureSearch(Rect localVisibleRect, Point windowOffset, Consumer results) { @@ -437,6 +501,8 @@ public class ViewGroupScrollCaptureTest { public static final class MockViewGroup extends ViewGroup { private ScrollCaptureCallback mInternalCallback; + private Rect mOnScrollCaptureSearchLastLocalVisibleRect; + private Point mOnScrollCaptureSearchLastWindowOffset; MockViewGroup(Context context) { this(context, /* left */ 0, /* top */0, /* right */ 0, /* bottom */0); @@ -464,6 +530,21 @@ public class ViewGroupScrollCaptureTest { return mInternalCallback; } + @Override + public void onScrollCaptureSearch(Rect localVisibleRect, Point windowOffset, + Consumer targets) { + super.onScrollCaptureSearch(localVisibleRect, windowOffset, targets); + mOnScrollCaptureSearchLastLocalVisibleRect = new Rect(localVisibleRect); + mOnScrollCaptureSearchLastWindowOffset = new Point(windowOffset); + } + + void assertOnScrollCaptureSearchLastArgs(Rect localVisibleRect, Point windowOffset) { + assertEquals("arg localVisibleRect was incorrect.", + localVisibleRect, mOnScrollCaptureSearchLastLocalVisibleRect); + assertEquals("arg windowOffset was incorrect.", + windowOffset, mOnScrollCaptureSearchLastWindowOffset); + } + @Override protected void onLayout(boolean changed, int l, int t, int r, int b) { // We don't layout this view.