Backed out changeset 1968ba484b6f (bug 1697905) on dev's request

docshell/base/CanonicalBrowsingContext.cpp

@@ -740,12 +740,12 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
           }
         }
 
-        if (!loadFromSessionHistory && addEntry) {
+        if (loadFromSessionHistory) {
+          // XXX Synchronize browsing context tree and session history tree?
+          shistory->UpdateIndex();
+        } else if (addEntry) {
           shistory->AddEntry(mActiveEntry, aPersist);
         }
-        // XXX Synchronize browsing context tree and session history tree?
-        // UpdateIndexWithEntry updates the index and clears the requestedIndex.
-        shistory->UpdateIndexWithEntry(mActiveEntry);
       } else {
         // FIXME The old implementations adds it to the parent's mLSHE if there
         //       is one, need to figure out if that makes sense here (peterv
@@ -759,6 +759,9 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
                                                          this);
           }
           mActiveEntry = newActiveEntry;
+          // FIXME UpdateIndex() here may update index too early (but even the
+          //       old implementation seems to have similar issues).
+          shistory->UpdateIndex();
         } else if (addEntry) {
           if (mActiveEntry) {
             if (LOAD_TYPE_HAS_FLAGS(
@@ -788,10 +791,6 @@ void CanonicalBrowsingContext::SessionHistoryCommit(uint64_t aLoadId,
             }
           }
         }
-        // FIXME UpdateIndex() here may update index too early (but even the
-        //       old implementation seems to have similar issues).
-        // UpdateIndexWithEntry updates the index and clears the requestedIndex.
-        shistory->UpdateIndexWithEntry(mActiveEntry);
       }
 
       ResetSHEntryHasUserInteractionCache();
@@ -2335,7 +2334,8 @@ bool CanonicalBrowsingContext::AllowedInBFCache(
   uint16_t bfcacheCombo = 0;
   if (mRestoreState) {
     bfcacheCombo |= BFCacheStatus::RESTORING;
-    MOZ_LOG(gSHIPBFCacheLog, LogLevel::Debug, (" * during session restore"));
+    MOZ_LOG(gSHIPBFCacheLog, LogLevel::Debug,
+            (" * during session restore"));
   }
 
   if (Group()->Toplevels().Length() > 1) {

docshell/shistory/nsISHistory.idl

@@ -164,16 +164,10 @@ interface nsISHistory: nsISupports
   void addEntry(in nsISHEntry aEntry, in boolean aPersist);
 
   /**
-   * Update the index maintained by sessionHistory to the value of
-   * the requested index.
+   * Update the index maintained by sessionHistory
    */
   void updateIndex();
 
-  /**
-   * Update the index based on the index of the root item of aEntry.
-   */
-  void updateIndexWithEntry(in nsISHEntry aEntry);
-
   /**
    * Replace the nsISHEntry at a particular index
    *

docshell/shistory/nsSHistory.cpp

@@ -1214,7 +1214,7 @@ static void FinishRestore(CanonicalBrowsingContext* aBrowsingContext,
 
     // Assuming we still have the session history, update the index.
     if (loadingBC->GetSessionHistory()) {
-      loadingBC->GetSessionHistory()->UpdateIndexWithEntry(aEntry);
+      loadingBC->GetSessionHistory()->UpdateIndex();
     }
     loadingBC->HistoryCommitIndexAndLength();
     Unused << loadingBC->SetIsInBFCache(false);
@@ -1915,19 +1915,6 @@ nsSHistory::UpdateIndex() {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsSHistory::UpdateIndexWithEntry(nsISHEntry* aEntry) {
-  nsCOMPtr<nsISHEntry> root = nsSHistory::GetRootSHEntry(aEntry);
-  if (root) {
-    int32_t index = GetIndexOfEntry(root);
-    if (index >= 0) {
-      mRequestedIndex = index;
-      UpdateIndex();
-    }
-  }
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsSHistory::GotoIndex(int32_t aIndex, bool aUserActivation) {
   nsTArray<LoadEntryResult> loadResults;

docshell/test/navigation/browser.ini

@@ -6,12 +6,9 @@ support-files =
   bug343515_pg3_1.html
   bug343515_pg3_1_1.html
   bug343515_pg3_2.html
-  blank.html
 
 [browser_test_bfcache_eviction.js]
 [browser_bug343515.js]
 [browser_test-content-chromeflags.js]
 tags = openwindow
 [browser_ghistorymaxsize_is_0.js]
-[browser_test_simultaneous_normal_and_history_loads.js]
-skip-if = !sessionHistoryInParent # The test is for the new session history

docshell/test/navigation/browser_test_simultaneous_normal_and_history_loads.js

@@ -1,52 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-add_task(async function test_normal_and_history_loads() {
-  // This test is for the case when session history lives in the parent process.
-  // BFCache is disabled to get more asynchronousness.
-  await SpecialPowers.pushPrefEnv({
-    set: [["fission.bfcacheInParent", false]],
-  });
-
-  let testPage =
-    getRootDirectory(gTestPath).replace(
-      "chrome://mochitests/content",
-      "http://example.com"
-    ) + "blank.html";
-  await BrowserTestUtils.withNewTab({ gBrowser, url: testPage }, async function(
-    browser
-  ) {
-    for (let i = 0; i < 2; ++i) {
-      BrowserTestUtils.loadURI(browser, testPage + "?" + i);
-      await BrowserTestUtils.browserLoaded(browser);
-    }
-
-    let sh = browser.browsingContext.sessionHistory;
-    is(sh.count, 3, "Should have 3 entries in the session history.");
-    is(sh.index, 2, "index should be 2");
-    is(sh.requestedIndex, -1, "requestedIndex should be -1");
-
-    // The following part is racy by definition. It is testing that
-    // eventually requestedIndex should become -1 again.
-    browser.browsingContext.goToIndex(1);
-    let historyLoad = BrowserTestUtils.browserLoaded(browser);
-    /* eslint-disable mozilla/no-arbitrary-setTimeout */
-    await new Promise(r => {
-      setTimeout(r, 10);
-    });
-    browser.browsingContext.loadURI(testPage + "?newload", {
-      triggeringPrincipal: browser.nodePrincipal,
-    });
-    let newLoad = BrowserTestUtils.browserLoaded(browser);
-    // Note, the loads are racy.
-    await historyLoad;
-    await newLoad;
-    is(sh.requestedIndex, -1, "requestedIndex should be -1");
-
-    browser.browsingContext.goBack();
-    await BrowserTestUtils.browserLoaded(browser);
-    is(sh.requestedIndex, -1, "requestedIndex should be -1");
-  });
-});