Refine snap scrolling on the Cards New Tab Page.

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
index 219526cee092fc74788d877f69971fbfa476a883..62215ac883d6e64f6f6a74498cb23350e2d272bb 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
@@ -45,12 +45,12 @@ import org.chromium.chrome.browser.ntp.LogoBridge.LogoObserver;
 import org.chromium.chrome.browser.ntp.MostVisitedItem.MostVisitedItemManager;
 import org.chromium.chrome.browser.ntp.NewTabPage.OnSearchBoxScrollListener;
 import org.chromium.chrome.browser.ntp.cards.CardItemDecoration;
-import org.chromium.chrome.browser.ntp.cards.CardsLayoutOperations;
 import org.chromium.chrome.browser.ntp.cards.NewTabPageAdapter;
 import org.chromium.chrome.browser.ntp.cards.NewTabPageListItem;
 import org.chromium.chrome.browser.ntp.cards.NewTabPageRecyclerView;
 import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
 import org.chromium.chrome.browser.profiles.MostVisitedSites.MostVisitedURLsObserver;
+import org.chromium.chrome.browser.util.MathUtils;
 import org.chromium.chrome.browser.util.ViewUtils;
 import org.chromium.chrome.browser.widget.RoundedIconGenerator;
 import org.chromium.ui.base.DeviceFormFactor;
@@ -415,8 +415,14 @@ public class NewTabPageView extends FrameLayout
             if (mUseCardsUi && !mRecyclerView.isFirstItemVisible()) {
                 percentage = 1f;
             } else {
-                int scrollY = getVerticalScroll();
-                percentage = Math.max(0f, Math.min(1f, scrollY / (float) mSearchBoxView.getTop()));
+                final int scrollY = getVerticalScroll();
+                final int top = mSearchBoxView.getTop();  // Relative to mNewTabPageLayout.
+                final int transitionLength = getResources()
+                        .getDimensionPixelSize(R.dimen.ntp_search_box_transition_length);
+
+                // |scrollY - top| gives the distance the search bar is from the top of the screen.
+                percentage = MathUtils.clamp(
+                        (scrollY - top + transitionLength) / (float) transitionLength, 0f, 1f);
             }
         }
 
@@ -448,22 +454,73 @@ public class NewTabPageView extends FrameLayout
     }
 
     /**
+     * If the RecyclerView is currently scrolled to between regionStart and regionEnd, smooth scroll
+     * out of the region. flipPoint is the threshold used to decide which bound of the region to
+     * scroll to.
+     */
+    private void scrollOutOfRegion(int regionStart, int flipPoint, int regionEnd) {
+        // This function is only called when we are using the RecyclerView.
+        final int currentScroll = mRecyclerView.computeVerticalScrollOffset();
+
+        if (currentScroll < regionStart || currentScroll > regionEnd) return;
+
+        if (currentScroll < flipPoint) {
+            mRecyclerView.smoothScrollBy(0, regionStart - currentScroll);
+        } else {
+            mRecyclerView.smoothScrollBy(0, regionEnd - currentScroll);
+        }
+    }
+
+    /**
+     * If the RecyclerView is currently scrolled to between regionStart and regionEnd, smooth scroll
+     * out of the region to the nearest edge.
+     */
+    private void scrollOutOfRegion(int regionStart, int regionEnd) {
+        scrollOutOfRegion(regionStart, (regionStart + regionEnd) / 2, regionEnd);
+    }
+
+    private void snapScroll() {

[Bernhard Bauer, 2016/06/28 14:00:17]

Should this whole thing move to the RecyclerView?

[PEConn, 2016/06/28 14:32:51]

I think scrollOutOfRegion should be moved into the RecyclerView. I'm not too
sure about snapScroll since it depends on a few things (the position of the
search box, the ntp height and the ntp current scroll).

[Bernhard Bauer, 2016/06/28 14:44:00]

Dunno... we could just pass them in. I think overall moving the code out of this
class outweighs the overhead of having to pass these as parameters.

[PEConn, 2016/06/28 15:39:04]

Done.

+        // Snap scroll to prevent resting in the middle of the omnibox transition.
+        final int searchBoxTransitionLength = getResources()
+                .getDimensionPixelSize(R.dimen.ntp_search_box_transition_length);
+        scrollOutOfRegion(mSearchBoxView.getTop() - searchBoxTransitionLength,

[Bernhard Bauer, 2016/06/28 14:00:17]

Do we actually want to have two calls to scrollOutOfRegion?

[PEConn, 2016/06/28 14:32:51]

I've ensured that only one should be called.

+                          mSearchBoxView.getTop());
+
+        // Snap scroll to prevent resting in the middle of the peeking card transition
+        // and to allow the peeking card to peek a bit before snapping back.
+        if (mRecyclerView.isPeekingCardVisible() && mRecyclerView.isFirstItemVisible()) {
+            final int peekingHeight = getResources().getDimensionPixelSize(
+                    R.dimen.snippets_padding_and_peeking_card_height);
+
+            // |A + B| gives the offset of the peeking card relative to the Recycler View,
+            // so scrolling to this point would put the peeking card at the top of the
+            // screen.
+            // |A + B - C| will scroll us so that the peeking card is just off the bottom
+            // of the screen.
+            // Finally, we get |A + B - C + B| because the transition starts from the

[Bernhard Bauer, 2016/06/28 14:00:17]

|A + B - C + D|?

[PEConn, 2016/06/28 14:32:51]

Done.

+            // peeking card's resting point, which is |D| from the bottom of the screen.
+            int start =  mRecyclerView.getPeekingCardTop()  // A.
+                    + getVerticalScroll()  // B.
+                    - getHeight()  // C.
+                    + peekingHeight;  // D.
+            scrollOutOfRegion(start,
+                              start + mRecyclerView.getPeekingCardHeight() / 2,
+                              start + mRecyclerView.getPeekingCardHeight() / 2);
+        }
+    }
+
+    /**
      * Sets up scrolling when snippets are enabled. It adds scroll listeners and touch listeners to
      * the RecyclerView.
      */
     private void initializeSearchBoxRecyclerViewScrollHandling() {
-        final NewTabPageUma.SnapStateObserver snapStateObserver =
-                new NewTabPageUma.SnapStateObserver();
-
         final Runnable mSnapScrollRunnable = new Runnable() {
             @Override
             public void run() {
                 assert mPendingSnapScroll;
                 mPendingSnapScroll = false;
-                NewTabPageUma.SnapState currentSnapState = CardsLayoutOperations.snapScroll(
-                        mRecyclerView, mNewTabPageLayout, mMostVisitedLayout, getVerticalScroll(),
-                        false);
-                snapStateObserver.updateSnapState(NewTabPageView.this, currentSnapState);
+
+                snapScroll();
             }
         };
 
@@ -693,7 +750,7 @@ public class NewTabPageView extends FrameLayout
         // Ensure there are no rounding issues when the animation percent is 0.
         if (transitionPercentage == 0f) searchUiAlpha = 1f;
 
-        mSearchProviderLogoView.setAlpha(searchUiAlpha);
+        // mSearchProviderLogoView.setAlpha(searchUiAlpha);

[Bernhard Bauer, 2016/06/28 14:00:17]

?

[PEConn, 2016/06/28 14:32:51]

Ah, sorry.

         mSearchBoxView.setAlpha(searchUiAlpha);
     }
 
@@ -816,10 +873,8 @@ public class NewTabPageView extends FrameLayout
         if (mUseCardsUi) {
             mRecyclerView.updatePeekingCard();
             // The positioning of elements may have been changed (since the elements expand to fill
-            // the available vertical space), so adjust the scroll. We force snapping to Most Likely
-            // so the user's snap point doesn't change on rotation.
-            CardsLayoutOperations.snapScroll(mRecyclerView, mNewTabPageLayout, mMostVisitedLayout,
-                    getVerticalScroll(), true);
+            // the available vertical space), so adjust the scroll.
+            snapScroll();
         }
     }