[Tabbed mode] Restore tabs even if TabState is missing

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java

 // Copyright 2014 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.tabmodel;
 
 import android.content.Context;
 import android.os.AsyncTask;
 import android.os.StrictMode;
 import android.text.TextUtils;
 import android.util.Pair;
 import android.util.SparseIntArray;
 
 import org.chromium.base.ImportantFileWriterAndroid;
 import org.chromium.base.Log;
 import org.chromium.base.StreamUtil;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.TabState;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.content_public.browser.LoadUrlParams;
 
 import java.io.BufferedInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Deque;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 
 /**
  * This class handles saving and loading tab state from the persistent storage.
  */
 public class TabPersistentStore extends TabPersister {
-    private static final String TAG = "TabPersistentStore";
+    private static final String TAG = "tabmodel";
 
-    /** The current version of the saved state file. */
-    private static final int SAVED_STATE_VERSION = 4;
+    /**
+     * The current version of the saved state file.
+     * Version 4: In addition to the tab's ID, save the tab's last URL.
+     * Version 5: In addition to the total tab count, save the incognito tab count.
+     */
+    private static final int SAVED_STATE_VERSION = 5;

[David Trainor- moved to gerrit, 2015/10/17 05:58:41]

at some point we should probably just go to protos.

[gone, 2015/10/19 17:42:06]

Yeah, was planning to but only after we had a standardized way of saving tabs. 
That'll happen soon™, but we should fix tab loss as much as we can given how far
off that always ends up being.

 
     private static final String BASE_STATE_FOLDER = "tabs";
 
     /** The name of the file where the state is saved. */
     @VisibleForTesting
     public static final String SAVED_STATE_FILE = "tab_state";
 
     /** Prevents two copies of the Migration task from being created. */
     private static final Object MIGRATION_LOCK = new Object();
 
     /** Prevents race conditions when setting the sBaseStateDirectory. */
     private static final Object BASE_STATE_DIRECTORY_LOCK = new Object();
 
     /**
      * Callback interface to use while reading the persisted TabModelSelector info from disk.
      */
     public static interface OnTabStateReadCallback {
         /**
          * To be called as the details about a persisted Tab are read from the TabModelSelector's
          * persisted data.
-         * @param index The index out of all tabs for the current tab read.
-         * @param id The id for the current tab read.
-         * @param url The url for the current tab read.
-         * @param isStandardActiveIndex Whether the current tab read is the normal active tab.
+         * @param index                  The index out of all tabs for the current tab read.
+         * @param id                     The id for the current tab read.
+         * @param url                    The url for the current tab read.
+         * @param incognitoCount         Whether the Tab is definitely Incognito, or null if it

[David Trainor- moved to gerrit, 2015/10/17 05:58:41]

incognitoCount -> isIncognito?  Also, when can't we tell?  That sounds scary.

[gone, 2015/10/19 17:42:06]

Ack; you seem to have figured this out later on.

+         *                               couldn't be determined because of a lack of information.
+         * @param isStandardActiveIndex  Whether the current tab read is the normal active tab.
          * @param isIncognitoActiveIndex Whether the current tab read is the incognito active tab.
          */
-        void onDetailsRead(int index, int id, String url,
+        void onDetailsRead(int index, int id, String url, Boolean isIncognito,
                 boolean isStandardActiveIndex, boolean isIncognitoActiveIndex);
     }
 
     /**
      * Alerted at various stages of initialization.
      */
     public static interface TabPersistentStoreObserver {
         /**
          * To be called when the file containing the initial information about the TabModels has
          * been loaded.
@@ skipping 260 matching lines @@
             while (!mTabsToRestore.isEmpty()
                     && mNormalTabsRestored.size() == 0
                     && mIncognitoTabsRestored.size() == 0) {
                 TabRestoreDetails tabToRestore = mTabsToRestore.removeFirst();
                 restoreTab(tabToRestore, true);
             }
         }
         loadNextTab();
     }
 
-    /** TODO(tedchoc): Remove this after migrating all callers to restoreTabStateForUrl. */
-    public boolean restoreTabState(String url) {
-        return restoreTabStateForUrl(url);
+    /**
+     * If a tab is being restored with the given url, then restore the tab in a frozen state
+     * synchronously.
+     */
+    public void restoreTabStateForUrl(String url) {
+        restoreTabStateInternal(url, Tab.INVALID_TAB_ID);
     }
 
     /**
-     * If a tab is being restored with the given url, then restore the tab
-     * in a frozen state synchronously.
-     *
-     * @return Whether the tab was restored.
+     * If a tab is being restored with the given id, then restore the tab in a frozen state
+     * synchronously.
      */
-    public boolean restoreTabStateForUrl(String url) {
-        return restoreTabStateInternal(url, Tab.INVALID_TAB_ID);
+    public void restoreTabStateForId(int id) {
+        restoreTabStateInternal(null, id);
     }
 
-    /**
-     * If a tab is being restored with the given id, then restore the tab
-     * in a frozen state synchronously.
-     *
-     * @return Whether the tab was restored.
-     */
-    public boolean restoreTabStateForId(int id) {
-        return restoreTabStateInternal(null, id);
-    }
-
-    private boolean restoreTabStateInternal(String url, int id) {
+    private void restoreTabStateInternal(String url, int id) {
         TabRestoreDetails tabToRestore = null;
         if (mLoadTabTask != null) {
             if ((url == null && mLoadTabTask.mTabToRestore.id == id)
                     || (url != null && TextUtils.equals(mLoadTabTask.mTabToRestore.url, url))) {
                 // Steal the task of restoring the tab from the active load tab task.
                 mLoadTabTask.cancel(false);
                 tabToRestore = mLoadTabTask.mTabToRestore;
                 loadNextTab();  // Queue up async task to load next tab after we're done here.
             }
         }
 
         if (tabToRestore == null) {
             if (url == null) {
                 tabToRestore = getTabToRestoreById(id);
             } else {
                 tabToRestore = getTabToRestoreByUrl(url);
             }
         }
 
         if (tabToRestore != null) {
             mTabsToRestore.remove(tabToRestore);
-            return restoreTab(tabToRestore, false);
-        } else {
-            return false;
+            restoreTab(tabToRestore, false);
         }
     }
 
-    private boolean restoreTab(TabRestoreDetails tabToRestore, boolean setAsActive) {
+    private void restoreTab(TabRestoreDetails tabToRestore, boolean setAsActive) {
         // As we do this in startup, and restoring the active tab's state is critical, we permit
         // this read.
         // TODO(joth): An improved solution would be to pre-read the files on a background and
         // block here waiting for that task to complete only if needed. See http://b/5518170
-        boolean tabRestored = false;
         StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
         try {
             TabState state = TabState.restoreTabState(getStateDirectory(), tabToRestore.id);
-
-            if (state != null) {
-                restoreTab(tabToRestore, state, setAsActive);
-                tabRestored = true;
-            }
+            restoreTab(tabToRestore, state, setAsActive);
         } catch (Exception e) {
             // Catch generic exception to prevent a corrupted state from crashing the app
             // at startup.
             Log.d(TAG, "loadTabs exception: " + e.toString(), e);
         } finally {
             StrictMode.setThreadPolicy(oldPolicy);
         }
-        return tabRestored;
     }
 
     private void restoreTab(
             TabRestoreDetails tabToRestore, TabState tabState, boolean setAsActive) {
-        TabModel model = mTabModelSelector.getModel(tabState.isIncognito());
-        SparseIntArray restoredTabs = tabState.isIncognito()
-                ? mIncognitoTabsRestored : mNormalTabsRestored;
+        // If we don't have enough information about the Tab, bail out.
+        boolean isIncognito = isIncognitoTabBeingRestored(tabToRestore, tabState);
+        if (tabState == null) {
+            if (tabToRestore.isIncognito == null) {
+                Log.w(TAG, "Failed to restore tab: not enough info about its type was available.");
+                return;
+            } else if (isIncognito) {
+                Log.i(TAG, "Failed to restore Incognito tab: its TabState could not be restored.");
+                return;
+            }
+        }
+
+        TabModel model = mTabModelSelector.getModel(isIncognito);
+        SparseIntArray restoredTabs = isIncognito ? mIncognitoTabsRestored : mNormalTabsRestored;
         int restoredIndex = 0;
         if (restoredTabs.size() > 0
                 && tabToRestore.originalIndex > restoredTabs.keyAt(restoredTabs.size() - 1)) {
             // Restore at end if our index is greater than all restored tabs.
             restoredIndex = restoredTabs.size();
         } else {
              // Otherwise try to find the tab we should restore before, if any.
             for (int i = 0; i < restoredTabs.size(); i++) {
                 if (restoredTabs.keyAt(i) > tabToRestore.originalIndex) {
                     Tab nextTabByIndex = TabModelUtils.getTabById(model, restoredTabs.valueAt(i));
                     restoredIndex = nextTabByIndex != null ? model.indexOf(nextTabByIndex) : -1;
                     break;
                 }
             }
         }
-        mTabCreatorManager.getTabCreator(tabState.isIncognito()).createFrozenTab(tabState,
-                tabToRestore.id, restoredIndex);
+
+        if (tabState != null) {
+            mTabCreatorManager.getTabCreator(isIncognito).createFrozenTab(
+                    tabState, tabToRestore.id, restoredIndex);
+        } else {
+            Log.w(TAG, "Failed to restore TabState; creating Tab with last known URL.");
+            Tab fallbackTab = mTabCreatorManager.getTabCreator(isIncognito).createNewTab(
+                    new LoadUrlParams(tabToRestore.url), TabModel.TabLaunchType.FROM_RESTORE, null);
+            model.moveTab(fallbackTab.getId(), restoredIndex);
+        }
+
         if (setAsActive) {
             TabModelUtils.setIndex(model, TabModelUtils.getTabIndexById(model, tabToRestore.id));
         }
         restoredTabs.put(tabToRestore.originalIndex, tabToRestore.id);
     }
 
     /**
      * @return Number of restored tabs on cold startup.
      */
     public int getRestoredTabCount() {
@@ skipping 99 matching lines @@
         for (TabRestoreDetails details : mTabsToRestore) {
             tabsToRestore.add(details);
         }
 
         return serializeTabModelSelector(mTabModelSelector, tabsToRestore);
     }
 
     /**
      * Serializes {@code selector} to a byte array, copying out the data pertaining to tab ordering
      * and selected indices.
-     * @param selector The {@link TabModelSelector} to serialize.
-     * @return         A {@code byte[]} containing the serialized state of {@code selector}.
+     * @param selector      The {@link TabModelSelector} to serialize.
+     * @param tabsToRestore Tabs that are in the process of being restored.
+     * @return              {@code byte[]} containing the serialized state of {@code selector}.
      */
     @VisibleForTesting
     public static byte[] serializeTabModelSelector(TabModelSelector selector,
             List<TabRestoreDetails> tabsToRestore) throws IOException {
         ThreadUtils.assertOnUiThread();
 
         TabModel incognitoList = selector.getModel(true);
         TabModel standardList = selector.getModel(false);
 
         // Determine how many Tabs there are, including those not yet been added to the TabLists.
         int numAlreadyLoaded = incognitoList.getCount() + standardList.getCount();
         int numStillBeingLoaded = tabsToRestore == null ? 0 : tabsToRestore.size();
         int numTabsTotal = numStillBeingLoaded + numAlreadyLoaded;
 
         // Save the index file containing the list of tabs to restore.
         ByteArrayOutputStream output = new ByteArrayOutputStream();
         DataOutputStream stream = new DataOutputStream(output);
         stream.writeInt(SAVED_STATE_VERSION);
         stream.writeInt(numTabsTotal);
+        stream.writeInt(incognitoList.getCount());
         stream.writeInt(incognitoList.index());
         stream.writeInt(standardList.index() + incognitoList.getCount());
-        Log.d(TAG, "Serializing tab lists; counts: "
-                + standardList.getCount() + "," + incognitoList.getCount());
+        Log.d(TAG, "Serializing tab lists; counts: " + standardList.getCount()
+                + ", " + incognitoList.getCount()
+                + ", " + (tabsToRestore == null ? 0 : tabsToRestore.size()));
 
         // Save incognito state first, so when we load, if the incognito files are unreadable
         // we can fall back easily onto the standard selected tab.
         for (int i = 0; i < incognitoList.getCount(); i++) {
             stream.writeInt(incognitoList.getTabAt(i).getId());
             stream.writeUTF(incognitoList.getTabAt(i).getUrl());
         }
         for (int i = 0; i < standardList.getCount(); i++) {
             stream.writeInt(standardList.getTabAt(i).getId());
             stream.writeUTF(standardList.getTabAt(i).getUrl());
@@ skipping 45 matching lines @@
                 if (!folder.isDirectory()) continue;
                 boolean readDir = folder.equals(stateFolder);
                 final Deque<TabRestoreDetails> restoreList = readDir ? mTabsToRestore : null;
                 final boolean isIncognitoSelected = mTabModelSelector.isIncognitoSelected();
 
                 // TODO(dfalcantara): Store the max tab ID in a shared preference so that it can be
                 //                    shared with all of the other modes that Chrome runs in and to
                 //                    avoid reading in all of the possible app_tabs subdirectories.
                 int curId = readSavedStateFile(folder, new OnTabStateReadCallback() {
                     @Override
-                    public void onDetailsRead(int index, int id, String url,
+                    public void onDetailsRead(int index, int id, String url, Boolean isIncognito,
                             boolean isStandardActiveIndex, boolean isIncognitoActiveIndex) {
                         // If we're not trying to build the restore list skip the build part.
                         // We've already read all the state for this entry from the input stream.
                         if (restoreList == null) return;
 
                         // Note that incognito tab may not load properly so we may need to use
                         // the current tab from the standard model.
                         // This logic only works because we store the incognito indices first.
+                        TabRestoreDetails details =
+                                new TabRestoreDetails(id, index, isIncognito, url);
+
                         if ((isIncognitoActiveIndex && isIncognitoSelected)
                                 || (isStandardActiveIndex && !isIncognitoSelected)) {
                             // Active tab gets loaded first
-                            restoreList.addFirst(new TabRestoreDetails(id, index, url));
+                            restoreList.addFirst(details);
                         } else {
-                            restoreList.addLast(new TabRestoreDetails(id, index, url));
+                            restoreList.addLast(details);
                         }
 
                         if (mObserver != null) {
                             mObserver.onDetailsRead(
                                     index, id, url, isStandardActiveIndex, isIncognitoActiveIndex);
                         }
                     }
                 });
                 maxId = Math.max(maxId, curId);
             }
@@ skipping 12 matching lines @@
         // block here waiting for that task to complete only if needed. See http://b/5518170
         StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
         try {
             File stateFile = new File(folder, SAVED_STATE_FILE);
             if (!stateFile.exists()) return 0;
 
             stream = new DataInputStream(new BufferedInputStream(new FileInputStream(stateFile)));
 
             int nextId = 0;
             boolean skipUrlRead = false;
+            boolean skipIncognitoCount = false;
             final int version = stream.readInt();
             if (version != SAVED_STATE_VERSION) {
-                if (version == 3 && SAVED_STATE_VERSION == 4) {
-                    // Can transition from version 3 to version 4 by skipping URL reads.
-                    skipUrlRead = true;
-                } else {
-                    return 0;
-                }
+                // We don't support restoring Tab data from before M18.
+                if (version < 3) return 0;
+
+                // Older versions are missing newer data.
+                if (version < 5) skipIncognitoCount = true;
+                if (version < 4) skipUrlRead = true;
             }
 
             final int count = stream.readInt();
+            final int incognitoCount = skipIncognitoCount ? -1 : stream.readInt();
             final int incognitoActiveIndex = stream.readInt();
             final int standardActiveIndex = stream.readInt();
             if (count < 0 || incognitoActiveIndex >= count || standardActiveIndex >= count) {
                 throw new IOException();
             }
 
             for (int i = 0; i < count; i++) {
                 int id = stream.readInt();
                 String tabUrl = skipUrlRead ? "" : stream.readUTF();
                 if (id >= nextId) nextId = id + 1;
 
-                callback.onDetailsRead(
-                        i, id, tabUrl, i == standardActiveIndex, i == incognitoActiveIndex);
+                Boolean isIncognito = (incognitoCount < 0) ? null : i < incognitoCount;

[David Trainor- moved to gerrit, 2015/10/17 05:58:41]

Ahh so we don't know if we don't know how many tabs are actually incognito or
not.  Ok!

[gone, 2015/10/19 17:42:06]

Acknowledged.

+                callback.onDetailsRead(i, id, tabUrl, isIncognito,
+                        i == standardActiveIndex, i == incognitoActiveIndex);
             }
             return nextId;
         } finally {
             StreamUtil.closeQuietly(stream);
             StrictMode.setThreadPolicy(oldPolicy);
         }
     }
 
     private void saveNextTab() {
         if (mSaveTabTask != null) return;
@@ skipping 169 matching lines @@
             } catch (Exception e) {
                 Log.w(TAG, "Unable to read state: " + e);
                 return null;
             }
         }
 
         @Override
         protected void onPostExecute(TabState tabState) {
             if (mDestroyed || isCancelled()) return;
 
-            if (tabState != null && ((tabState.isIncognito() && !mCancelIncognitoTabLoads)
-                    || (!tabState.isIncognito() && !mCancelNormalTabLoads))) {
-                restoreTab(mTabToRestore, tabState, false);
-            }
+            boolean isIncognito = isIncognitoTabBeingRestored(mTabToRestore, tabState);
+            boolean isLoadCancelled = (isIncognito && mCancelIncognitoTabLoads)
+                    || (!isIncognito && mCancelNormalTabLoads);
+            if (!isLoadCancelled) restoreTab(mTabToRestore, tabState, false);
+
             loadNextTab();
         }
     }
 
     private static final class TabRestoreDetails {
 
         public final int id;
         public final int originalIndex;
         public final String url;
+        public final Boolean isIncognito;
 
-        public TabRestoreDetails(int id, int originalIndex, String url) {
+        public TabRestoreDetails(int id, int originalIndex, Boolean isIncognito, String url) {
             this.id = id;
             this.originalIndex = originalIndex;
             this.url = url;
+            this.isIncognito = isIncognito;
         }
     }
 
     private class FileMigrationTask extends AsyncTask<Void, Void, Void> {
         @Override
         protected Void doInBackground(Void... params) {
             File oldFolder = mContext.getFilesDir();
             File newFolder = getStateDirectory();
             // If we already have files here just return.
             File[] newFiles = newFolder.listFiles();
@@ skipping 18 matching lines @@
             }
 
             return null;
         }
     }
 
     private boolean isTabUrlContentScheme(Tab tab) {
         String url = tab.getUrl();
         return url != null && url.startsWith("content");
     }
+
+    /**
+     * Determines if a Tab being restored is definitely an Incognito Tab.
+     *
+     * This function can fail to determine if a Tab is incognito if not enough data about the Tab
+     * was successfully saved out.
+     *
+     * @return True if the tab is definitely Incognito, false if it's not or if it's undecideable.
+     */
+    private static boolean isIncognitoTabBeingRestored(
+            TabRestoreDetails tabDetails, TabState tabState) {
+        if (tabState != null) {
+            // The Tab's previous state was completely restored.
+            return tabState.isIncognito();
+        } else if (tabDetails.isIncognito != null) {
+            // The TabState couldn't be restored, but we have some information about the tab.
+            return tabDetails.isIncognito;
+        } else {
+            // The tab's type is undecideable.
+            return false;
+        }
+    }
 }