Bug 1090918 - Multiple History items when visiting links and using 'B…

…ack', r=bz --HG-- extra : rebase_source : ce5b34fff505a4dc0d98b8cc384e84d33bc0aed1
dothq · Nov 13, 2014 · 65ad793 · 65ad793
1 parent a56c861
commit 65ad793
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 14 deletions.
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -4243,15 +4243,17 @@ nsDocShell::AddChildSHEntry(nsISHEntry * aCloneRef, nsISHEntry * aNewEntry,
                             int32_t aChildOffset, uint32_t loadType,
                             bool aCloneChildren)
 {
-    nsresult rv;
+    nsresult rv = NS_OK;
 
-    if (mLSHE && loadType != LOAD_PUSHSTATE && !aCloneRef) {
+    if (mLSHE && loadType != LOAD_PUSHSTATE) {
         /* You get here if you are currently building a 
          * hierarchy ie.,you just visited a frameset page
          */
         nsCOMPtr<nsISHContainer> container(do_QueryInterface(mLSHE, &rv));
         if (container) {
-            rv = container->AddChild(aNewEntry, aChildOffset);
+            if (NS_FAILED(container->ReplaceChild(aNewEntry))) {
+                rv = container->AddChild(aNewEntry, aChildOffset);
+            }
         }
     }
     else if (!aCloneRef) {
@@ -4260,8 +4262,22 @@ nsDocShell::AddChildSHEntry(nsISHEntry * aCloneRef, nsISHEntry * aNewEntry,
         if (container) {
             rv = container->AddChild(aNewEntry, aChildOffset);
         }
+    } else {
+        rv = AddChildSHEntryInternal(aCloneRef, aNewEntry, aChildOffset,
+                                     loadType, aCloneChildren);
     }
-    else if (mSessionHistory) {
+    return rv;
+}
+
+nsresult
+nsDocShell::AddChildSHEntryInternal(nsISHEntry* aCloneRef,
+                                    nsISHEntry* aNewEntry,
+                                    int32_t aChildOffset,
+                                    uint32_t aLoadType,
+                                    bool aCloneChildren)
+{
+    nsresult rv = NS_OK;
+    if (mSessionHistory) {
         /* You are currently in the rootDocShell.
          * You will get here when a subframe has a new url
          * to load and you have walked up the tree all the 
@@ -4300,16 +4316,17 @@ nsDocShell::AddChildSHEntry(nsISHEntry * aCloneRef, nsISHEntry * aNewEntry,
         nsCOMPtr<nsIDocShell> parent =
             do_QueryInterface(GetAsSupports(mParent), &rv);
         if (parent) {
-            rv = parent->AddChildSHEntry(aCloneRef, aNewEntry, aChildOffset,
-                                         loadType, aCloneChildren);
-        }          
+            rv = static_cast<nsDocShell*>(parent.get())->
+              AddChildSHEntryInternal(aCloneRef, aNewEntry, aChildOffset,
+                                      aLoadType, aCloneChildren);
+        }
     }
     return rv;
 }
 
 nsresult
-nsDocShell::DoAddChildSHEntry(nsISHEntry* aNewEntry, int32_t aChildOffset,
-                              bool aCloneChildren)
+nsDocShell::AddChildSHEntryToParent(nsISHEntry* aNewEntry, int32_t aChildOffset,
+                                    bool aCloneChildren)
 {
     /* You will get here when you are in a subframe and
      * a new url has been loaded on you. 
@@ -11653,7 +11670,7 @@ nsDocShell::AddToSessionHistory(nsIURI * aURI, nsIChannel * aChannel,
         // This is a subframe.
         if (!mOSHE || !LOAD_TYPE_HAS_FLAGS(mLoadType,
                                            LOAD_FLAGS_REPLACE_HISTORY))
-            rv = DoAddChildSHEntry(entry, mChildOffset, aCloneChildren);
+            rv = AddChildSHEntryToParent(entry, mChildOffset, aCloneChildren);
     }
 
     // Return the new SH entry...

diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h
@@ -371,8 +371,12 @@ class nsDocShell MOZ_FINAL : public nsDocLoader,
                                          nsISupports* aOwner,
                                          bool aCloneChildren,
                                          nsISHEntry ** aNewEntry);
-    nsresult DoAddChildSHEntry(nsISHEntry* aNewEntry, int32_t aChildOffset,
-                               bool aCloneChildren);
+    nsresult AddChildSHEntryToParent(nsISHEntry* aNewEntry, int32_t aChildOffset,
+                                     bool aCloneChildren);
+
+    nsresult AddChildSHEntryInternal(nsISHEntry* aCloneRef, nsISHEntry* aNewEntry,
+                                     int32_t aChildOffset, uint32_t loadType,
+                                     bool aCloneChildren);
 
     NS_IMETHOD LoadHistoryEntry(nsISHEntry * aEntry, uint32_t aLoadType);
     NS_IMETHOD PersistLayoutHistoryState();

diff --git a/docshell/shistory/public/nsISHContainer.idl b/docshell/shistory/public/nsISHContainer.idl
@@ -14,7 +14,7 @@ interface nsISHEntry;
  * 
  */
 
-[scriptable, uuid(65281BA2-988A-11d3-BDC7-0050040A9B44)]
+[scriptable, uuid(67dd0357-8372-4122-bff6-217435e8b7e4)]
 interface nsISHContainer : nsISupports
 {
 	/**
@@ -38,5 +38,12 @@ interface nsISHContainer : nsISupports
 	 */
 	nsISHEntry GetChildAt(in long index);
 
+	/**
+	 * Replaces a child which is for the same docshell as aNewChild
+	 * with aNewChild.
+	 * @throw if nothing was replaced.
+	 */
+	void ReplaceChild(in nsISHEntry aNewChild);
+
 };
 
diff --git a/docshell/shistory/src/nsSHEntry.cpp b/docshell/shistory/src/nsSHEntry.cpp
@@ -690,6 +690,27 @@ nsSHEntry::GetChildAt(int32_t aIndex, nsISHEntry ** aResult)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsSHEntry::ReplaceChild(nsISHEntry* aNewEntry)
+{
+  NS_ENSURE_STATE(aNewEntry);
+
+  uint64_t docshellID;
+  aNewEntry->GetDocshellID(&docshellID);
+
+  uint64_t otherID;
+  for (int32_t i = 0; i < mChildren.Count(); ++i) {
+    if (mChildren[i] && NS_SUCCEEDED(mChildren[i]->GetDocshellID(&otherID)) &&
+        docshellID == otherID) {
+      mChildren[i]->SetParent(nullptr);
+      if (mChildren.ReplaceObjectAt(aNewEntry, i)) {
+        return aNewEntry->SetParent(this);
+      }
+    }
+  }
+  return NS_ERROR_FAILURE;
+}
+
 NS_IMETHODIMP
 nsSHEntry::AddChildShell(nsIDocShellTreeItem *aShell)
 {

diff --git a/docshell/test/navigation/file_nested_frames.html b/docshell/test/navigation/file_nested_frames.html
@@ -0,0 +1,28 @@
+<html>
+  <head>
+    <script>
+      function nestedIframeLoaded() {
+        var tf = document.getElementById("testframe");
+        var innerf = tf.contentDocument.getElementsByTagName("iframe")[0];
+        if (innerf.contentDocument.documentURI.indexOf("frame0") < 0) {
+          innerf.contentWindow.location.href = "http://mochi.test:8888/tests/docshell/test/navigation/frame0.html";
+          return;
+        }
+        innerf.onload = null;
+        innerf.src = "about:blank";
+        var d = innerf.contentDocument;
+        d.open();
+        d.write("test");
+        d.close();
+        opener.is(window.history.length, 1, "Unexpected history length");
+        opener.nextTest();
+        window.close();
+      }
+    </script>
+  </head>
+  <body>
+  <iframe id="testframe" src="data:text/html,<iframe onload='parent.nestedIframeLoaded();'></iframe>" onload="frameLoaded()"></iframe>
+  <script>
+  </script>
+  </body>
+</html>
diff --git a/docshell/test/navigation/mochitest.ini b/docshell/test/navigation/mochitest.ini
@@ -9,6 +9,7 @@ support-files =
   file_bug534178.html
   file_document_write_1.html
   file_fragment_handling_during_load.html
+  file_nested_frames.html
   file_static_and_dynamic_1.html
   frame0.html
   frame1.html

diff --git a/docshell/test/navigation/test_sessionhistory.html b/docshell/test/navigation/test_sessionhistory.html
@@ -27,7 +27,8 @@
     "file_document_write_1.html",    // Session history + document.write
     //"file_static_and_dynamic_1.html",// Static and dynamic frames and forward-back
     "file_bug534178.html",           // Session history transaction clean-up.
-    "file_fragment_handling_during_load.html"
+    "file_fragment_handling_during_load.html",
+    "file_nested_frames.html"
   ];
 var testCount = 0; // Used by the test files.