diff options
3 files changed, 269 insertions, 15 deletions
diff --git a/core/java/android/view/flags/scroll_capture.aconfig b/core/java/android/view/flags/scroll_capture.aconfig index 9080b1669ed5..6dccbad3b6a9 100644 --- a/core/java/android/view/flags/scroll_capture.aconfig +++ b/core/java/android/view/flags/scroll_capture.aconfig @@ -11,3 +11,12 @@ flag { } } +flag { + name: "scroll_capture_relax_scroll_view_criteria" + namespace: "systemui" + description: "Treat all custom ViewGroups which support scrollTo as ScrollView" + bug: "189827634" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/core/java/com/android/internal/view/ScrollCaptureInternal.java b/core/java/com/android/internal/view/ScrollCaptureInternal.java index 72b5488f4bac..0ed0613d02e6 100644 --- a/core/java/com/android/internal/view/ScrollCaptureInternal.java +++ b/core/java/com/android/internal/view/ScrollCaptureInternal.java @@ -16,6 +16,8 @@ package com.android.internal.view; +import static android.view.flags.Flags.scrollCaptureRelaxScrollViewCriteria; + import android.annotation.Nullable; import android.content.Context; import android.content.res.Resources; @@ -49,7 +51,7 @@ public class ScrollCaptureInternal { public static final int TYPE_FIXED = 0; /** - * Slides a single child view using mScrollX/mScrollY. + * Moves the viewport across absolute positioned child views using the scrollY property. */ public static final int TYPE_SCROLLING = 1; @@ -63,7 +65,7 @@ public class ScrollCaptureInternal { /** * Unknown scrollable view with no child views (or not a subclass of ViewGroup). */ - private static final int TYPE_OPAQUE = 3; + public static final int TYPE_OPAQUE = 3; /** * Performs tests on the given View and determines: @@ -73,7 +75,7 @@ public class ScrollCaptureInternal { * This needs to be fast and not alloc memory. It's called on everything in the tree not marked * as excluded during scroll capture search. */ - private static int detectScrollingType(View view) { + public static int detectScrollingType(View view) { // Confirm that it can scroll. if (!(view.canScrollVertically(DOWN) || view.canScrollVertically(UP))) { // Nothing to scroll here, move along. @@ -95,25 +97,25 @@ public class ScrollCaptureInternal { if (DEBUG_VERBOSE) { Log.v(TAG, "hint: is a subclass of ViewGroup"); } - - // ScrollViews accept only a single child. - if (((ViewGroup) view).getChildCount() > 1) { - if (DEBUG_VERBOSE) { - Log.v(TAG, "hint: scrollable with multiple children"); + // Flag: Optionally allow ScrollView-like ViewGroups which have more than one child view. + if (!scrollCaptureRelaxScrollViewCriteria()) { + // ScrollViews accept only a single child. + if (((ViewGroup) view).getChildCount() > 1) { + if (DEBUG_VERBOSE) { + Log.v(TAG, "hint: scrollable with multiple children"); + } + return TYPE_RECYCLING; } - return TYPE_RECYCLING; } // At least one child view is required. - if (((ViewGroup) view).getChildCount() < 1) { - if (DEBUG_VERBOSE) { - Log.v(TAG, "scrollable with no children"); - } + if (((ViewGroup) view).getChildCount() == 0) { + Log.w(TAG, "scrollable but no children!"); return TYPE_OPAQUE; } if (DEBUG_VERBOSE) { Log.v(TAG, "hint: single child view"); } - //Because recycling containers don't use scrollY, a non-zero value means Scroll view. + // Because recycling containers don't use scrollY, a non-zero value means Scroll view. if (view.getScrollY() != 0) { if (DEBUG_VERBOSE) { Log.v(TAG, "hint: scrollY != 0"); @@ -132,7 +134,7 @@ public class ScrollCaptureInternal { Log.v(TAG, "hint: cannot be scrolled up"); } - // canScrollVertically(UP) == false, getScrollY() == 0, getChildCount() == 1. + // canScrollVertically(UP) == false, getScrollY() == 0, getChildCount() >= 1. // For Recycling containers, this should be a no-op (RecyclerView logs a warning) view.scrollTo(view.getScrollX(), 1); diff --git a/core/tests/coretests/src/com/android/internal/view/ScrollCaptureInternalTest.java b/core/tests/coretests/src/com/android/internal/view/ScrollCaptureInternalTest.java new file mode 100644 index 000000000000..5f6d806bc064 --- /dev/null +++ b/core/tests/coretests/src/com/android/internal/view/ScrollCaptureInternalTest.java @@ -0,0 +1,243 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.view; + +import static android.view.flags.Flags.FLAG_SCROLL_CAPTURE_RELAX_SCROLL_VIEW_CRITERIA; + +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; + +import static com.android.internal.view.ScrollCaptureInternal.TYPE_FIXED; +import static com.android.internal.view.ScrollCaptureInternal.TYPE_OPAQUE; +import static com.android.internal.view.ScrollCaptureInternal.TYPE_RECYCLING; +import static com.android.internal.view.ScrollCaptureInternal.TYPE_SCROLLING; +import static com.android.internal.view.ScrollCaptureInternal.detectScrollingType; + +import static org.junit.Assert.assertEquals; + +import android.content.Context; +import android.graphics.Rect; +import android.platform.test.annotations.EnableFlags; +import android.platform.test.annotations.Presubmit; +import android.platform.test.flag.junit.SetFlagsRule; +import android.testing.AndroidTestingRunner; +import android.view.ViewGroup; + +import androidx.test.filters.SmallTest; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** + * Tests scrolling detection. + */ +@Presubmit +@SmallTest +@RunWith(AndroidTestingRunner.class) +public class ScrollCaptureInternalTest { + + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + + /** + * Tests the effect of padding on scroll capture search dispatch. + * <p> + * Verifies computation of child visible bounds with padding. + */ + @Test + public void testDetectScrollingType_scrolling_notScrollable() { + MockScrollable scrollable = new MockScrollable.Builder() + .bounds(0, 0, 200, 200) + .childCount(1) + .canScrollUp(false) + .canScrollDown(false) + .scrollToEnabled(false) + .build(getInstrumentation().getContext()); + + assertEquals(TYPE_FIXED, detectScrollingType(scrollable)); + } + + @Test + public void testDetectScrollingType_scrolling_noChildren() { + MockScrollable scrollable = new MockScrollable.Builder() + .bounds(0, 0, 200, 200) + .childCount(0) + .canScrollUp(false) + .canScrollDown(true) + .scrollToEnabled(true) + .build(getInstrumentation().getContext()); + + assertEquals(TYPE_OPAQUE, detectScrollingType(scrollable)); + } + + @Test + public void testDetectScrollingType_scrolling() { + MockScrollable scrollable = new MockScrollable.Builder() + .bounds(0, 0, 200, 200) + .childCount(1) + .canScrollUp(false) + .canScrollDown(true) + .scrollToEnabled(true) + .build(getInstrumentation().getContext()); + + assertEquals(TYPE_SCROLLING, detectScrollingType(scrollable)); + } + + @Test + public void testDetectScrollingType_scrolling_partiallyScrolled() { + MockScrollable scrollable = new MockScrollable.Builder() + .bounds(0, 0, 200, 200) + .childCount(1) + .canScrollUp(true) + .canScrollDown(true) + .scrollToEnabled(true) + .build(getInstrumentation().getContext()); + scrollable.scrollTo(0, 100); + + assertEquals(TYPE_SCROLLING, detectScrollingType(scrollable)); + } + + @Test + @EnableFlags(FLAG_SCROLL_CAPTURE_RELAX_SCROLL_VIEW_CRITERIA) + public void testDetectScrollingType_scrolling_multipleChildren() { + MockScrollable scrollable = new MockScrollable.Builder() + .bounds(0, 0, 200, 200) + .childCount(10) + .canScrollUp(false) + .canScrollDown(true) + .scrollToEnabled(true) + .build(getInstrumentation().getContext()); + + assertEquals(TYPE_SCROLLING, detectScrollingType(scrollable)); + } + + @Test + public void testDetectScrollingType_recycling() { + MockScrollable scrollable = new MockScrollable.Builder() + .bounds(0, 0, 200, 200) + .childCount(10) + .canScrollUp(false) + .canScrollDown(true) + .scrollToEnabled(false) + .build(getInstrumentation().getContext()); + + assertEquals(TYPE_RECYCLING, detectScrollingType(scrollable)); + } + + @Test + public void testDetectScrollingType_noChildren() { + MockScrollable scrollable = new MockScrollable.Builder() + .bounds(0, 0, 200, 200) + .childCount(0) + .canScrollUp(true) + .canScrollDown(true) + .scrollToEnabled(false) + .build(getInstrumentation().getContext()); + + assertEquals(TYPE_OPAQUE, detectScrollingType(scrollable)); + } + + + /** + * A mock which can exhibit some attributes and behaviors used to detect different types + * of scrolling content. + */ + private static class MockScrollable extends ViewGroup { + private final int mChildCount; + private final boolean mCanScrollUp; + private final boolean mCanScrollDown; + private final boolean mScrollToEnabled; + + MockScrollable(Context context, Rect bounds, int childCount, boolean canScrollUp, + boolean canScrollDown, boolean scrollToEnabled) { + super(context); + setFrame(bounds.left, bounds.top, bounds.right, bounds.bottom); + mCanScrollUp = canScrollUp; + mCanScrollDown = canScrollDown; + mScrollToEnabled = scrollToEnabled; + mChildCount = childCount; + } + + private static class Builder { + private int mChildCount; + private boolean mCanScrollUp; + private boolean mCanScrollDown; + private boolean mScrollToEnabled = true; + + private final Rect mBounds = new Rect(); + + public MockScrollable build(Context context) { + return new MockScrollable(context, + mBounds, mChildCount, mCanScrollUp, mCanScrollDown, + mScrollToEnabled); + } + + public Builder canScrollUp(boolean canScrollUp) { + mCanScrollUp = canScrollUp; + return this; + } + + public Builder canScrollDown(boolean canScrollDown) { + mCanScrollDown = canScrollDown; + return this; + } + + public Builder scrollToEnabled(boolean enabled) { + mScrollToEnabled = enabled; + return this; + } + + public Builder childCount(int childCount) { + mChildCount = childCount; + return this; + } + + public Builder bounds(int left, int top, int right, int bottom) { + mBounds.set(left, top, right, bottom); + return this; + } + } + + @Override + public boolean canScrollVertically(int direction) { + if (direction > 0) { + return mCanScrollDown; + } else if (direction < 0) { + return mCanScrollUp; + } else { + return false; + } + } + + @Override + public int getChildCount() { + return mChildCount; + } + + @Override + public void scrollTo(int x, int y) { + if (mScrollToEnabled) { + super.scrollTo(x, y); + } + } + + @Override + protected void onLayout(boolean changed, int l, int t, int r, int b) { + // We don't layout this view. + } + } +} |