Bug 1373672 - Part 3: Expose childOffset from nsIDocShell to use in nsSessionStoreUtils, r=ttaubert, r=smaug
☠☠ backed out by 896709cd1daf ☠ ☠
authorMichael Layzell <michael@thelayzells.com>
Tue, 01 Aug 2017 11:22:53 +0200
changeset 420910 36bb09c4b28edaefacb7199cd1d73e010753ecb9
parent 420909 dc318b0913b97dba5f8c8ec2735f96b7d5a30d69
child 420911 f2cf93fdcb4c1662470b75275b2bf36943ffe264
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert, smaug
bugs1373672
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1373672 - Part 3: Expose childOffset from nsIDocShell to use in nsSessionStoreUtils, r=ttaubert, r=smaug The reasoning behind this is that with this change, removing a non-dynamic docshell from the document dynamically shouldn't affect the indexes which we use for both recording and restoring data in child docshells. MozReview-Commit-ID: JIK8GBSWDEF
browser/components/sessionstore/nsSessionStoreUtils.cpp
browser/components/sessionstore/test/browser_frametree.js
docshell/base/nsDocShell.cpp
docshell/base/nsIDocShell.idl
--- a/browser/components/sessionstore/nsSessionStoreUtils.cpp
+++ b/browser/components/sessionstore/nsSessionStoreUtils.cpp
@@ -84,31 +84,36 @@ nsSessionStoreUtils::ForEachNonDynamicCh
 
   nsCOMPtr<nsIDocShell> docShell = outer->GetDocShell();
   NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);
 
   int32_t length;
   nsresult rv = docShell->GetChildCount(&length);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  for (int32_t i = 0, idx = 0; i < length; ++i) {
+  for (int32_t i = 0; i < length; ++i) {
     nsCOMPtr<nsIDocShellTreeItem> item;
     docShell->GetChildAt(i, getter_AddRefs(item));
     NS_ENSURE_TRUE(item, NS_ERROR_FAILURE);
 
     nsCOMPtr<nsIDocShell> childDocShell(do_QueryInterface(item));
     NS_ENSURE_TRUE(childDocShell, NS_ERROR_FAILURE);
 
     bool isDynamic = false;
     nsresult rv = childDocShell->GetCreatedDynamically(&isDynamic);
     if (NS_SUCCEEDED(rv) && isDynamic) {
       continue;
     }
 
-    aCallback->HandleFrame(item->GetWindow(), idx++);
+    int32_t childOffset = 0;
+    rv = childDocShell->GetChildOffset(&childOffset);
+    NS_ENSURE_SUCCESS(rv, rv);
+    MOZ_ASSERT(childOffset > 0);
+
+    aCallback->HandleFrame(item->GetWindow(), childOffset);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSessionStoreUtils::CreateDynamicFrameEventFilter(nsIDOMEventListener* aListener,
                                                    nsIDOMEventListener** aResult)
--- a/browser/components/sessionstore/test/browser_frametree.js
+++ b/browser/components/sessionstore/test/browser_frametree.js
@@ -83,16 +83,25 @@ add_task(async function test_frametree_d
     content.document.body.appendChild(frame);
     return ContentTaskUtils.waitForEvent(frame, "load");
   });
 
   // The page still has two iframes.
   is(await countNonDynamicFrames(browser), 2, "two non-dynamic child frames");
   is(await enumerateIndexes(browser), "0,1", "correct indexes 0 and 1");
 
+  // Remopve a non-dynamic iframe.
+  await ContentTask.spawn(browser, URL, async ([url]) => {
+    // Remove the first iframe, which should be a non-dynamic iframe.
+    content.document.body.removeChild(content.document.getElementsByTagName("iframe")[0]);
+  });
+
+  is(await countNonDynamicFrames(browser), 1, "one non-dynamic child frame");
+  is(await enumerateIndexes(browser), "1", "correct index 1");
+
   // Cleanup.
   await promiseRemoveTab(tab);
 });
 
 async function countNonDynamicFrames(browser) {
   return await ContentTask.spawn(browser, null, async () => {
     const ssu = Cc["@mozilla.org/browser/sessionstore/utils;1"]
                   .getService(Ci.nsISessionStoreUtils);
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -4085,16 +4085,23 @@ nsDocShell::SetTreeOwner(nsIDocShellTree
 NS_IMETHODIMP
 nsDocShell::SetChildOffset(int32_t aChildOffset)
 {
   mChildOffset = aChildOffset;
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsDocShell::GetChildOffset(int32_t* aChildOffset)
+{
+  *aChildOffset = mChildOffset;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsDocShell::GetHistoryID(nsID** aID)
 {
   *aID = static_cast<nsID*>(nsMemory::Clone(&mHistoryID, sizeof(nsID)));
   return NS_OK;
 }
 
 const nsID
 nsDocShell::HistoryID()
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -544,19 +544,20 @@ interface nsIDocShell : nsIDocShellTreeI
   /**
    * Gets the channel for the currently loaded document, if any.
    * For a new document load, this will be the channel of the previous document
    * until after OnLocationChange fires.
    */
   readonly attribute nsIChannel currentDocumentChannel;
 
   /**
-   * Set the offset of this child in its container.
+   * The original offset of this child in its container. This property is -1 for
+   * dynamically added docShells.
    */
-  [noscript] void setChildOffset(in long offset);
+  [noscript] attribute long childOffset;
 
   /**
    * Find out whether the docshell is currently in the middle of a page
    * transition. This is set just before the pagehide/unload events fire.
    */
   readonly attribute boolean isInUnload;
 
   /**