[Feed v2] Bring back toolbar when the new tab page layout view slides down

When the top toolbar in new tab page layout view is out of screen,
we need to notify SnapScrollHelper in FeedSurfaceMediator.ContentChanged
to bring back the toolbar. This is done by hooking up the stream
content change listener between FeedStream and FeedSurfaceMediator.

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

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

@@ -184,12 +184,12 @@ public class FeedStream implements Stream {
 
     @Override
     public void addOnContentChangedListener(ContentChangedListener listener) {
-        // Not longer needed.
+        mFeedStreamSurface.addContentChangedListener(listener);
     }
 
     @Override
     public void removeOnContentChangedListener(ContentChangedListener listener) {
-        // Not longer needed.
+        mFeedStreamSurface.removeContentChangedListener(listener);
     }
 
     @Override

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

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.feed.v2;
 
 import android.app.Activity;
 import android.content.Context;
+import android.os.Handler;
 import android.view.ContextThemeWrapper;
 import android.view.View;
 import android.view.ViewParent;
@@ -14,10 +15,12 @@ import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.recyclerview.widget.LinearLayoutManager;
 import androidx.recyclerview.widget.RecyclerView;
+import androidx.recyclerview.widget.RecyclerView.ItemAnimator.ItemAnimatorFinishedListener;
 
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
+import org.chromium.base.ObserverList;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
@@ -27,6 +30,7 @@ import org.chromium.base.task.TaskTraits;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.AppHooks;
 import org.chromium.chrome.browser.feed.shared.ScrollTracker;
+import org.chromium.chrome.browser.feed.shared.stream.Stream.ContentChangedListener;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.help.HelpAndFeedback;
 import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
@@ -105,6 +109,10 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
     private final NativePageNavigationDelegate mPageNavigationDelegate;
     private final HelpAndFeedback mHelpAndFeedback;
     private final ScrollReporter mScrollReporter = new ScrollReporter();
+    private final ObserverList<ContentChangedListener> mContentChangedListeners =
+            new ObserverList<ContentChangedListener>();
+    private final RecyclerViewAnimationFinishDetector mRecyclerViewAnimationFinishDetector =
+            new RecyclerViewAnimationFinishDetector();
     // True after onSurfaceOpened(), and before onSurfaceClosed().
     private boolean mOpened;
     private boolean mStreamContentVisible;
@@ -518,9 +526,12 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
 
     private void updateContentsInPlace(
             ArrayList<FeedListContentManager.FeedContent> newContentList) {
+        boolean hasContentChange = false;
+
         // 1) Builds the hash set based on keys of new contents.
         HashSet<String> newContentKeySet = new HashSet<String>();
         for (int i = 0; i < newContentList.size(); ++i) {
+            hasContentChange = true;
             newContentKeySet.add(newContentList.get(i).getKey());
         }
 
@@ -536,6 +547,7 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
         for (int i = mContentManager.getItemCount() - 1; i >= 0; --i) {
             String key = mContentManager.getContent(i).getKey();
             if (!newContentKeySet.contains(key)) {
+                hasContentChange = true;
                 mContentManager.removeContents(i, 1);
                 existingContentMap.remove(key);
             }
@@ -549,6 +561,7 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
 
             // If this is an existing content, moves it to new position.
             if (existingContentMap.containsKey(content.getKey())) {
+                hasContentChange = true;
                 mContentManager.moveContent(
                         mContentManager.findContentPositionByKey(content.getKey()), i);
                 ++i;
@@ -561,8 +574,21 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
                     && !existingContentMap.containsKey(newContentList.get(i).getKey())) {
                 ++i;
             }
+            hasContentChange = true;
             mContentManager.addContents(startIndex, newContentList.subList(startIndex, i));
         }
+
+        if (hasContentChange) {
+            mRecyclerViewAnimationFinishDetector.asyncWait();
+        }
+    }
+
+    private void notifyContentChanged() {
+        for (ContentChangedListener listener : mContentChangedListeners) {
+            // For Feed v2, we only need to report if the content has changed. All other callbacks
+            // are not used at this point.
+            listener.onContentChanged();
+        }
     }
 
     private FeedListContentManager.FeedContent createContentFromSlice(Slice slice) {
@@ -896,6 +922,7 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
         int feedCount = mContentManager.getItemCount() - mHeaderCount;
         if (feedCount > 0) {
             mContentManager.removeContents(mHeaderCount, feedCount);
+            mRecyclerViewAnimationFinishDetector.asyncWait();
         }
 
         mScrollReporter.onUnbind();
@@ -928,6 +955,14 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
         }
     }
 
+    public void addContentChangedListener(ContentChangedListener listener) {
+        mContentChangedListeners.addObserver(listener);
+    }
+
+    public void removeContentChangedListener(ContentChangedListener listener) {
+        mContentChangedListeners.removeObserver(listener);
+    }
+
     // Called when the stream is scrolled.
     void streamScrolled(int dx, int dy) {
         FeedStreamSurfaceJni.get().reportStreamScrollStart(
@@ -935,6 +970,51 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
         mScrollReporter.trackScroll(dx, dy);
     }
 
+    // Detects animation finishes in RecyclerView.
+    // https://stackoverflow.com/questions/33710605/detect-animation-finish-in-androids-recyclerview
+    private class RecyclerViewAnimationFinishDetector implements ItemAnimatorFinishedListener {
+        private boolean mWaitingStarted;
+
+        /** Asynchronousy waits for the animation to finish. */
+        public void asyncWait() {
+            if (mWaitingStarted) {
+                return;
+            }
+            mWaitingStarted = true;
+
+            // The RecyclerView has not started animating yet, so post a message to the
+            // message queue that will be run after the RecyclerView has started animating.
+            new Handler().post(() -> { checkFinish(); });
+        }
+
+        private void checkFinish() {
+            if (mRootView.isAnimating()) {
+                // The RecyclerView is still animating, try again when the animation has finished.
+                mRootView.getItemAnimator().isRunning(this);
+                return;
+            }
+
+            // The RecyclerView has animated all it's views.
+            onFinished();
+        }
+
+        private void onFinished() {
+            mWaitingStarted = false;
+
+            // This works around the bug that the out-of-screen toolbar is not brought back together
+            // with the new tab page view when it slides down. This is because the RecyclerView
+            // animation may not finish when content changed event is triggered and thus the new tab
+            // page layout view may still be partially off screen.
+            notifyContentChanged();
+        }
+
+        @Override
+        public void onAnimationsFinished() {
+            // There might still be more items that will be animated after this one.
+            new Handler().post(() -> { checkFinish(); });
+        }
+    }
+
     // Ingests scroll events and reports scroll completion back to native.
     private class ScrollReporter extends ScrollTracker {
         @Override

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

@@ -55,6 +55,7 @@ import org.chromium.base.test.util.JniMocker;
 import org.chromium.base.test.util.MetricsUtils.HistogramDelta;
 import org.chromium.chrome.browser.AppHooks;
 import org.chromium.chrome.browser.AppHooksImpl;
+import org.chromium.chrome.browser.feed.shared.stream.Stream.ContentChangedListener;
 import org.chromium.chrome.browser.help.HelpAndFeedback;
 import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
 import org.chromium.chrome.browser.ntp.NewTabPageUma;
@@ -80,6 +81,19 @@ import java.util.concurrent.TimeUnit;
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE, shadows = {ShadowPostTask.class, ShadowRecordHistogram.class})
 public class FeedStreamSurfaceTest {
+    class ContentChangeWatcher implements ContentChangedListener {
+        @Override
+        public void onContentChanged() {
+            mContentChanged = true;
+        }
+
+        @Override
+        public void onAddStarting() {}
+
+        @Override
+        public void onAddFinished() {}
+    }
+
     private static final String TEST_DATA = "test";
     private static final String TEST_URL = "https://www.chromium.org";
     private static final int LOAD_MORE_TRIGGER_LOOKAHEAD = 5;
@@ -89,6 +103,7 @@ public class FeedStreamSurfaceTest {
     private LinearLayout mParent;
     private FakeLinearLayoutManager mLayoutManager;
     private FeedListContentManager mContentManager;
+    private boolean mContentChanged;
 
     @Mock
     private SnackbarManager mSnackbarManager;
@@ -146,6 +161,7 @@ public class FeedStreamSurfaceTest {
         mRecyclerView = mFeedStreamSurface.mRootView;
         mLayoutManager = new FakeLinearLayoutManager(mActivity);
         mRecyclerView.setLayoutManager(mLayoutManager);
+        mFeedStreamSurface.addContentChangedListener(new ContentChangeWatcher());
 
         // Since we use a mockito spy, we need to replace the entry in sSurfaces.
         FeedStreamSurface.sSurfaces.clear();
@@ -162,6 +178,39 @@ public class FeedStreamSurfaceTest {
         AppHooks.setInstanceForTesting(null);
     }
 
+    @Test
+    @SmallTest
+    public void testContentChangedOnStreamUpdated() {
+        startupAndSetVisible();
+
+        // Add 1 slice.
+        StreamUpdate update = StreamUpdate.newBuilder()
+                                      .addUpdatedSlices(createSliceUpdateForNewXSurfaceSlice("a"))
+                                      .build();
+        mContentChanged = false;
+        mFeedStreamSurface.onStreamUpdated(update.toByteArray());
+        assertTrue(mContentChanged);
+        assertEquals(1, mContentManager.getItemCount());
+
+        // Remove 1 slice.
+        update = StreamUpdate.newBuilder().build();
+        mContentChanged = false;
+        mFeedStreamSurface.onStreamUpdated(update.toByteArray());
+        assertTrue(mContentChanged);
+        assertEquals(0, mContentManager.getItemCount());
+    }
+
+    @Test
+    @SmallTest
+    public void testContentChangedOnSetHeaderViews() {
+        startupAndSetVisible();
+
+        mContentChanged = false;
+        mFeedStreamSurface.setHeaderViews(Arrays.asList(new View(mActivity)));
+        assertTrue(mContentChanged);
+        assertEquals(1, mContentManager.getItemCount());
+    }
+
     @Test
     @SmallTest
     public void testAddSlicesOnStreamUpdated() {
@@ -529,7 +578,9 @@ public class FeedStreamSurfaceTest {
         assertEquals(headers + 3, mContentManager.getItemCount());
 
         // Closing the surface should remove all non-header contents.
+        mContentChanged = false;
         mFeedStreamSurface.setStreamVisibility(false);
+        assertTrue(mContentChanged);
         assertEquals(headers, mContentManager.getItemCount());
         assertEquals(v0, getNativeView(0));
         assertEquals(v1, getNativeView(1));