Commit e318bb92 authored by Matt Jones's avatar Matt Jones Committed by Commit Bot

Split out a public and private interface for BottomSheetController

This patch splits the BottomSheetController into 3 pieces:
  - BottomSheetController: This interface is intended to be the public
    interface for anyone trying to use the bottom sheet and should
    freely be passed around in features.
  - BottomSheetControllerInternal: This interface specifies methods
    that are necessary for "glue" code to effectively manage the
    sheet. When the sheet has its own component, the only code allowed
    to depend on it will be glue.
  - BottomSheetControllerImpl: This is the implementation of the above
    interfaces and will eventually be private (or just internal to the
    bottom sheet widget).

This patch also replaces most of the test-exclusive methods using
various alternatives in tests outside of the bottom sheet's package.

Bug: 1002277
Change-Id: Ic84ae52fab3554ef38d84ee32734340c224d8872
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225845
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775278}
parent 826af0ee
......@@ -1782,8 +1782,11 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/widget/ScrimView.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetController.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetControllerImpl.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetControllerInternal.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetObserver.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetSwipeDetector.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetTestSupport.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/EmptyBottomSheetObserver.java",
"java/src/org/chromium/chrome/browser/widget/bottomsheet/TouchRestrictingFrameLayout.java",
]
......@@ -180,13 +180,11 @@ public class AssistantOnboardingCoordinatorTest {
coordinator.disableAnimationForTesting();
showOnboardingAndWait(coordinator, mCallback);
TextView termsView = mBottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.onboarding_subtitle);
TextView termsView = mActivity.findViewById(R.id.onboarding_subtitle);
assertEquals(
mActivity.getResources().getText(R.string.autofill_assistant_init_message_short),
termsView.getText());
TextView titleView = mBottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.onboarding_try_assistant);
TextView titleView = mActivity.findViewById(R.id.onboarding_try_assistant);
assertEquals(
mActivity.getResources().getText(R.string.autofill_assistant_init_message_rent_car),
titleView.getText());
......@@ -205,13 +203,11 @@ public class AssistantOnboardingCoordinatorTest {
coordinator.disableAnimationForTesting();
showOnboardingAndWait(coordinator, mCallback);
TextView termsView = mBottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.onboarding_subtitle);
TextView termsView = mActivity.findViewById(R.id.onboarding_subtitle);
assertEquals(
mActivity.getResources().getText(R.string.autofill_assistant_init_message_short),
termsView.getText());
TextView titleView = mBottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.onboarding_try_assistant);
TextView titleView = mActivity.findViewById(R.id.onboarding_try_assistant);
assertEquals(mActivity.getResources().getText(
R.string.autofill_assistant_init_message_buy_movie_tickets),
titleView.getText());
......@@ -229,13 +225,11 @@ public class AssistantOnboardingCoordinatorTest {
coordinator.disableAnimationForTesting();
showOnboardingAndWait(coordinator, mCallback);
TextView termsView = mBottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.onboarding_subtitle);
TextView termsView = mActivity.findViewById(R.id.onboarding_subtitle);
assertEquals(View.VISIBLE, termsView.getVisibility());
assertEquals(mActivity.getResources().getText(R.string.autofill_assistant_init_message),
termsView.getText());
TextView titleView = mBottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.onboarding_try_assistant);
TextView titleView = mActivity.findViewById(R.id.onboarding_try_assistant);
assertEquals(mActivity.getResources().getText(R.string.autofill_assistant_init_title),
titleView.getText());
}
......
......@@ -132,9 +132,10 @@ public class AutofillAssistantUiTest {
/* bottomSheetDelegate= */ null));
// Bottom sheet is shown in the BottomSheet when creating the AssistantCoordinator.
ViewGroup bottomSheetContent =
bottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.autofill_assistant);
View contentView = AutofillAssistantUiTestUtil.getBottomSheetController(getActivity())
.getCurrentSheetContent()
.getContentView();
ViewGroup bottomSheetContent = contentView.findViewById(R.id.autofill_assistant);
Assert.assertNotNull(bottomSheetContent);
// Disable bottom sheet content animations. This is a workaround for http://crbug/943483.
......@@ -258,9 +259,10 @@ public class AutofillAssistantUiTest {
/* bottomSheetDelegate= */ null));
// Bottom sheet is shown in the BottomSheet when creating the AssistantCoordinator.
ViewGroup bottomSheetContent =
bottomSheetController.getBottomSheetViewForTesting().findViewById(
R.id.autofill_assistant);
View contentView = AutofillAssistantUiTestUtil.getBottomSheetController(getActivity())
.getCurrentSheetContent()
.getContentView();
ViewGroup bottomSheetContent = contentView.findViewById(R.id.autofill_assistant);
Assert.assertNotNull(bottomSheetContent);
// Disable bottom sheet content animations. This is a workaround for http://crbug/943483.
......
......@@ -1825,7 +1825,8 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
return true;
}
if (getBottomSheetController().handleBackPress()) return true;
// TODO(1091411): Find a better mechanism for back-press handling for features.
if (mRootUiCoordinator.getBottomSheetController().handleBackPress()) return true;
if (mTabModalHandler.handleBackPress()) return true;
......
......@@ -23,7 +23,6 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.thinwebview.ThinWebView;
import org.chromium.chrome.browser.thinwebview.ThinWebViewConstraints;
import org.chromium.chrome.browser.thinwebview.ThinWebViewFactory;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent;
import org.chromium.components.browser_ui.widget.FadingShadow;
import org.chromium.components.browser_ui.widget.FadingShadowView;
......@@ -39,6 +38,12 @@ import org.chromium.url.GURL;
* Represents ephemeral tab content and the toolbar, which can be included inside the bottom sheet.
*/
public class EphemeralTabSheetContent implements BottomSheetContent {
/**
* The base duration of the settling animation of the sheet. 218 ms is a spec for material
* design (this is the minimum time a user is guaranteed to pay attention to something).
*/
private static final int BASE_ANIMATION_DURATION_MS = 218;
private static final float PEEK_TOOLBAR_HEIGHT_MULTIPLE = 2.f;
private final Context mContext;
......@@ -159,7 +164,7 @@ public class EphemeralTabSheetContent implements BottomSheetContent {
TransitionDrawable transitionDrawable = ApiCompatibilityUtils.createTransitionDrawable(
new Drawable[] {mCurrentFavicon, favicon});
transitionDrawable.setCrossFadeEnabled(true);
transitionDrawable.startTransition(BottomSheetController.BASE_ANIMATION_DURATION_MS);
transitionDrawable.startTransition(BASE_ANIMATION_DURATION_MS);
presentedDrawable = transitionDrawable;
}
......
......@@ -17,6 +17,7 @@ import org.chromium.chrome.browser.ui.TabObscuringHandler;
import org.chromium.chrome.browser.widget.ScrimView;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController.SheetState;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetControllerImpl;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent;
import org.chromium.content_public.browser.NavigationController;
......@@ -137,9 +138,12 @@ import org.chromium.ui.util.TokenHolder;
ChromeActivity activity = ChromeActivity.fromWebContents(mWebContentsRef);
assert activity != null;
ScrimView.ScrimParams params = activity.getBottomSheetController().createScrimParams(
new ScrimView.EmptyScrimObserver());
ScrimView scrim = activity.getScrim();
// TODO(1002277): Use the proper scrim API when available.
BottomSheetControllerImpl controller =
(BottomSheetControllerImpl) activity.getBottomSheetController();
ScrimView.ScrimParams params =
controller.createScrimParams(new ScrimView.EmptyScrimObserver());
ScrimView scrim = ChromeActivity.fromWebContents(mWebContentsRef).getScrim();
scrim.showScrim(params);
scrim.setViewAlpha(0);
......
......@@ -65,6 +65,8 @@ import org.chromium.chrome.browser.vr.VrModuleProvider;
import org.chromium.chrome.browser.widget.ScrimView;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController.SheetState;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetControllerImpl;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetControllerInternal;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
import org.chromium.chrome.browser.widget.bottomsheet.EmptyBottomSheetObserver;
import org.chromium.components.browser_ui.widget.MenuOrKeyboardActionController;
......@@ -124,7 +126,7 @@ public class RootUiCoordinator
private VrModeObserver mVrModeObserver;
private BottomSheetManager mBottomSheetManager;
private BottomSheetController mBottomSheetController;
private BottomSheetControllerImpl mBottomSheetController;
private SnackbarManager mBottomSheetSnackbarManager;
private ScrimView mScrimView;
......@@ -629,12 +631,10 @@ public class RootUiCoordinator
Supplier<OverlayPanelManager> panelManagerSupplier = ()
-> mActivity.getCompositorViewHolder().getLayoutManager().getOverlayPanelManager();
mBottomSheetController =
new BottomSheetController(mActivity.getLifecycleDispatcher(), mActivityTabProvider,
() -> mScrimCoordinator, sheetViewSupplier, panelManagerSupplier,
mActivity.getFullscreenManager(), mActivity.getWindow(),
mActivity.getWindowAndroid().getKeyboardDelegate(),
mOmniboxFocusStateSupplier);
mBottomSheetController = new BottomSheetControllerImpl(mActivityTabProvider,
() -> mScrimCoordinator, sheetViewSupplier, panelManagerSupplier,
mActivity.getFullscreenManager(), mActivity.getWindow(),
mActivity.getWindowAndroid().getKeyboardDelegate(), mOmniboxFocusStateSupplier);
mBottomSheetManager = new BottomSheetManager(mBottomSheetController, mActivityTabProvider,
mActivity::getModalDialogManager, this::getBottomSheetSnackbarManager,
......@@ -650,7 +650,7 @@ public class RootUiCoordinator
}
/** @return The {@link BottomSheetController} for this activity. */
public BottomSheetController getBottomSheetController() {
public BottomSheetControllerInternal getBottomSheetController() {
return mBottomSheetController;
}
......
......@@ -45,6 +45,12 @@ import org.chromium.ui.KeyboardVisibilityDelegate;
*/
class BottomSheet extends FrameLayout
implements BottomSheetSwipeDetector.SwipeableBottomSheet, View.OnLayoutChangeListener {
/**
* The base duration of the settling animation of the sheet. 218 ms is a spec for material
* design (this is the minimum time a user is guaranteed to pay attention to something).
*/
private static final int BASE_ANIMATION_DURATION_MS = 218;
/**
* The fraction of the way to the next state the sheet must be swiped to animate there when
* released. This is the value used when there are 3 active states. A smaller value here means
......@@ -559,7 +565,7 @@ class BottomSheet extends FrameLayout
mTargetState = targetState;
mSettleAnimator =
ValueAnimator.ofFloat(getCurrentOffsetPx(), getSheetHeightForState(targetState));
mSettleAnimator.setDuration(BottomSheetController.BASE_ANIMATION_DURATION_MS);
mSettleAnimator.setDuration(BASE_ANIMATION_DURATION_MS);
mSettleAnimator.setInterpolator(mInterpolator);
// When the animation is canceled or ends, reset the handle to null.
......@@ -663,16 +669,6 @@ class BottomSheet extends FrameLayout
}
}
/**
* This is the same as {@link #setSheetOffsetFromBottom(float, int)} but exclusively for
* testing.
* @param offset The offset to set the sheet to.
*/
@VisibleForTesting
public void setSheetOffsetFromBottomForTesting(float offset) {
setSheetOffsetFromBottom(offset, StateChangeReason.NONE);
}
/**
* @return The ratio of the height of the screen that the hidden state is.
*/
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.widget.bottomsheet;
/**
* An interface for the owning object to manage interaction between the bottom sheet and the rest
* of the system.
*/
public interface BottomSheetControllerInternal extends BottomSheetController {
/**
* Temporarily suppress the bottom sheet while other UI is showing. This will not itself change
* the content displayed by the sheet.
* @param reason The reason the sheet was suppressed.
*/
void suppressSheet(@StateChangeReason int reason);
/**
* Unsuppress the bottom sheet. This may or may not affect the sheet depending on the state of
* the browser (i.e. the tab switcher may be showing).
*/
void unsuppressSheet();
/**
* For all contents that don't have a custom lifecycle, we remove them from show requests or
* hide it if it is currently shown.
*/
void clearRequestsAndHide();
/**
* Handle a back press event. By default this will return the bottom sheet to it's minimum /
* peeking state if it is open. However, the sheet's content has the opportunity to intercept
* this event and block the default behavior {@see BottomSheetContent#handleBackPress()}.
* @return {@code true} if the sheet or content handled the back press.
*/
boolean handleBackPress();
/** Clean up any state maintained by the controller. */
void destroy();
}
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.widget.bottomsheet;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController.SheetState;
/** Utilities to support testing with the {@link BottomSheetController}. */
public class BottomSheetTestSupport {
/** A handle to the actual implementation class of the {@link BottomSheetController}. */
BottomSheetControllerImpl mController;
/**
* @param controller A handle to the public {@link BottomSheetController}.
*/
public BottomSheetTestSupport(BottomSheetController controller) {
mController = (BottomSheetControllerImpl) controller;
}
/** End all animations on the sheet for testing purposes. */
public void endAllAnimations() {
mController.endAnimationsForTesting();
}
/**
* Force the sheet's state for testing.
* @param state The state the sheet should be in.
* @param animate Whether the sheet should animate to the specified state.
*/
public void setSheetState(@SheetState int state, boolean animate) {
mController.setSheetStateForTesting(state, animate);
}
}
......@@ -22,6 +22,7 @@ import org.chromium.chrome.browser.firstrun.DisableFirstRun;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabbed_mode.TabbedRootUiCoordinator;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetTestSupport;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
......@@ -56,6 +57,7 @@ public class PreviewTabTest {
private static final String NEAR_BOTTOM_DOM_ID = "nearBottom";
private EphemeralTabCoordinator mEphemeralTabCoordinator;
private BottomSheetTestSupport mSheetTestSupport;
@Before
public void setUp() {
......@@ -67,13 +69,13 @@ public class PreviewTabTest {
mEphemeralTabCoordinator =
tabbedRootUiCoordinator.getEphemeralTabCoordinatorForTesting();
});
mSheetTestSupport = new BottomSheetTestSupport(mActivityTestRule.getActivity()
.getRootUiCoordinatorForTesting()
.getBottomSheetController());
}
private void endAnimations() {
TestThreadUtils.runOnUiThreadBlocking(
mActivityTestRule.getActivity()
.getRootUiCoordinatorForTesting()
.getBottomSheetController()::endAnimationsForTesting);
TestThreadUtils.runOnUiThreadBlocking(mSheetTestSupport::endAllAnimations);
}
private void closePreviewTab() {
......
......@@ -58,7 +58,7 @@ public class BottomSheetControllerTest {
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
private BottomSheetController mSheetController;
private BottomSheetControllerImpl mSheetController;
private TestBottomSheetContent mLowPriorityContent;
private TestBottomSheetContent mHighPriorityContent;
private TestBottomSheetContent mPeekableContent;
......@@ -80,7 +80,8 @@ public class BottomSheetControllerTest {
.getScrimCoordinatorForTesting();
mScrimCoordinator.disableAnimationForTesting(true);
mSheetController = activity.getBottomSheetController();
mSheetController = (BottomSheetControllerImpl) activity.getRootUiCoordinatorForTesting()
.getBottomSheetController();
mLowPriorityContent = new TestBottomSheetContent(
mActivityTestRule.getActivity(), BottomSheetContent.ContentPriority.LOW, false);
......
......@@ -101,15 +101,16 @@ public class BottomSheetObserverTest {
public ChromeTabbedActivityTestRule mTestRule = new ChromeTabbedActivityTestRule();
private TestSheetObserver mObserver;
private TestBottomSheetContent mSheetContent;
private BottomSheetController mBottomSheetController;
private BottomSheetControllerImpl mBottomSheetController;
private BottomSheet mSheetView;
@Before
public void setUp() throws Exception {
BottomSheet.setSmallScreenForTesting(false);
mTestRule.startMainActivityOnBlankPage();
mBottomSheetController =
mTestRule.getActivity().getRootUiCoordinatorForTesting().getBottomSheetController();
mBottomSheetController = (BottomSheetControllerImpl) mTestRule.getActivity()
.getRootUiCoordinatorForTesting()
.getBottomSheetController();
ThreadUtils.runOnUiThreadBlocking(() -> {
mSheetContent = new TestBottomSheetContent(
mTestRule.getActivity(), BottomSheetContent.ContentPriority.HIGH, false);
......
......@@ -48,14 +48,15 @@ public class BottomSheetTest {
public ChromeTabbedActivityTestRule mTestRule = new ChromeTabbedActivityTestRule();
private TestBottomSheetContent mLowPriorityContent;
private TestBottomSheetContent mHighPriorityContent;
private BottomSheetController mSheetController;
private BottomSheetControllerImpl mSheetController;
@Before
public void setUp() throws Exception {
BottomSheet.setSmallScreenForTesting(false);
mTestRule.startMainActivityOnBlankPage();
mSheetController =
mTestRule.getActivity().getRootUiCoordinatorForTesting().getBottomSheetController();
mSheetController = (BottomSheetControllerImpl) mTestRule.getActivity()
.getRootUiCoordinatorForTesting()
.getBottomSheetController();
runOnUiThreadBlocking(() -> {
mLowPriorityContent = new TestBottomSheetContent(
mTestRule.getActivity(), BottomSheetContent.ContentPriority.LOW, false);
......
......@@ -50,6 +50,7 @@ import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderPro
import org.chromium.chrome.browser.touch_to_fill.data.Credential;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController.SheetState;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetTestSupport;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
......@@ -306,12 +307,12 @@ public class TouchToFillViewTest {
mModel.set(VISIBLE, true);
});
pollUiThread(() -> getBottomSheetState() == BottomSheetController.SheetState.HALF);
BottomSheetTestSupport sheetSupport = new BottomSheetTestSupport(
getActivity().getRootUiCoordinatorForTesting().getBottomSheetController());
TestThreadUtils.runOnUiThreadBlocking(() -> {
getActivity().getBottomSheetController().setSheetStateForTesting(
SheetState.FULL, false);
});
pollUiThread(() -> getBottomSheetState() == SheetState.FULL);
// Swipe the sheet up to it's full state.
TestThreadUtils.runOnUiThreadBlocking(
() -> { sheetSupport.setSheetState(SheetState.FULL, false); });
TextView manageButton = mTouchToFillView.getContentView().findViewById(
R.id.touch_to_fill_sheet_manage_passwords);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment