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
diff --git a/src/com/android/browser/Controller.java b/src/com/android/browser/Controller.java
index 92cb743..1e89af1 100644
--- a/src/com/android/browser/Controller.java
+++ b/src/com/android/browser/Controller.java
@@ -332,6 +332,9 @@
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 @@
// 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 c519c91..672a1a6 100644
--- a/src/com/android/browser/Tab.java
+++ b/src/com/android/browser/Tab.java
@@ -1412,6 +1412,9 @@
R.dimen.tab_thumbnail_height);
updateShouldCaptureThumbnails();
restoreState(state);
+ if (getId() == -1) {
+ mId = TabControl.getNextId();
+ }
setWebView(w);
mHandler = new Handler() {
@Override
@@ -1450,10 +1453,6 @@
updateShouldCaptureThumbnails();
}
- public void setId(long id) {
- mId = id;
- }
-
public long getId() {
return mId;
}
@@ -1592,6 +1591,9 @@
* 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 @@
!= 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 @@
if (!mInForeground) {
return;
}
+ capture();
mInForeground = false;
pause();
mMainView.setOnCreateContextMenuListener(null);
@@ -2175,4 +2182,27 @@
}
};
+ @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 e99071a..932a811 100644
--- a/src/com/android/browser/TabControl.java
+++ b/src/com/android/browser/TabControl.java
@@ -62,7 +62,7 @@
mTabQueue = new ArrayList<Tab>(mMaxTabs);
}
- static long getNextId() {
+ synchronized static long getNextId() {
return sNextId++;
}
@@ -170,7 +170,6 @@
}
void addPreloadedTab(Tab tab) {
- tab.setId(getNextId());
mTabs.add(tab);
tab.setController(mController);
mController.onSetWebView(tab, tab.getWebView());
@@ -197,7 +196,6 @@
// 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 @@
SnapshotTab createSnapshotTab(long snapshotId) {
SnapshotTab t = new SnapshotTab(mController, snapshotId);
- t.setId(getNextId());
mTabs.add(t);
return t;
}
@@ -302,9 +299,20 @@
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 @@
// 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 @@
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