Trigger loading more feed content when scrolling close to bottom

Bug: none
Change-Id: I538d5a32853ea8a62b54f25f9c00c69d12fd53ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2278344Reviewed-by: Dan H <harringtond@chromium.org>
Commit-Queue: Jian Li <jianli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786111}

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/v2/FeedServiceBridge.java

@@ -39,9 +39,15 @@ public final class FeedServiceBridge {
         FeedServiceBridgeJni.get().startup();
     }
 
+    /** Retrieves the config value for load_more_trigger_lookahead. */
+    public static int getLoadMoreTriggerLookahead() {
+        return FeedServiceBridgeJni.get().getLoadMoreTriggerLookahead();
+    }
+
     @NativeMethods
     interface Natives {
         boolean isEnabled();
         void startup();
+        int getLoadMoreTriggerLookahead();
     }
 }

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/v2/FeedStream.java

@@ -5,9 +5,11 @@
 package org.chromium.chrome.browser.feed.v2;
 
 import android.app.Activity;
+import android.util.TypedValue;
 import android.view.View;
 
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import androidx.recyclerview.widget.LinearLayoutManager;
 import androidx.recyclerview.widget.RecyclerView;
 
@@ -34,16 +36,23 @@ public class FeedStream implements Stream {
     private static final String TAG = "FeedStream";
     private static final String SCROLL_POSITION = "scroll_pos";
     private static final String SCROLL_OFFSET = "scroll_off";
+    // How far the user has to scroll down in DP before attempting to load more content.
+    static final int LOAD_MORE_TRIGGER_SCROLL_DISTANCE_DP = 100;
 
     private final Activity mActivity;
     private final FeedStreamSurface mFeedStreamSurface;
-    private final ObserverList<ScrollListener> mScrollListeners;
+    private final ObserverList<ScrollListener> mScrollListeners =
+            new ObserverList<ScrollListener>();
+    private final int mLoadMoreTriggerLookahead;
 
     private RecyclerView mRecyclerView;
     // setStreamContentVisibility() is always called once after onCreate(). So we can assume the
     // stream content is hidden initially and it can be made visible later when
     // setStreamContentVisibility() is called.
     private boolean mIsStreamContentVisible = false;
+    // For loading more content.
+    private int mAccumulatedDySinceLastLoadMore;
+    private boolean mIsLoadingMoreContent;
 
     public FeedStream(Activity activity, boolean isBackgroundDark, SnackbarManager snackbarManager,
             NativePageNavigationDelegate nativePageNavigationDelegate,
@@ -52,7 +61,7 @@ public class FeedStream implements Stream {
         this.mActivity = activity;
         this.mFeedStreamSurface = new FeedStreamSurface(activity, isBackgroundDark, snackbarManager,
                 nativePageNavigationDelegate, bottomSheetController);
-        this.mScrollListeners = new ObserverList<ScrollListener>();
+        this.mLoadMoreTriggerLookahead = FeedServiceBridge.getLoadMoreTriggerLookahead();
     }
 
     @Override
@@ -205,14 +214,49 @@ public class FeedStream implements Stream {
         mRecyclerView.setClipToPadding(false);
         mRecyclerView.addOnScrollListener(new RecyclerView.OnScrollListener() {
             @Override
-            public void onScrolled(RecyclerView v, int x, int y) {
+            public void onScrolled(RecyclerView v, int dx, int dy) {
+                super.onScrolled(v, dx, dy);
+                checkScrollingForLoadMore(dy);
                 for (ScrollListener listener : mScrollListeners) {
-                    listener.onScrolled(x, y);
+                    listener.onScrolled(dx, dy);
                 }
             }
         });
     }
 
+    @VisibleForTesting
+    void checkScrollingForLoadMore(int dy) {
+        if (!mIsStreamContentVisible) {
+            return;
+        }
+
+        mAccumulatedDySinceLastLoadMore += dy;
+        if (mAccumulatedDySinceLastLoadMore < TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP,
+                    LOAD_MORE_TRIGGER_SCROLL_DISTANCE_DP,
+                    mRecyclerView.getResources().getDisplayMetrics())) {
+            return;
+        }
+
+        LinearLayoutManager layoutManager = (LinearLayoutManager) mRecyclerView.getLayoutManager();
+        if (layoutManager == null) {
+            return;
+        }
+        int totalItemCount = layoutManager.getItemCount();
+        int lastVisibleItem = layoutManager.findLastVisibleItemPosition();
+        if (totalItemCount - lastVisibleItem <= mLoadMoreTriggerLookahead) {
+            mAccumulatedDySinceLastLoadMore = 0;
+            loadMore();
+        }
+    }
+
+    private void loadMore() {
+        if (mIsLoadingMoreContent) {
+            return;
+        }
+        mIsLoadingMoreContent = true;
+        mFeedStreamSurface.loadMoreContent((Boolean success) -> { mIsLoadingMoreContent = false; });
+    }
+
     private void restoreScrollState(String savedInstanceState) {
         try {
             JSONObject jsonSavedState = new JSONObject(savedInstanceState);

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/v2/FeedStreamSurface.java

@@ -13,6 +13,7 @@ import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.recyclerview.widget.RecyclerView;
 
+import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.annotations.CalledByNative;
@@ -309,6 +310,14 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
         return mRootView;
     }
 
+    /**
+     * Loads more content. The callback will be called upon completion.
+     */
+    public void loadMoreContent(Callback<Boolean> callback) {
+        FeedStreamSurfaceJni.get().loadMore(
+                mNativeFeedStreamSurface, FeedStreamSurface.this, callback);
+    }
+
     @VisibleForTesting
     FeedListContentManager getFeedListContentManagerForTesting() {
         return mContentManager;
@@ -461,7 +470,7 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
 
     @Override
     public void loadMore() {
-        FeedStreamSurfaceJni.get().loadMore(mNativeFeedStreamSurface, FeedStreamSurface.this);
+        // TODO(jianli): Remove this from FeedActionsHandler interface.
     }
 
     @Override
@@ -600,7 +609,8 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
                 long nativeFeedStreamSurface, FeedStreamSurface caller, int distanceDp);
         // TODO(jianli): Call this function at the appropriate time.
         void reportStreamScrollStart(long nativeFeedStreamSurface, FeedStreamSurface caller);
-        void loadMore(long nativeFeedStreamSurface, FeedStreamSurface caller);
+        void loadMore(
+                long nativeFeedStreamSurface, FeedStreamSurface caller, Callback<Boolean> callback);
         void processThereAndBackAgain(
                 long nativeFeedStreamSurface, FeedStreamSurface caller, byte[] data);
         int executeEphemeralChange(

chrome/android/feed/feed_java_sources.gni

@@ -651,6 +651,7 @@ if (enable_feed_in_chrome) {
     "junit/src/org/chromium/chrome/browser/feed/v2/FeedListContentManagerTest.java",
     "junit/src/org/chromium/chrome/browser/feed/v2/FeedSliceViewTrackerTest.java",
     "junit/src/org/chromium/chrome/browser/feed/v2/FeedStreamSurfaceTest.java",
+    "junit/src/org/chromium/chrome/browser/feed/v2/FeedStreamTest.java",
     "junit/src/org/chromium/chrome/browser/feed/v2/NativeViewListRendererTest.java",
   ]
 

chrome/android/junit/src/org/chromium/chrome/browser/feed/v2/FeedStreamTest.java

+// 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.feed.v2;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import static org.chromium.chrome.browser.feed.shared.stream.Stream.POSITION_NOT_KNOWN;
+
+import android.app.Activity;
+import android.content.Context;
+import android.util.TypedValue;
+import android.view.View;
+import android.widget.FrameLayout;
+
+import androidx.recyclerview.widget.LinearLayoutManager;
+import androidx.recyclerview.widget.RecyclerView;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.Robolectric;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.Callback;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.JniMocker;
+import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
+import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
+import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/** Unit tests for {@link FeedStream}. */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class FeedStreamTest {
+    private class FakeLinearLayoutManager extends LinearLayoutManager {
+        private final List<View> mChildViews;
+        private int mFirstVisiblePosition = RecyclerView.NO_POSITION;
+        private int mLastVisiblePosition = RecyclerView.NO_POSITION;
+        private int mItemCount = RecyclerView.NO_POSITION;
+
+        public FakeLinearLayoutManager(Context context) {
+            super(context);
+            mChildViews = new ArrayList<>();
+        }
+
+        @Override
+        public int findFirstVisibleItemPosition() {
+            return mFirstVisiblePosition;
+        }
+
+        @Override
+        public int findLastVisibleItemPosition() {
+            return mLastVisiblePosition;
+        }
+
+        @Override
+        public int getItemCount() {
+            return mItemCount;
+        }
+
+        @Override
+        public View findViewByPosition(int i) {
+            if (i < 0 || i >= mChildViews.size()) {
+                return null;
+            }
+            return mChildViews.get(i);
+        }
+
+        private void addChildToPosition(int position, View child) {
+            mChildViews.add(position, child);
+        }
+    }
+
+    private static final int LOAD_MORE_TRIGGER_LOOKAHEAD = 5;
+    private Activity mActivity;
+    private RecyclerView mRecyclerView;
+    private FakeLinearLayoutManager mLayoutManager;
+    private FeedStream mFeedStream;
+
+    @Mock
+    private SnackbarManager mSnackbarManager;
+    @Mock
+    private NativePageNavigationDelegate mPageNavigationDelegate;
+    @Mock
+    private BottomSheetController mBottomSheetController;
+    @Mock
+    private FeedStreamSurface.Natives mFeedStreamSurfaceJniMock;
+    @Mock
+    private FeedServiceBridge.Natives mFeedServiceBridgeJniMock;
+
+    @Rule
+    public JniMocker mocker = new JniMocker();
+
+    @Before
+    public void setUp() throws Exception {
+        MockitoAnnotations.initMocks(this);
+        mActivity = Robolectric.buildActivity(Activity.class).get();
+        mocker.mock(FeedStreamSurfaceJni.TEST_HOOKS, mFeedStreamSurfaceJniMock);
+        mocker.mock(FeedServiceBridgeJni.TEST_HOOKS, mFeedServiceBridgeJniMock);
+
+        when(mFeedServiceBridgeJniMock.getLoadMoreTriggerLookahead())
+                .thenReturn(LOAD_MORE_TRIGGER_LOOKAHEAD);
+
+        mFeedStream = new FeedStream(mActivity, false, mSnackbarManager, mPageNavigationDelegate,
+                mBottomSheetController);
+        mFeedStream.onCreate(null);
+        mRecyclerView = (RecyclerView) mFeedStream.getView();
+        mLayoutManager = new FakeLinearLayoutManager(mActivity);
+        mRecyclerView.setLayoutManager(mLayoutManager);
+    }
+
+    @Test
+    public void testIsChildAtPositionVisible() {
+        mLayoutManager.mFirstVisiblePosition = 0;
+        mLayoutManager.mLastVisiblePosition = 1;
+        assertThat(mFeedStream.isChildAtPositionVisible(-2)).isFalse();
+        assertThat(mFeedStream.isChildAtPositionVisible(-1)).isFalse();
+        assertThat(mFeedStream.isChildAtPositionVisible(0)).isTrue();
+        assertThat(mFeedStream.isChildAtPositionVisible(1)).isTrue();
+        assertThat(mFeedStream.isChildAtPositionVisible(2)).isFalse();
+    }
+
+    @Test
+    public void testIsChildAtPositionVisible_nothingVisible() {
+        assertThat(mFeedStream.isChildAtPositionVisible(0)).isFalse();
+    }
+
+    @Test
+    public void testIsChildAtPositionVisible_validTop() {
+        mLayoutManager.mFirstVisiblePosition = 0;
+        assertThat(mFeedStream.isChildAtPositionVisible(0)).isFalse();
+    }
+
+    @Test
+    public void testIsChildAtPositionVisible_validBottom() {
+        mLayoutManager.mLastVisiblePosition = 1;
+        assertThat(mFeedStream.isChildAtPositionVisible(0)).isFalse();
+    }
+
+    @Test
+    public void testGetChildTopAt_noVisibleChild() {
+        assertThat(mFeedStream.getChildTopAt(0)).isEqualTo(POSITION_NOT_KNOWN);
+    }
+
+    @Test
+    public void testGetChildTopAt_noChild() {
+        mLayoutManager.mFirstVisiblePosition = 0;
+        mLayoutManager.mLastVisiblePosition = 1;
+        assertThat(mFeedStream.getChildTopAt(0)).isEqualTo(POSITION_NOT_KNOWN);
+    }
+
+    @Test
+    public void testGetChildTopAt() {
+        mLayoutManager.mFirstVisiblePosition = 0;
+        mLayoutManager.mLastVisiblePosition = 1;
+        View view = new FrameLayout(mActivity);
+        mLayoutManager.addChildToPosition(0, view);
+
+        assertThat(mFeedStream.getChildTopAt(0)).isEqualTo(view.getTop());
+    }
+
+    @Test
+    public void testCheckScrollingForLoadMore_StreamContentHidden() {
+        // By default, stream content is not visible.
+        final int triggerDistance = getLoadMoreTriggerScrollDistance();
+        mFeedStream.checkScrollingForLoadMore(triggerDistance);
+        verify(mFeedStreamSurfaceJniMock, never())
+                .loadMore(anyLong(), any(FeedStreamSurface.class), any(Callback.class));
+    }
+
+    @Test
+    public void testCheckScrollingForLoadMore_StreamContentVisible() {
+        mFeedStream.setStreamContentVisibility(true);
+        final int triggerDistance = getLoadMoreTriggerScrollDistance();
+        final int itemCount = 10;
+
+        // loadMore not triggered due to not enough accumulated scrolling distance.
+        mFeedStream.checkScrollingForLoadMore(triggerDistance / 2);
+        verify(mFeedStreamSurfaceJniMock, never())
+                .loadMore(anyLong(), any(FeedStreamSurface.class), any(Callback.class));
+
+        // loadMore not triggered due to last visible item not falling into lookahead range.
+        mLayoutManager.mLastVisiblePosition = itemCount - LOAD_MORE_TRIGGER_LOOKAHEAD - 1;
+        mLayoutManager.mItemCount = itemCount;
+        mFeedStream.checkScrollingForLoadMore(triggerDistance / 2);
+        verify(mFeedStreamSurfaceJniMock, never())
+                .loadMore(anyLong(), any(FeedStreamSurface.class), any(Callback.class));
+
+        // loadMore triggered.
+        mLayoutManager.mLastVisiblePosition = itemCount - LOAD_MORE_TRIGGER_LOOKAHEAD + 1;
+        mLayoutManager.mItemCount = itemCount;
+        mFeedStream.checkScrollingForLoadMore(triggerDistance / 2);
+        verify(mFeedStreamSurfaceJniMock)
+                .loadMore(anyLong(), any(FeedStreamSurface.class), any(Callback.class));
+    }
+
+    private int getLoadMoreTriggerScrollDistance() {
+        return (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP,
+                FeedStream.LOAD_MORE_TRIGGER_SCROLL_DISTANCE_DP,
+                mRecyclerView.getResources().getDisplayMetrics());
+    }
+}

chrome/browser/android/feed/v2/feed_service_bridge.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/android/feed/v2/feed_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "components/feed/core/v2/config.h"
 #include "components/feed/core/v2/public/feed_service.h"
 
 namespace feed {
@@ -32,6 +33,10 @@ static void JNI_FeedServiceBridge_Startup(JNIEnv* env) {
   FeedServiceFactory::GetForBrowserContext(profile);
 }
 
+static int JNI_FeedServiceBridge_GetLoadMoreTriggerLookahead(JNIEnv* env) {
+  return GetFeedConfig().load_more_trigger_lookahead;
+}
+
 std::string FeedServiceBridge::GetLanguageTag() {
   JNIEnv* env = base::android::AttachCurrentThread();
   return ConvertJavaStringToUTF8(env,

chrome/browser/android/feed/v2/feed_stream_surface.cc

@@ -7,6 +7,7 @@
 #include <string>
 #include <vector>
 
+#include "base/android/callback_android.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
@@ -22,6 +23,7 @@
 
 using base::android::JavaParamRef;
 using base::android::JavaRef;
+using base::android::ScopedJavaGlobalRef;
 using base::android::ScopedJavaLocalRef;
 using base::android::ToJavaByteArray;
 
@@ -75,8 +77,12 @@ void FeedStreamSurface::StreamUpdate(
 }
 
 void FeedStreamSurface::LoadMore(JNIEnv* env,
-                                 const JavaParamRef<jobject>& obj) {
-  feed_stream_api_->LoadMore(GetSurfaceId(), base::DoNothing());
+                                 const JavaParamRef<jobject>& obj,
+                                 const JavaParamRef<jobject>& callback_obj) {
+  feed_stream_api_->LoadMore(
+      GetSurfaceId(),
+      base::BindOnce(&base::android::RunBooleanCallbackAndroid,
+                     ScopedJavaGlobalRef<jobject>(callback_obj)));
 }
 
 void FeedStreamSurface::ProcessThereAndBackAgain(

chrome/browser/android/feed/v2/feed_stream_surface.h

@@ -31,7 +31,9 @@ class FeedStreamSurface : public FeedStreamApi::SurfaceInterface {
 
   void OnStreamUpdated(const feedui::StreamUpdate& stream_update);
 
-  void LoadMore(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
+  void LoadMore(JNIEnv* env,
+                const base::android::JavaParamRef<jobject>& obj,
+                const base::android::JavaParamRef<jobject>& callback_obj);
 
   void ProcessThereAndBackAgain(
       JNIEnv* env,

components/feed/core/v2/config.cc

@@ -57,6 +57,10 @@ void OverrideWithFinch(Config* config) {
       base::TimeDelta::FromSecondsD(base::GetFieldTrialParamByFeatureAsDouble(
           kInterestFeedV2, "model_unload_timeout_seconds",
           config->model_unload_timeout.InSecondsF()));
+
+  config->load_more_trigger_lookahead = base::GetFieldTrialParamByFeatureAsInt(
+      kInterestFeedV2, "load_more_trigger_lookahead",
+      config->load_more_trigger_lookahead);
 }
 
 }  // namespace

components/feed/core/v2/config.h

@@ -31,6 +31,9 @@ struct Config {
   // If no surfaces are attached, the stream model is unloaded after this
   // timeout.
   base::TimeDelta model_unload_timeout = base::TimeDelta::FromSeconds(1);
+  // How far ahead in number of items from last visible item to final item
+  // before attempting to load more content.
+  int load_more_trigger_lookahead = 5;
 };
 
 // Gets the current configuration.