Commit 5d1750a5 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere Committed by Commit Bot

Add #hasCustomLifecycle() to BottomSheetContent.

This CL adds the concept of contents with custom lifecycle. This type of
content is not hidden nor cleared from the content queue when the user
navigates away from the current page or switches tab.

Bug: 933070
Change-Id: I092bff8a76bf119b2064a6a66a6e58b431486c5c
Reviewed-on: https://chromium-review.googlesource.com/c/1470228Reviewed-by: default avatarTheresa <twellington@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Jordan Demeulenaere <jdemeulenaere@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635666}
parent 078cd435
...@@ -321,6 +321,14 @@ public class BottomSheet ...@@ -321,6 +321,14 @@ public class BottomSheet
return false; return false;
} }
/**
* @return Whether this content owns its lifecycle. If false, the content will be hidden
* when the user navigates away from the page or switches tab.
*/
default boolean hasCustomLifecycle() {
return false;
}
/** /**
* @return The resource id of the content description for the bottom sheet. This is * @return The resource id of the content description for the bottom sheet. This is
* generally the name of the feature/content that is showing. 'Swipe down to close.' * generally the name of the feature/content that is showing. 'Swipe down to close.'
......
...@@ -26,6 +26,7 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetCon ...@@ -26,6 +26,7 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetCon
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.StateChangeReason; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.StateChangeReason;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator;
import java.util.PriorityQueue; import java.util.PriorityQueue;
import java.util.Set; import java.util.Set;
...@@ -140,7 +141,7 @@ public class BottomSheetController { ...@@ -140,7 +141,7 @@ public class BottomSheetController {
return; return;
} }
// If refocusing the same tab, simply unsupress the sheet. // If refocusing the same tab, simply unsuppress the sheet.
if (mLastActivityTab == tab) { if (mLastActivityTab == tab) {
unsuppressSheet(); unsuppressSheet();
return; return;
...@@ -375,17 +376,34 @@ public class BottomSheetController { ...@@ -375,17 +376,34 @@ public class BottomSheetController {
} }
/** /**
* Clear all the content show requests and hide the current content. * For all contents that don't have a custom lifecycle, we remove them from show requests or
* hide it if it is currently shown.
*/ */
private void clearRequestsAndHide() { private void clearRequestsAndHide() {
mContentQueue.clear(); clearRequests(mContentQueue.iterator());
mFullShowRequestedSet.clear(); clearRequests(mFullShowRequestedSet.iterator());
// TODO(mdjones): Replace usages of bottom sheet with a model in line with MVC.
// TODO(mdjones): It would probably be useful to expose an observer method that notifies BottomSheetContent currentContent = mBottomSheet.getCurrentSheetContent();
// objects when all content requests are cleared. if (currentContent != null && !currentContent.hasCustomLifecycle()) {
hideContent(mBottomSheet.getCurrentSheetContent(), true); if (mContentQueue.isEmpty() && mFullShowRequestedSet.isEmpty()) {
mWasShownForCurrentTab = false; mWasShownForCurrentTab = false;
mIsSuppressed = false; mIsSuppressed = false;
}
hideContent(currentContent, /* animate= */ true);
}
}
/**
* Remove all contents from {@code iterator} that don't have a custom lifecycle.
* @param iterator The iterator whose items must be removed.
*/
private void clearRequests(Iterator<BottomSheetContent> iterator) {
while (iterator.hasNext()) {
if (!iterator.next().hasCustomLifecycle()) {
iterator.remove();
}
}
} }
/** /**
......
...@@ -123,7 +123,7 @@ public class ScrimTest { ...@@ -123,7 +123,7 @@ public class ScrimTest {
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
mBottomSheet.showContent(new TestBottomSheetContent( mBottomSheet.showContent(new TestBottomSheetContent(
mActivityTestRule.getActivity(), BottomSheet.ContentPriority.HIGH)); mActivityTestRule.getActivity(), BottomSheet.ContentPriority.HIGH, false));
mBottomSheet.setSheetState(BottomSheet.SheetState.HALF, false); mBottomSheet.setSheetState(BottomSheet.SheetState.HALF, false);
}); });
......
...@@ -24,6 +24,7 @@ import org.chromium.base.test.util.FlakyTest; ...@@ -24,6 +24,7 @@ import org.chromium.base.test.util.FlakyTest;
import org.chromium.base.test.util.Restriction; import org.chromium.base.test.util.Restriction;
import org.chromium.chrome.browser.ChromeSwitches; import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver; import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabLaunchType; import org.chromium.chrome.browser.tabmodel.TabLaunchType;
...@@ -77,9 +78,9 @@ public class BottomSheetControllerTest { ...@@ -77,9 +78,9 @@ public class BottomSheetControllerTest {
true); true);
mLowPriorityContent = new TestBottomSheetContent( mLowPriorityContent = new TestBottomSheetContent(
mActivityTestRule.getActivity(), ContentPriority.LOW); mActivityTestRule.getActivity(), ContentPriority.LOW, false);
mHighPriorityContent = new TestBottomSheetContent( mHighPriorityContent = new TestBottomSheetContent(
mActivityTestRule.getActivity(), ContentPriority.HIGH); mActivityTestRule.getActivity(), ContentPriority.HIGH, false);
}); });
} }
...@@ -225,6 +226,34 @@ public class BottomSheetControllerTest { ...@@ -225,6 +226,34 @@ public class BottomSheetControllerTest {
mBottomSheet.getCurrentSheetContent()); mBottomSheet.getCurrentSheetContent());
} }
@Test
@MediumTest
public void testCustomLifecycleContent() throws TimeoutException, InterruptedException {
requestContentInSheet(mHighPriorityContent, true);
requestContentInSheet(mLowPriorityContent, false);
TestBottomSheetContent customLifecycleContent = new TestBottomSheetContent(
mActivityTestRule.getActivity(), ContentPriority.LOW, true);
requestContentInSheet(customLifecycleContent, false);
assertEquals(mHighPriorityContent, mBottomSheet.getCurrentSheetContent());
// Change URL and wait for PageLoadStarted event.
CallbackHelper pageLoadStartedHelper = new CallbackHelper();
Tab tab = mActivityTestRule.getActivity().getActivityTab();
tab.addObserver(new EmptyTabObserver() {
@Override
public void onPageLoadStarted(Tab tab, String url) {
pageLoadStartedHelper.notifyCalled();
}
});
int currentCallCount = pageLoadStartedHelper.getCallCount();
ChromeTabUtils.loadUrlOnUiThread(tab, "about:blank");
pageLoadStartedHelper.waitForCallback(currentCallCount, 1);
ThreadUtils.runOnUiThreadBlocking(mBottomSheet::endAnimations);
assertEquals(customLifecycleContent, mBottomSheet.getCurrentSheetContent());
}
/** /**
* Request content be shown in the bottom sheet and end animations. * Request content be shown in the bottom sheet and end animations.
* @param content The content to show. * @param content The content to show.
......
...@@ -40,7 +40,7 @@ public class BottomSheetObserverTest { ...@@ -40,7 +40,7 @@ public class BottomSheetObserverTest {
mBottomSheetTestRule.startMainActivityOnBottomSheet(BottomSheet.SheetState.PEEK); mBottomSheetTestRule.startMainActivityOnBottomSheet(BottomSheet.SheetState.PEEK);
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
mBottomSheetTestRule.getBottomSheet().showContent(new TestBottomSheetContent( mBottomSheetTestRule.getBottomSheet().showContent(new TestBottomSheetContent(
mBottomSheetTestRule.getActivity(), BottomSheet.ContentPriority.HIGH)); mBottomSheetTestRule.getActivity(), BottomSheet.ContentPriority.HIGH, false));
}); });
mObserver = mBottomSheetTestRule.getObserver(); mObserver = mBottomSheetTestRule.getObserver();
} }
...@@ -223,7 +223,7 @@ public class BottomSheetObserverTest { ...@@ -223,7 +223,7 @@ public class BottomSheetObserverTest {
int callCount = callbackHelper.getCallCount(); int callCount = callbackHelper.getCallCount();
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
bottomSheet.showContent(new TestBottomSheetContent( bottomSheet.showContent(new TestBottomSheetContent(
mBottomSheetTestRule.getActivity(), BottomSheet.ContentPriority.HIGH) { mBottomSheetTestRule.getActivity(), BottomSheet.ContentPriority.HIGH, false) {
private final ViewGroup mContentView; private final ViewGroup mContentView;
{ {
......
...@@ -27,12 +27,18 @@ public class TestBottomSheetContent implements BottomSheetContent { ...@@ -27,12 +27,18 @@ public class TestBottomSheetContent implements BottomSheetContent {
/** This content's priority. */ /** This content's priority. */
private @ContentPriority int mPriority; private @ContentPriority int mPriority;
/** Whether this content is browser specific. */
private boolean mHasCustomLifecycle;
/** /**
* @param context A context to inflate views with. * @param context A context to inflate views with.
* @param priority The content's priority. * @param priority The content's priority.
* @param hasCustomLifecycle Whether the content is browser specific.
*/ */
public TestBottomSheetContent(Context context, @ContentPriority int priority) { public TestBottomSheetContent(
Context context, @ContentPriority int priority, boolean hasCustomLifecycle) {
mPriority = priority; mPriority = priority;
mHasCustomLifecycle = hasCustomLifecycle;
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
mToolbarView = new View(context); mToolbarView = new View(context);
ViewGroup.LayoutParams params = ViewGroup.LayoutParams params =
...@@ -82,6 +88,11 @@ public class TestBottomSheetContent implements BottomSheetContent { ...@@ -82,6 +88,11 @@ public class TestBottomSheetContent implements BottomSheetContent {
return true; return true;
} }
@Override
public boolean hasCustomLifecycle() {
return mHasCustomLifecycle;
}
@Override @Override
public int getSheetContentDescriptionStringId() { public int getSheetContentDescriptionStringId() {
return R.string.contextual_suggestions_button_description; return R.string.contextual_suggestions_button_description;
......
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