Commit c482ee98 authored by Becky Zhou's avatar Becky Zhou Committed by Commit Bot

[EoC] Adjust peek behavior part 1

Use server-controlled 1) maximum number of peeks and 2) minimum delay
in seconds after page loaded to adjust whether to auto peek the bottom
sheet on reverse scroll.

Bug: 833865
Change-Id: I021f94236d8f59a8fac9057b80130e4326517c4e
Reviewed-on: https://chromium-review.googlesource.com/1040345Reviewed-by: default avatarTheresa <twellington@chromium.org>
Commit-Queue: Becky Zhou <huayinz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556152}
parent bab79f05
......@@ -5,6 +5,8 @@
package org.chromium.chrome.browser.contextual_suggestions;
import android.content.Context;
import android.os.Handler;
import android.os.SystemClock;
import android.support.annotation.Nullable;
import android.text.TextUtils;
import android.view.View;
......@@ -46,6 +48,7 @@ class ContextualSuggestionsMediator
private final ChromeFullscreenManager mFullscreenManager;
private final View mIphParentView;
private final EnabledStateMonitor mEnabledStateMonitor;
private final Handler mHandler = new Handler();
private @Nullable ContextualSuggestionsSource mSuggestionsSource;
private @Nullable FetchHelper mFetchHelper;
......@@ -56,6 +59,10 @@ class ContextualSuggestionsMediator
private boolean mDidSuggestionsShowForTab;
private boolean mHasRecordedPeekEventForTab;
private boolean mHasPeekDelayPassed;
private boolean mUpdateRemainingCountOnNextPeek;
private float mRemainingPeekCount;
/** Whether the content sheet is observed to be opened for the first time. */
private boolean mHasSheetBeenOpened;
......@@ -146,6 +153,7 @@ class ContextualSuggestionsMediator
.createContextualSuggestionsSource(mProfile);
mFetchHelper = ContextualSuggestionsDependencyFactory.getInstance().createFetchHelper(
this, mTabModelSelector);
mFetchHelper.initialize();
} else {
clearSuggestions();
......@@ -177,12 +185,32 @@ class ContextualSuggestionsMediator
mCurrentRequestUrl = url;
mSuggestionsSource.fetchSuggestions(url, (suggestionsResult) -> {
if (mSuggestionsSource == null) return;
assert mFetchHelper != null;
// Avoiding double fetches causing suggestions for incorrect context.
if (!TextUtils.equals(url, mCurrentRequestUrl)) return;
List<ContextualSuggestionsCluster> clusters = suggestionsResult.getClusters();
PeekConditions peekConditions = suggestionsResult.getPeekConditions();
mRemainingPeekCount = peekConditions.getMaximumNumberOfPeeks();
long remainingDelay =
mFetchHelper.getFetchTimeBaselineMillis(mTabModelSelector.getCurrentTab())
+ Math.round(peekConditions.getMinimumSecondsOnPage() * 1000)
- SystemClock.uptimeMillis();
Runnable runnable = () -> mHasPeekDelayPassed = true;
if (remainingDelay <= 0) {
// Don't postDelayed if the minimum delay has passed so that the suggestions may
// be shown through the following #showContentInSheet() call.
runnable.run();
} else {
// Once delay expires, the bottom sheet can be peeked the next time the browser
// controls are fully hidden and reshown. Note that this triggering is handled by
// FullscreenListener#onControlsOffsetChanged() in this class.
mHandler.postDelayed(runnable, remainingDelay);
}
if (clusters.size() > 0 && clusters.get(0).getSuggestions().size() > 0) {
preloadContentInSheet(
generateClusterList(clusters), suggestionsResult.getPeekText());
......@@ -236,6 +264,10 @@ class ContextualSuggestionsMediator
mDidSuggestionsShowForTab = false;
mHasRecordedPeekEventForTab = false;
mHasSheetBeenOpened = false;
mHandler.removeCallbacksAndMessages(null);
mHasPeekDelayPassed = false;
mUpdateRemainingCountOnNextPeek = false;
mRemainingPeekCount = 0f;
mModel.setClusterList(new ClusterList(Collections.emptyList()));
mModel.setCloseButtonOnClickListener(null);
mModel.setMenuButtonVisibility(false);
......@@ -278,11 +310,19 @@ class ContextualSuggestionsMediator
}
private void showContentInSheet() {
if (!mHasPeekDelayPassed || !hasRemainingPeek()) return;
mDidSuggestionsShowForTab = true;
mUpdateRemainingCountOnNextPeek = true;
mSheetObserver = new EmptyBottomSheetObserver() {
@Override
public void onSheetFullyPeeked() {
if (mUpdateRemainingCountOnNextPeek) {
mUpdateRemainingCountOnNextPeek = false;
--mRemainingPeekCount;
}
if (mHasRecordedPeekEventForTab) return;
assert !mHasSheetBeenOpened;
......@@ -297,6 +337,17 @@ class ContextualSuggestionsMediator
@Override
public void onSheetOffsetChanged(float heightFraction) {
if (mHelpBubble != null) mHelpBubble.dismiss();
// When sheet is fully hidden, clear suggestions if the sheet is not allowed to peek
// anymore or reset mUpdateCountOnNextPeek so mRemainingPeekCount is updated the
// next time the sheet fully peeks.
if (Float.compare(0f, heightFraction) == 0) {
if (hasRemainingPeek()) {
mUpdateRemainingCountOnNextPeek = true;
} else {
clearSuggestions();
}
}
}
@Override
......@@ -326,6 +377,10 @@ class ContextualSuggestionsMediator
mCoordinator.showContentInSheet();
}
private boolean hasRemainingPeek() {
return Float.compare(mRemainingPeekCount, 1f) >= 0;
}
private void maybeShowHelpBubble() {
Tracker tracker = TrackerFactory.getTrackerForProfile(mProfile);
if (!tracker.shouldTriggerHelpUI(FeatureConstants.CONTEXTUAL_SUGGESTIONS_FEATURE)) {
......@@ -371,7 +426,8 @@ class ContextualSuggestionsMediator
}
@VisibleForTesting
void showContentInSheetForTesting() {
void showContentInSheetForTesting(boolean disablePeekDelay) {
if (disablePeekDelay) mHasPeekDelayPassed = true;
showContentInSheet();
}
......
......@@ -5,6 +5,7 @@
package org.chromium.chrome.browser.contextual_suggestions;
import android.os.SystemClock;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.webkit.URLUtil;
......@@ -113,13 +114,16 @@ class FetchHelper {
// TODO(fgorski): flip this to finch controlled setting.
private final static long MINIMUM_FETCH_DELAY_MILLIS = 2 * 1000; // 2 seconds.
private static boolean sDisableDelayForTesting;
private static long sFetchTimeBaselineMillisForTesting;
private final Delegate mDelegate;
private TabModelSelector mTabModelSelector;
private final TabModelSelector mTabModelSelector;
private final Map<Integer, TabFetchReadinessState> mObservedTabs = new HashMap<>();
private TabModelSelectorTabModelObserver mTabModelObserver;
private TabObserver mTabObserver;
private final Map<Integer, TabFetchReadinessState> mObservedTabs = new HashMap<>();
private boolean mFetchRequestedForCurrentTab = false;
private boolean mFetchRequestedForCurrentTab;
private boolean mIsInitialized;
@Nullable
private Tab mCurrentTab;
......@@ -131,15 +135,16 @@ class FetchHelper {
*/
FetchHelper(Delegate delegate, TabModelSelector tabModelSelector) {
mDelegate = delegate;
init(tabModelSelector);
mTabModelSelector = tabModelSelector;
}
/**
* Initializes the FetchHelper. Intended to encapsulate creating connections to native code,
* so that this can be easily stubbed out during tests.
* Initializes the FetchHelper to listen for notifications.
*/
protected void init(TabModelSelector tabModelSelector) {
mTabModelSelector = tabModelSelector;
protected void initialize() {
assert !mIsInitialized;
mIsInitialized = true;
mTabObserver = new EmptyTabObserver() {
@Override
public void onUpdateUrl(Tab tab, String url) {
......@@ -345,8 +350,23 @@ class FetchHelper {
return mObservedTabs.get(tab.getId());
}
/**
* @param tab The specified {@link Tab}.
* @return The baseline fetch time for the specified tab.
*/
long getFetchTimeBaselineMillis(@NonNull Tab tab) {
return sDisableDelayForTesting
? sFetchTimeBaselineMillisForTesting
: mObservedTabs.get(tab.getId()).getFetchTimeBaselineMillis();
}
@VisibleForTesting
static void setDisableDelayForTesting(boolean disable) {
sDisableDelayForTesting = disable;
}
@VisibleForTesting
static void setFetchTimeBaselineMillisForTesting(long uptimeMillis) {
sFetchTimeBaselineMillisForTesting = uptimeMillis;
}
}
......@@ -29,4 +29,21 @@ public class PeekConditions {
mMinimumSecondsOnPage = minimumSecondsOnPage;
mMaximumNumberOfPeeks = maximumNumberOfPeeks;
}
/**
* @return The percentage of the page that the user scrolls required for an auto peek to occur.
*/
public float getPageScrollPercentage() {
return mPageScrollPercentage;
}
/** @return The minimum time (seconds) the user spends on the page required for auto peek. */
public float getMinimumSecondsOnPage() {
return mMinimumSecondsOnPage;
}
/** @return The maximum number of auto peeks that we can show for this page. */
public float getMaximumNumberOfPeeks() {
return mMaximumNumberOfPeeks;
}
}
\ No newline at end of file
......@@ -7,9 +7,11 @@ package org.chromium.chrome.browser.contextual_suggestions;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import android.os.SystemClock;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.LargeTest;
import android.support.test.filters.MediumTest;
......@@ -59,11 +61,13 @@ import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.chrome.test.util.browser.RecyclerViewTestUtils;
import org.chromium.chrome.test.util.browser.compositor.layouts.DisableChromeAnimations;
import org.chromium.components.feature_engagement.FeatureConstants;
import org.chromium.content.browser.test.util.CriteriaHelper;
import org.chromium.content.browser.test.util.TestWebContentsObserver;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.net.test.EmbeddedTestServer;
import org.chromium.ui.test.util.UiRestriction;
import java.util.Locale;
import java.util.concurrent.TimeoutException;
/**
......@@ -167,7 +171,7 @@ public class ContextualSuggestionsTest {
assertEquals("RecyclerView should be empty.", 0, recyclerView.getChildCount());
ThreadUtils.runOnUiThreadBlocking(() -> {
mMediator.showContentInSheetForTesting();
mMediator.showContentInSheetForTesting(true);
mBottomSheet.endAnimations();
});
......@@ -423,7 +427,7 @@ public class ContextualSuggestionsTest {
mModel2.getClusterList().getItemCount());
ThreadUtils.runOnUiThreadBlocking(() -> {
mMediator2.showContentInSheetForTesting();
mMediator2.showContentInSheetForTesting(true);
mBottomSheet2.endAnimations();
ContextualSuggestionsBottomSheetContent content1 =
......@@ -531,13 +535,78 @@ public class ContextualSuggestionsTest {
mRenderTestRule.render(mBottomSheet, "full_height_scrolled");
}
@Test
@MediumTest
@Feature({"ContextualSuggestions"})
public void testPeekDelay() throws Exception {
// Close the suggestions from setUp().
ThreadUtils.runOnUiThreadBlocking(() -> {
mMediator.clearState();
mBottomSheet.endAnimations();
});
// Request suggestions with fetch time baseline set for testing.
long startTime = SystemClock.uptimeMillis();
FetchHelper.setFetchTimeBaselineMillisForTesting(startTime);
ThreadUtils.runOnUiThreadBlocking(
() -> mMediator.requestSuggestions("http://www.testurl.com"));
assertTrue("Bottom sheet should be hidden before delay.",
mBottomSheet.getSheetState() == BottomSheet.SHEET_STATE_HIDDEN);
// Simulate user scroll by calling showContentInSheet until the sheet is peeked.
CriteriaHelper.pollUiThread(() -> {
mMediator.showContentInSheetForTesting(false);
mBottomSheet.endAnimations();
return mBottomSheet.getSheetState() == BottomSheet.SHEET_STATE_PEEK;
});
// Verify that suggestions is shown after the expected delay.
long duration = SystemClock.uptimeMillis() - startTime;
long expected = FakeContextualSuggestionsSource.TEST_PEEK_DELAY_SECONDS * 1000;
assertTrue(String.format(Locale.US,
"The peek delay should be greater than %d ms, but was %d ms.",
expected, duration),
duration >= expected);
}
@Test
@MediumTest
@Feature({"ContextualSuggestions"})
public void testPeekCount() throws Exception {
forceShowSuggestions();
// Opening the sheet and setting it back to peek state shouldn't affect the peek count.
setSheetOffsetForState(BottomSheet.SHEET_STATE_FULL);
setSheetOffsetForState(BottomSheet.SHEET_STATE_PEEK);
// Hide and peek the bottom sheet for (TEST_PEEK_COUNT - 1) number of times, since
// #forceShowSuggestions() has already peeked the bottom sheet once.
for (int i = 1; i < FakeContextualSuggestionsSource.TEST_PEEK_COUNT; ++i) {
setSheetOffsetForState(BottomSheet.SHEET_STATE_HIDDEN);
// Verify that the suggestions are not cleared.
assertEquals("Model has incorrect number of items.",
(int) FakeContextualSuggestionsSource.TOTAL_ITEM_COUNT,
mModel.getClusterList().getItemCount());
assertNotNull("Bottom sheet contents should not be null.",
mBottomSheet.getCurrentSheetContent());
setSheetOffsetForState(BottomSheet.SHEET_STATE_PEEK);
}
// Hide the sheet and verify that the suggestions are cleared.
setSheetOffsetForState(BottomSheet.SHEET_STATE_HIDDEN);
assertEquals("Model should be empty.", 0, mModel.getClusterList().getItemCount());
assertNull("Bottom sheet contents should be null.", mBottomSheet.getCurrentSheetContent());
}
private void forceShowSuggestions() throws InterruptedException, TimeoutException {
assertEquals("Model has incorrect number of items.",
(int) FakeContextualSuggestionsSource.TOTAL_ITEM_COUNT,
mModel.getClusterList().getItemCount());
ThreadUtils.runOnUiThreadBlocking(() -> {
mMediator.showContentInSheetForTesting();
mMediator.showContentInSheetForTesting(true);
mBottomSheet.endAnimations();
assertEquals("Sheet should be peeked.", BottomSheet.SHEET_STATE_PEEK,
......@@ -567,6 +636,14 @@ public class ContextualSuggestionsTest {
() -> mBottomSheet.setSheetState(BottomSheet.SHEET_STATE_FULL, false));
}
private void setSheetOffsetForState(@BottomSheet.SheetState int state) {
ThreadUtils.runOnUiThreadBlocking(() -> {
mBottomSheet.setSheetOffsetFromBottomForTesting(
mBottomSheet.getSheetHeightForState(state));
mBottomSheet.endAnimations();
});
}
private SnippetArticleViewHolder getFirstSuggestionViewHolder() {
return getFirstSuggestionViewHolder(mBottomSheet);
}
......
......@@ -28,6 +28,8 @@ public class FakeContextualSuggestionsSource extends ContextualSuggestionsSource
final static String TEST_TOOLBAR_TITLE = "More about capybaras";
// There should be 6 items in the cluster list - 5 articles and one cluster title.
final static Integer TOTAL_ITEM_COUNT = 6;
final static int TEST_PEEK_COUNT = 3;
final static int TEST_PEEK_DELAY_SECONDS = 2;
private final Map<String, Bitmap> mSuggestionBitmaps = new HashMap<>();
private final List<Pair<SnippetArticle, Callback<Bitmap>>> mPendingImageRequests =
......@@ -133,6 +135,7 @@ public class FakeContextualSuggestionsSource extends ContextualSuggestionsSource
cluster2.getSuggestions().add(article5);
ContextualSuggestionsResult result = new ContextualSuggestionsResult(TEST_TOOLBAR_TITLE);
result.setPeekConditions(new PeekConditions(0f, TEST_PEEK_DELAY_SECONDS, TEST_PEEK_COUNT));
result.getClusters().add(cluster1);
result.getClusters().add(cluster2);
......
......@@ -351,6 +351,7 @@ public final class FetchHelperTest {
private FetchHelper createFetchHelper() {
FetchHelper helper = new FetchHelper(mDelegate, mTabModelSelector);
helper.initialize();
if (mTabModelSelector.getCurrentTab() != null && !mTab.isIncognito()) {
verify(mTab, times(1)).addObserver(mTabObserverCaptor.capture());
}
......@@ -359,7 +360,7 @@ public final class FetchHelperTest {
}
private void delayFetchExecutionTest(Consumer<TabObserver> consumer) {
FetchHelper helper = new FetchHelper(mDelegate, mTabModelSelector);
FetchHelper helper = createFetchHelper();
verify(mTab, times(1)).addObserver(mTabObserverCaptor.capture());
consumer.accept(getTabObserver());
verify(mDelegate, times(0)).requestSuggestions(eq(STARTING_URL));
......@@ -370,7 +371,7 @@ public final class FetchHelperTest {
}
private void delayFetchExecutionTest_updateUrl_toSame(Consumer<TabObserver> consumer) {
FetchHelper helper = new FetchHelper(mDelegate, mTabModelSelector);
FetchHelper helper = createFetchHelper();
verify(mTab, times(1)).addObserver(mTabObserverCaptor.capture());
consumer.accept(getTabObserver());
......@@ -382,7 +383,7 @@ public final class FetchHelperTest {
}
private void delayFetchExecutionTest_updateUrl_toDifferent(Consumer<TabObserver> consumer) {
FetchHelper helper = new FetchHelper(mDelegate, mTabModelSelector);
FetchHelper helper = createFetchHelper();
verify(mTab, times(1)).addObserver(mTabObserverCaptor.capture());
consumer.accept(getTabObserver());
......@@ -397,7 +398,7 @@ public final class FetchHelperTest {
private void addAndRemoveNonSelectedTab() {
// Starting with null tab so we can add one.
doReturn(null).when(mTabModelSelector).getCurrentTab();
FetchHelper helper = new FetchHelper(mDelegate, mTabModelSelector);
FetchHelper helper = createFetchHelper();
verify(mTabModel, times(1)).addObserver(mTabModelObserverCaptor.capture());
addTab(mTab);
......
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