Fix issues with state save/restore

 Bug: 5144214
 Tracked down the issue with messed up state to a bug where
 tab ids were not unique, and would actually get messed up in restore.
 Switched it to the tab's responsibility to assign an id to itself
 in the ctor to make sure all possible paths where a tab is created
 are fixed as well as the tab being the best informed about whether
 or not it has an ID to restore from. Added some checks to watch for
 a similar problem in the future as well.

Change-Id: Icd8333232a0baca7a3639323538886ea595de05a

3 files changed, 50 insertions, 22 deletions

diff --git a/src/com/android/browser/Controller.java b/src/com/android/browser/Controller.java
index 92cb743b7..1e89af116 100644
--- a/src/com/android/browser/Controller.java
+++ b/src/com/android/browser/Controller.java
@@ -332,6 +332,9 @@ public class Controller
                 restoredTabs.add(t.getId());
             }
             BackgroundHandler.execute(new PruneThumbnails(mActivity, restoredTabs));
+            if (tabs.size() == 0) {
+                openTabToHomePage();
+            }
             mUi.updateTabs(tabs);
             // TabControl.restoreState() will create a new tab even if
             // restoring the state fails.
@@ -1207,13 +1210,6 @@ public class Controller
 
     // combo view callbacks
 
-    /**
-     * callback from ComboPage when clear history is requested
-     */
-    public void onRemoveParentChildRelationships() {
-        mTabControl.removeParentChildRelationShips();
-    }
-
     // key handling
     protected void onBackKey() {
         if (!mUi.onBackKey()) {
diff --git a/src/com/android/browser/Tab.java b/src/com/android/browser/Tab.java
index c519c910d..672a1a675 100644
--- a/src/com/android/browser/Tab.java
+++ b/src/com/android/browser/Tab.java
@@ -1412,6 +1412,9 @@ class Tab implements PictureListener {
                 R.dimen.tab_thumbnail_height);
         updateShouldCaptureThumbnails();
         restoreState(state);
+        if (getId() == -1) {
+            mId = TabControl.getNextId();
+        }
         setWebView(w);
         mHandler = new Handler() {
             @Override
@@ -1450,10 +1453,6 @@ class Tab implements PictureListener {
         updateShouldCaptureThumbnails();
     }
 
-    public void setId(long id) {
-        mId = id;
-    }
-
     public long getId() {
         return mId;
     }
@@ -1592,6 +1591,9 @@ class Tab implements PictureListener {
      * Set the parent tab of this tab.
      */
     void setParent(Tab parent) {
+        if (parent == this) {
+            throw new IllegalStateException("Cannot set parent to self!");
+        }
         mParent = parent;
         // This tab may have been freed due to low memory. If that is the case,
         // the parent tab id is already saved. If we are changing that id
@@ -1610,6 +1612,10 @@ class Tab implements PictureListener {
                 != mSettings.hasDesktopUseragent(getWebView())) {
             mSettings.toggleDesktopUseragent(getWebView());
         }
+
+        if (parent != null && parent.getId() == getId()) {
+            throw new IllegalStateException("Parent has same ID as child!");
+        }
     }
 
     /**
@@ -1689,6 +1695,7 @@ class Tab implements PictureListener {
         if (!mInForeground) {
             return;
         }
+        capture();
         mInForeground = false;
         pause();
         mMainView.setOnCreateContextMenuListener(null);
@@ -2175,4 +2182,27 @@ class Tab implements PictureListener {
         }
     };
 
+    @Override
+    public String toString() {
+        StringBuilder builder = new StringBuilder(100);
+        builder.append(mId);
+        builder.append(") has parent: ");
+        if (getParent() != null) {
+            builder.append("true[");
+            builder.append(getParent().getId());
+            builder.append("]");
+        } else {
+            builder.append("false");
+        }
+        builder.append(", incog: ");
+        builder.append(isPrivateBrowsingEnabled());
+        if (!isPrivateBrowsingEnabled()) {
+            builder.append(", title: ");
+            builder.append(getTitle());
+            builder.append(", url: ");
+            builder.append(getUrl());
+        }
+        return builder.toString();
+    }
+
 }
diff --git a/src/com/android/browser/TabControl.java b/src/com/android/browser/TabControl.java
index e99071a62..932a81169 100644
--- a/src/com/android/browser/TabControl.java
+++ b/src/com/android/browser/TabControl.java
@@ -62,7 +62,7 @@ class TabControl {
         mTabQueue = new ArrayList<Tab>(mMaxTabs);
     }
 
-    static long getNextId() {
+    synchronized static long getNextId() {
         return sNextId++;
     }
 
@@ -170,7 +170,6 @@ class TabControl {
     }
 
     void addPreloadedTab(Tab tab) {
-        tab.setId(getNextId());
         mTabs.add(tab);
         tab.setController(mController);
         mController.onSetWebView(tab, tab.getWebView());
@@ -197,7 +196,6 @@ class TabControl {
 
         // Create a new tab and add it to the tab list
         Tab t = new Tab(mController, w, state);
-        t.setId(getNextId());
         mTabs.add(t);
         // Initially put the tab in the background.
         t.putInBackground();
@@ -214,7 +212,6 @@ class TabControl {
 
     SnapshotTab createSnapshotTab(long snapshotId) {
         SnapshotTab t = new SnapshotTab(mController, snapshotId);
-        t.setId(getNextId());
         mTabs.add(t);
         return t;
     }
@@ -302,9 +299,20 @@ class TabControl {
             Bundle tabState = tab.saveState();
             if (tabState != null) {
                 ids[i++] = tab.getId();
-                outState.putBundle(Long.toString(tab.getId()), tabState);
+                String key = Long.toString(tab.getId());
+                if (outState.containsKey(key)) {
+                    // Dump the tab state for debugging purposes
+                    for (Tab dt : mTabs) {
+                        Log.e(LOGTAG, dt.toString());
+                    }
+                    throw new IllegalStateException(
+                            "Error saving state, duplicate tab ids!");
+                }
+                outState.putBundle(key, tabState);
             } else {
                 ids[i++] = -1;
+                // Since we won't be restoring the thumbnail, delete it
+                tab.deleteThumbnail();
             }
         }
         if (!outState.isEmpty()) {
@@ -404,7 +412,6 @@ class TabControl {
                 // Create a new tab and don't restore the state yet, add it
                 // to the tab list
                 Tab t = new Tab(mController, state);
-                t.setId(id);
                 tabMap.put(id, t);
                 mTabs.add(t);
                 // added the tab to the front as they are not current
@@ -419,11 +426,6 @@ class TabControl {
         if (mCurrentTab == -1) {
             if (getTabCount() > 0) {
                 setCurrentTab(getTab(0));
-            } else {
-                Tab t = createNewTab();
-                setCurrentTab(t);
-                t.getWebView().loadUrl(BrowserSettings.getInstance()
-                        .getHomePage());
             }
         }
         // restore parent/child relationships