NTP: Prevent conflict of dismiss animations and scroll position handling.

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

 // Copyright 2015 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.ntp;
 
 import android.annotation.SuppressLint;
 import android.content.Context;
 import android.content.res.Configuration;
 import android.graphics.Canvas;
@@ skipping 40 matching lines @@
 import org.chromium.chrome.browser.widget.displaystyle.UiConfig;
 import org.chromium.ui.base.DeviceFormFactor;
 
 /**
  * The native new tab page, represented by some basic data such as title and url, and an Android
  * View that displays the page.
  */
 public class NewTabPageView extends FrameLayout implements TileGroup.Observer {
     private static final String TAG = "NewTabPageView";
 
-    private static final long SNAP_SCROLL_DELAY_MS = 30;
+    private static final long SCROLL_RUNNABLE_DELAY_MS = 30;
 
     /**
      * Experiment parameter for the maximum number of tile suggestion rows to show.
      */
     private static final String PARAM_NTP_MAX_TILE_ROWS = "ntp_max_tile_rows";
 
     /**
      * Experiment parameter for the number of tile title lines to show.
      */
     private static final String PARAM_NTP_TILE_TITLE_LINES = "ntp_tile_title_lines";
@@ skipping 22 matching lines @@
 
     private OnSearchBoxScrollListener mSearchBoxScrollListener;
 
     private ChromeActivity mActivity;
     private NewTabPageManager mManager;
     private LogoView.Delegate mLogoDelegate;
     private TileGroup.Delegate mTileGroupDelegate;
     private TileGroup mTileGroup;
     private UiConfig mUiConfig;
     private Runnable mSnapScrollRunnable;
+    private Runnable mUpdateSearchBoxOnScrollRunnable;
     private boolean mFirstShow = true;
     private boolean mSearchProviderHasLogo = true;
     private boolean mPendingSnapScroll;
     private boolean mInitialized;
     private int mLastScrollY = -1;
 
     /**
      * The number of asynchronous tasks that need to complete before the page is done loading.
      * This starts at one to track when the view is finished attaching to the window.
      */
@@ skipping 75 matching lines @@
 
         mRecyclerView = new NewTabPageRecyclerView(getContext());
         mRecyclerView.setContainsLocationBar(manager.isLocationBarShownInNTP());
         addView(mRecyclerView);
 
         // Don't attach now, the recyclerView itself will determine when to do it.
         mNewTabPageLayout = mRecyclerView.getAboveTheFoldView();
 
         mRecyclerView.setItemAnimator(new DefaultItemAnimator() {
             @Override
+            public boolean animateMove(ViewHolder holder, int fromX, int fromY, int toX, int toY) {
+                // If |mNewTabPageLayout| is animated by the RecyclerView because an item below it

[Bernhard Bauer, 2017/03/27 17:36:54]

Stupid question, why doesn't it work to call updateSearchBoxOnScroll in here if
the animated item is the NewTabPageLayout?

[Michael van Ouwerkerk, 2017/03/28 10:44:15]

Because this method (animateMove) is called only once for each item than needs
to be moved, in order to schedule its animation. There are no intermediate
updates that would allow us to respond with causing jank, and we only see the
end scroll state here.

+                // was dismissed, avoid also manipulating its vertical offset in our scroll handling
+                // at the same time. The onScrolled() method is called when an item is dismissed and
+                // the item at the top of the viewport is repositioned.
+                if (holder.itemView == mNewTabPageLayout) setUrlFocusAnimationsDisabled(true);
+
+                // Cancel any pending scroll update handling, a new one will be scheduled in
+                // onAnimationFinished().
+                mRecyclerView.removeCallbacks(mUpdateSearchBoxOnScrollRunnable);
+
+                return super.animateMove(holder, fromX, fromY, toX, toY);
+            }
+
+            @Override
             public void onAnimationFinished(ViewHolder viewHolder) {
                 super.onAnimationFinished(viewHolder);
-                // When removing sections, because the animations are all translations, the
-                // scroll events don't fire and we can get in the situation where the toolbar
-                // buttons disappear.
-                updateSearchBoxOnScroll();
+
+                // When an item is dismissed, the items at the top of the viewport might not move,
+                // and onScrolled() might not be called. We can get in the situation where the
+                // toolbar buttons disappear, so schedule an update for it. This can be cancelled
+                // from animateMove() in case |mNewTabPageLayout| will be moved. We don't know that
+                // from here, as the RecyclerView will animate multiple items when one is dismissed,
+                // and some will "finish" synchronously if they are already in the correct place,
+                // before other moves have even been scheduled.
+                if (viewHolder.itemView == mNewTabPageLayout) setUrlFocusAnimationsDisabled(false);
+                mRecyclerView.removeCallbacks(mUpdateSearchBoxOnScrollRunnable);
+                mRecyclerView.postDelayed(
+                        mUpdateSearchBoxOnScrollRunnable, SCROLL_RUNNABLE_DELAY_MS);

[Bernhard Bauer, 2017/03/27 17:36:54]

Is there a reason to use that particular delay (which is for snap scrolling)?
Would 0 work?

[Michael van Ouwerkerk, 2017/03/28 10:44:15]

As it turns out, there's no need to use postDelayed() here, simply post() is
sufficient.

             }
         });
 
         mContextMenuManager =
                 new ContextMenuManager(mActivity, mManager.getNavigationDelegate(), mRecyclerView);
         mActivity.getWindowAndroid().addContextMenuCloseListener(mContextMenuManager);
         manager.addDestructionObserver(new DestructionObserver() {
             @Override
             public void onDestroy() {
                 mActivity.getWindowAndroid().removeContextMenuCloseListener(mContextMenuManager);
             }
         });
 
         OfflinePageBridge offlinePageBridge =
                 OfflinePageBridge.getForProfile(Profile.getLastUsedProfile());
 
         mTileGridLayout = (TileGridLayout) mNewTabPageLayout.findViewById(R.id.tile_grid_layout);
         mTileGridLayout.setMaxRows(getMaxTileRows(searchProviderHasLogo));
         mTileGridLayout.setMaxColumns(getMaxTileColumns());
         mTileGroup = new TileGroup(mActivity, mManager, mContextMenuManager, mTileGroupDelegate,
                 /* observer = */ this, offlinePageBridge, getTileTitleLines());
 
         mSearchProviderLogoView =
                 (LogoView) mNewTabPageLayout.findViewById(R.id.search_provider_logo);
         mLogoDelegate = new LogoDelegateImpl(tab, mSearchProviderLogoView);
         mSearchBoxView = mNewTabPageLayout.findViewById(R.id.search_box);
         mNoSearchLogoSpacer = mNewTabPageLayout.findViewById(R.id.no_search_logo_spacer);
 
         mSnapScrollRunnable = new SnapScrollRunnable();
+        mUpdateSearchBoxOnScrollRunnable = new UpdateSearchBoxOnScrollRunnable();
 
         initializeSearchBoxTextView();
         initializeVoiceSearchButton();
         initializeLayoutChangeListeners();
         setSearchProviderHasLogo(searchProviderHasLogo);
 
         tab.addObserver(new EmptyTabObserver() {
             @Override
             public void onShown(Tab tab) {
                 mTileGroup.onSwitchToForeground();
@@ skipping 243 matching lines @@
 
         mRecyclerView.setOnTouchListener(new OnTouchListener() {
             @Override
             @SuppressLint("ClickableViewAccessibility")
             public boolean onTouch(View v, MotionEvent event) {
                 mRecyclerView.removeCallbacks(mSnapScrollRunnable);
 
                 if (event.getActionMasked() == MotionEvent.ACTION_CANCEL
                         || event.getActionMasked() == MotionEvent.ACTION_UP) {
                     mPendingSnapScroll = true;
-                    mRecyclerView.postDelayed(mSnapScrollRunnable, SNAP_SCROLL_DELAY_MS);
+                    mRecyclerView.postDelayed(mSnapScrollRunnable, SCROLL_RUNNABLE_DELAY_MS);
                 } else {
                     mPendingSnapScroll = false;
                 }
                 return false;
             }
         });
         TraceEvent.end(TAG + ".setupScrollHandling()");
     }
 
     private void handleScroll() {
         if (mPendingSnapScroll) {
             mRecyclerView.removeCallbacks(mSnapScrollRunnable);
-            mRecyclerView.postDelayed(mSnapScrollRunnable, SNAP_SCROLL_DELAY_MS);
+            mRecyclerView.postDelayed(mSnapScrollRunnable, SCROLL_RUNNABLE_DELAY_MS);
         }
         updateSearchBoxOnScroll();
         mRecyclerView.updatePeekingCardAndHeader();
     }
 
     /**
      * Decrements the count of pending load tasks and notifies the manager when the page load
      * is complete.
      */
     private void loadTaskCompleted() {
@@ skipping 407 matching lines @@
 
     private class SnapScrollRunnable implements Runnable {
         @Override
         public void run() {
             assert mPendingSnapScroll;
             mPendingSnapScroll = false;
 
             mRecyclerView.snapScroll(mSearchBoxView, getHeight());
         }
     }
+
+    private class UpdateSearchBoxOnScrollRunnable implements Runnable {
+        @Override
+        public void run() {
+            updateSearchBoxOnScroll();
+        }
+    }
 }