Commit 1bf7c11e authored by Matt Jones's avatar Matt Jones Committed by Commit Bot

Make sure to call BottomSheetContent#destroy

This patch calls the previously manually called destroy method on
BottomSheetContent. This is triggered when the sheet is hidden (and
not suppressed), regardless of whether the sheet has a custom
lifecycle.

Change-Id: I392fd98e384cfbdc73422959417e20b246da2f3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879688Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709918}
parent 1eaae7db
......@@ -279,7 +279,11 @@ public class BottomSheet
int getVerticalScrollOffset();
/**
* Called to destroy the {@link BottomSheetContent} when it is no longer in use.
* Called to destroy the {@link BottomSheetContent} when it is dismissed. The means the
* sheet is in the {@link SheetState#HIDDEN} state without being suppressed. This method
* does not necessarily need to be used but exists for convenience. Cleanup can be done
* manually via the owning component (likely watching for the sheet hidden event using an
* observer).
*/
void destroy();
......
......@@ -189,6 +189,9 @@ public class BottomSheetController implements Destroyable {
@Override
public void onSheetStateChanged(@SheetState int state) {
if (state != SheetState.HIDDEN || mIsSuppressed) return;
if (mBottomSheet.getCurrentSheetContent() != null) {
mBottomSheet.getCurrentSheetContent().destroy();
}
mIsProcessingHideRequest = false;
showNextContent(true);
}
......
......@@ -210,6 +210,24 @@ public class BottomSheetControllerTest {
mBottomSheet.getCurrentSheetContent());
}
@Test
@MediumTest
@Feature({"BottomSheetController"})
public void testContentDestroyedOnHidden() throws TimeoutException {
requestContentInSheet(mLowPriorityContent, true);
int destroyCallCount = mLowPriorityContent.destroyCallbackHelper.getCallCount();
// Enter the tab switcher and select a different tab.
ThreadUtils.runOnUiThreadBlocking(
() -> { mBottomSheet.setSheetState(BottomSheet.SheetState.HIDDEN, false); });
mLowPriorityContent.destroyCallbackHelper.waitForCallback(destroyCallCount);
assertEquals("The bottom sheet should be hidden.", BottomSheet.SheetState.HIDDEN,
mBottomSheet.getSheetState());
assertEquals("The bottom sheet is showing incorrect content.", null,
mBottomSheet.getCurrentSheetContent());
}
@Test
@MediumTest
@Feature({"BottomSheetController"})
......
......@@ -12,12 +12,16 @@ import android.view.ViewGroup;
import androidx.annotation.Nullable;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.ContentPriority;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
/** A simple sheet content to test with. This only displays two empty white views. */
public class TestBottomSheetContent implements BottomSheetContent {
/** {@link CallbackHelper} to ensure the destroy method is called. */
public final CallbackHelper destroyCallbackHelper = new CallbackHelper();
/** Empty view that represents the toolbar. */
private View mToolbarView;
......@@ -82,7 +86,9 @@ public class TestBottomSheetContent implements BottomSheetContent {
}
@Override
public void destroy() {}
public void destroy() {
destroyCallbackHelper.notifyCalled();
}
@Override
public int getPriority() {
......
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