Bug 699164 - Port Bug 687710 [Restoring session causes hang from nsSHEntry parent loop]. r=Neil

suite/common/src/nsSessionStore.js

@@ -2891,6 +2891,7 @@ SessionStoreService.prototype = {
       shEntry.postData = stream;
     }
 
+    let childDocIdents = {};
     if (aEntry.docIdentifier) {
       // If we have a serialized document identifier, try to find an SHEntry
       // which matches that doc identifier and adopt that SHEntry's
@@ -2898,10 +2899,12 @@ SessionStoreService.prototype = {
       // for the document identifier.
       let matchingEntry = aDocIdentMap[aEntry.docIdentifier];
       if (!matchingEntry) {
-        aDocIdentMap[aEntry.docIdentifier] = shEntry;
+        matchingEntry = {shEntry: shEntry, childDocIdents: childDocIdents};
+        aDocIdentMap[aEntry.docIdentifier] = matchingEntry;
       }
       else {
-        shEntry.adoptBFCacheEntry(matchingEntry);
+        shEntry.adoptBFCacheEntry(matchingEntry.shEntry);
+        childDocIdents = matchingEntry.childDocIdents;
       }
     }
 
@@ -2923,8 +2926,24 @@ SessionStoreService.prototype = {
         //XXXzpao Wallpaper patch for bug 509315
         if (!aEntry.children[i].url)
           continue;
+
+        // We're mysteriously getting sessionrestore.js files with a cycle in
+        // the doc-identifier graph.  (That is, we have an entry where doc
+        // identifier A is an ancestor of doc identifier B, and another entry
+        // where doc identifier B is an ancestor of A.)
+        //
+        // If we were to respect these doc identifiers, we'd create a cycle in
+        // the SHEntries themselves, which causes the docshell to loop forever
+        // when it looks for the root SHEntry.
+        //
+        // So as a hack to fix this, we restrict the scope of a doc identifier
+        // to be a node's siblings and cousins, and pass childDocIdents, not
+        // aDocIdents, to _deserializeHistoryEntry.  That is, we say that two
+        // SHEntries with the same doc identifier have the same document iff
+        // they have the same parent or their parents have the same document.
+
         shEntry.AddChild(this._deserializeHistoryEntry(aEntry.children[i], aIdMap,
-                                                       aDocIdentMap), i);
+                                                       childDocIdents), i);
       }
     }
 

suite/common/tests/browser/Makefile.in

@@ -104,6 +104,8 @@ _BROWSER_FILES = \
 	browser_615394-SSWindowState_events.js \
 	browser_625257.js \
 	browser_665702-state_session.js \
+	browser_687710.js \
+	browser_687710_2.js \
         browser_694378.js \
 	browser_isempty.js \
 	browser_markPageAsFollowedLink.js \

suite/common/tests/browser/browser_687710.js

@@ -0,0 +1,44 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that sessionrestore handles cycles in the shentry graph properly.
+//
+// These cycles shouldn't be there in the first place, but they cause hangs
+// when they mysteriously appear (bug 687710).  Docshell code assumes this
+// graph is a tree and tires to walk to the root.  But if there's a cycle,
+// there is no root, and we loop forever.
+
+let stateBackup = ss.getBrowserState();
+
+let state = {windows:[{tabs:[{entries:[
+  {
+    docIdentifier: 1,
+    url: "http://example.com",
+    children: [
+      {
+        docIdentifier: 2,
+        url: "http://example.com"
+      }
+    ]
+  },
+  {
+    docIdentifier: 2,
+    url: "http://example.com",
+    children: [
+      {
+        docIdentifier: 1,
+        url: "http://example.com"
+      }
+    ]
+  }
+]}]}]}
+
+function test() {
+  registerCleanupFunction(function () {
+    ss.setBrowserState(stateBackup);
+  });
+
+  /* This test fails by hanging. */
+  ss.setBrowserState(JSON.stringify(state));
+  ok(true, "Didn't hang!");
+}

suite/common/tests/browser/browser_687710_2.js

@@ -0,0 +1,64 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that the fix for bug 687710 isn't too aggressive -- shentries which are
+// cousins should be able to share bfcache entries.
+
+let stateBackup = ss.getBrowserState();
+
+let state = {entries:[
+  {
+    docIdentifier: 1,
+    url: "http://example.com?1",
+    children: [{ docIdentifier: 10,
+                 url: "http://example.com?10" }]
+  },
+  {
+    docIdentifier: 1,
+    url: "http://example.com?1#a",
+    children: [{ docIdentifier: 10,
+                 url: "http://example.com?10#aa" }]
+  }
+]};
+
+function test()
+{
+  registerCleanupFunction(function () {
+    ss.setBrowserState(stateBackup);
+  });
+
+  let tab = getBrowser().addTab("about:blank");
+  ss.setTabState(tab, JSON.stringify(state));
+  let history = tab.linkedBrowser.webNavigation.sessionHistory;
+
+  is(history.count, 2, "history.count");
+  for (let i = 0; i < history.count; i++) {
+    for (let j = 0; j < history.count; j++) {
+      compareEntries(i, j, history);
+    }
+  }
+}
+
+function compareEntries(i, j, history)
+{
+  let e1 = history.getEntryAtIndex(i, false)
+                  .QueryInterface(Components.interfaces.nsISHEntry)
+                  .QueryInterface(Components.interfaces.nsISHContainer);
+
+  let e2 = history.getEntryAtIndex(j, false)
+                  .QueryInterface(Components.interfaces.nsISHEntry)
+                  .QueryInterface(Components.interfaces.nsISHContainer);
+
+  ok(e1.sharesDocumentWith(e2),
+     i + ' should share doc with ' + j);
+  is(e1.childCount, e2.childCount,
+     'Child count mismatch (' + i + ', ' + j + ')');
+
+  for (let c = 0; c < e1.childCount; c++) {
+    let c1 = e1.GetChildAt(c);
+    let c2 = e2.GetChildAt(c);
+
+    ok(c1.sharesDocumentWith(c2),
+       'Cousins should share documents. (' + i + ', ' + j + ', ' + c + ')');
+  }
+}