Reland: Ntp: restore scroll position.

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.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.TargetApi;
 import android.content.Context;
 import android.graphics.Canvas;
 import android.graphics.Point;
 import android.graphics.Rect;
 import android.net.Uri;
 import android.os.Build;
 import android.os.SystemClock;
 import android.support.v4.view.ViewCompat;
+import android.support.v7.widget.RecyclerView;
 import android.view.ContextMenu;
 import android.view.LayoutInflater;
 import android.view.Menu;
 import android.view.MenuItem.OnMenuItemClickListener;
 import android.view.View;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.Callback;
 import org.chromium.base.CommandLine;
+import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.NativePage;
 import org.chromium.chrome.browser.UrlConstants;
@@ skipping 27 matching lines @@
 import org.chromium.chrome.browser.tab.EmptyTabObserver;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabObserver;
 import org.chromium.chrome.browser.tabmodel.TabModel;
 import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.tabmodel.TabModelUtils;
 import org.chromium.chrome.browser.tabmodel.document.TabDelegate;
 import org.chromium.chrome.browser.util.UrlUtilities;
 import org.chromium.content_public.browser.LoadUrlParams;
+import org.chromium.content_public.browser.NavigationController;
+import org.chromium.content_public.browser.NavigationEntry;
 import org.chromium.content_public.common.Referrer;
 import org.chromium.net.NetworkChangeNotifier;
 import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.ui.base.PageTransition;
 import org.chromium.ui.mojom.WindowOpenDisposition;
 
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import jp.tomorrowkey.android.gifplayer.BaseGifImage;
 
 /**
  * Provides functionality when the user interacts with the NTP.
  */
 public class NewTabPage
         implements NativePage, InvalidationAwareThumbnailProvider, TemplateUrlServiceObserver {
+    private static final String TAG = "NewTabPage";
 
     // MostVisitedItem Context menu item IDs.
     static final int ID_OPEN_IN_NEW_WINDOW = 0;
     static final int ID_OPEN_IN_NEW_TAB = 1;
     static final int ID_OPEN_IN_INCOGNITO_TAB = 2;
     static final int ID_REMOVE = 3;
 
     // UMA enum constants. CTA means the "click-to-action" icon.
     private static final String LOGO_SHOWN_UMA_NAME = "NewTabPage.LogoShown";
     private static final int STATIC_LOGO_SHOWN = 0;
     private static final int CTA_IMAGE_SHOWN = 1;
 
     private static final String LOGO_CLICK_UMA_NAME = "NewTabPage.LogoClick";
     private static final int STATIC_LOGO_CLICKED = 0;
     private static final int CTA_IMAGE_CLICKED = 1;
     private static final int ANIMATED_LOGO_CLICKED = 2;
 
+    // Key for the scroll position data that may be stored in a navigation entry.
+    private static final String NAVIGATION_ENTRY_SCROLL_POSITION_KEY = "NewTabPageScrollPosition";
+
     private static final String CHROME_CONTENT_SUGGESTIONS_REFERRER =
             "https://www.googleapis.com/auth/chrome-content-suggestions";
 
     private static MostVisitedSites sMostVisitedSitesForTests;
 
     private final Tab mTab;
     private final TabModelSelector mTabModelSelector;
     private final ChromeActivity mActivity;
 
     private final Profile mProfile;
@@ skipping 542 matching lines @@
             @Override
             public void onShown(Tab tab) {
                 // Showing the NTP is only meaningful when the page has been loaded already.
                 if (mIsLoaded) recordNTPShown();
             }
 
             @Override
             public void onHidden(Tab tab) {
                 if (mIsLoaded) recordNTPInteractionTime();
             }
+
+            @Override
+            public void onPageLoadStarted(Tab tab, String url) {
+                int scrollPosition = mNewTabPageView.getScrollPosition();
+                if (scrollPosition == RecyclerView.NO_POSITION) return;
+
+                if (mTab.getWebContents() == null) return;
+
+                NavigationController controller = mTab.getWebContents().getNavigationController();
+                int index = controller.getLastCommittedEntryIndex();
+                NavigationEntry entry = controller.getEntryAtIndex(index);
+                if (entry == null) return;
+
+                assert isNTPUrl(entry.getUrl());
+                controller.setEntryExtraData(index, NAVIGATION_ENTRY_SCROLL_POSITION_KEY,
+                        Integer.toString(scrollPosition));
+            }
         };
         mTab.addObserver(mTabObserver);
         mMostVisitedSites = buildMostVisitedSites(mProfile);
         mLogoBridge = new LogoBridge(mProfile);
         updateSearchProviderHasLogo();
 
         if (SnippetsConfig.isEnabled()) {
             mSnippetsBridge = new SnippetsBridge(mProfile);
         }
 
         LayoutInflater inflater = LayoutInflater.from(activity);
         mNewTabPageView = (NewTabPageView) inflater.inflate(R.layout.new_tab_page_view, null);
-        mNewTabPageView.initialize(mNewTabPageManager, mSearchProviderHasLogo, mSnippetsBridge);
+        mNewTabPageView.initialize(mNewTabPageManager, mSearchProviderHasLogo, mSnippetsBridge,
+                getScrollPositionFromNavigationEntry());
 
         RecordHistogram.recordBooleanHistogram(
                 "NewTabPage.MobileIsUserOnline", NetworkChangeNotifier.isOnline());
     }
 
     private static MostVisitedSites buildMostVisitedSites(Profile profile) {
         if (sMostVisitedSitesForTests != null) {
             return sMostVisitedSitesForTests;
         } else {
             return new MostVisitedSites(profile);
@@ skipping 139 matching lines @@
         mLastShownTimeNs = System.nanoTime();
         RecordUserAction.record("MobileNTPShown");
     }
 
     private void recordNTPInteractionTime() {
         RecordHistogram.recordMediumTimesHistogram(
                 "NewTabPage.TimeSpent", System.nanoTime() - mLastShownTimeNs, TimeUnit.NANOSECONDS);
     }
 
     /**
+     * Returns the value of the adapter scroll position that was stored in the last committed
+     * navigation entry. Returns {@code RecyclerView.NO_POSITION} if there is no last committed
+     * navigation entry, or if no data is found.
+     * @return The adapter scroll position.
+     */
+    private int getScrollPositionFromNavigationEntry() {
+        if (mTab.getWebContents() == null) return RecyclerView.NO_POSITION;
+
+        NavigationController controller = mTab.getWebContents().getNavigationController();
+        int index = controller.getLastCommittedEntryIndex();
+        String scrollPositionData =
+                controller.getEntryExtraData(index, NAVIGATION_ENTRY_SCROLL_POSITION_KEY);
+        if (scrollPositionData == null) return RecyclerView.NO_POSITION;

[Ted C, 2016/09/26 18:40:26]

I was testing locally and got this stack:
W/cr_NewTabPage(28076): Bad data found for scroll position: 
W/cr_NewTabPage(28076): java.lang.NumberFormatException: Invalid int: ""
W/cr_NewTabPage(28076): 	at java.lang.Integer.invalidInt(Integer.java:138)
W/cr_NewTabPage(28076): 	at java.lang.Integer.parseInt(Integer.java:358)
W/cr_NewTabPage(28076): 	at java.lang.Integer.parseInt(Integer.java:334)
W/cr_NewTabPage(28076): 	at
org.chromium.chrome.browser.ntp.NewTabPage.getScrollPositionFromNavigationEntry(NewTabPage.java:881)
W/cr_NewTabPage(28076): 	at
org.chromium.chrome.browser.ntp.NewTabPage.<init>(NewTabPage.java:705)
W/cr_NewTabPage(28076): 	at
org.chromium.chrome.browser.ntp.NativePageFactory.createNativePageForURL(NativePageFactory.java:4037)
W/cr_NewTabPage(28076): 	at
org.chromium.chrome.browser.tab.Tab.maybeShowNativePage(Tab.java:1968)
W/cr_NewTabPage(28076): 	at
org.chromium.chrome.browser.tab.Tab.handleDidCommitProvisonalLoadForFrame(Tab.java:1987)
W/cr_NewTabPage(28076): 	at
org.chromium.chrome.browser.tab.TabWebContentsObserver.didCommitProvisionalLoadForFrame(TabWebContentsObserver.java:234)
W/cr_NewTabPage(28076): 	at
org.chromium.content.browser.webcontents.WebContentsObserverProxy.didCommitProvisionalLoadForFrame(WebContentsObserverProxy.java:172)
W/cr_NewTabPage(28076): 	at
org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce(Native Method)
W/cr_NewTabPage(28076): 	at
org.chromium.base.SystemMessageHandler.handleMessage(SystemMessageHandler.java:41)
W/cr_NewTabPage(28076): 	at android.os.Handler.dispatchMessage(Handler.java:102)
W/cr_NewTabPage(28076): 	at android.os.Looper.loop(Looper.java:135)
W/cr_NewTabPage(28076): 	at
android.app.ActivityThread.main(ActivityThread.java:5254)
W/cr_NewTabPage(28076): 	at java.lang.reflect.Method.invoke(Native Method)
W/cr_NewTabPage(28076): 	at java.lang.reflect.Method.invoke(Method.java:372)
W/cr_NewTabPage(28076): 	at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
W/cr_NewTabPage(28076): 	at
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)

Should we use TextUtils.isEmpty here instead of == null?

[Michael van Ouwerkerk, 2016/09/27 14:36:58]

Agreed, done.

+
+        try {
+            return Integer.parseInt(scrollPositionData);
+        } catch (NumberFormatException e) {
+            Log.w(TAG, "Bad data found for scroll position: %s", scrollPositionData, e);
+            return RecyclerView.NO_POSITION;
+        }
+    }
+
+    /**
      * @return Whether the NTP has finished loaded.
      */
     @VisibleForTesting
     public boolean isLoadedForTests() {
         return mIsLoaded;
     }
 
     // TemplateUrlServiceObserver overrides
 
     @Override
@@ skipping 85 matching lines @@
     @Override
     public boolean shouldCaptureThumbnail() {
         return mNewTabPageView.shouldCaptureThumbnail();
     }
 
     @Override
     public void captureThumbnail(Canvas canvas) {
         mNewTabPageView.captureThumbnail(canvas);
     }
 }