Backed out changeset 2bd2a22267d6 (bug 1609475) for failing test_sessionhistory.html on a CLOSED TREE

2020-02-22 01:46:26 +02:00 · 2020-02-22 01:46:26 +02:00 · 3f1905ccdc
--- a/docshell/shistory/PSHistory.ipdl
+++ b/docshell/shistory/PSHistory.ipdl
@ -26,7 +26,7 @@ struct LoadSHEntryData

 union LoadSHEntryResult {
  nsresult;
-  LoadSHEntryData[];
+  LoadSHEntryData;
 };

 sync protocol PSHistory {
--- a/docshell/shistory/SHistoryChild.cpp
+++ b/docshell/shistory/SHistoryChild.cpp
@ -369,27 +369,23 @@ SHistoryChild::CreateEntry(nsISHEntry** aEntry) {
  return NS_OK;
 }

-nsresult SHistoryChild::LoadURI(nsTArray<LoadSHEntryData>& aLoadData) {
-  for (LoadSHEntryData& l : aLoadData) {
-     if (l.browsingContext().IsNullOrDiscarded()) {
-       continue;
+nsresult SHistoryChild::LoadURI(LoadSHEntryData& aLoadData) {
+  if (NS_WARN_IF(aLoadData.browsingContext().IsNullOrDiscarded())) {
+    return NS_ERROR_FAILURE;
  }

-    nsCOMPtr<nsIDocShell> docShell = l.browsingContext().get()->GetDocShell();
-    if (!docShell) {
-      continue;
-    }
+  nsCOMPtr<nsIDocShell> docShell =
+      aLoadData.browsingContext().get()->GetDocShell();
+  NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);

  RefPtr<SHEntryChild> entry;
-    if (l.shEntry()) {
-      entry = l.shEntry()->ToSHEntryChild();
+  if (aLoadData.shEntry()) {
+    entry = aLoadData.shEntry()->ToSHEntryChild();
  }

  // FIXME Should this be sent through IPC?
-    l.loadState()->SetSHEntry(entry);
-    docShell->LoadURI(l.loadState(), false);
-  }
-  return NS_OK;
+  aLoadData.loadState()->SetSHEntry(entry);
+  return docShell->LoadURI(aLoadData.loadState(), false);
 }

 }  // namespace dom
--- a/docshell/shistory/SHistoryChild.h
+++ b/docshell/shistory/SHistoryChild.h
@ -69,7 +69,7 @@ class SHistoryChild final : public PSHistoryChild,

  ~SHistoryChild() = default;

-  nsresult LoadURI(nsTArray<LoadSHEntryData>& aLoadData);
+  nsresult LoadURI(LoadSHEntryData& aLoadData);

  // Track all bfcache entries and evict on expiration.
  mozilla::UniquePtr<HistoryTracker> mHistoryTracker;
--- a/docshell/shistory/SHistoryParent.cpp
+++ b/docshell/shistory/SHistoryParent.cpp
@ -24,19 +24,13 @@ LegacySHistory::LegacySHistory(SHistoryParent* aSHistoryParent,
  aRootBC->SetSessionHistory(this);
 }

-static void FillInLoadResult(
-    nsresult aRv, const nsTArray<nsSHistory::LoadEntryResult>& aLoadResults,
+static void FillInLoadResult(nsresult aRv,
+                             const nsSHistory::LoadEntryResult& aLoadResult,
                             LoadSHEntryResult* aResult) {
  if (NS_SUCCEEDED(aRv)) {
-    nsTArray<LoadSHEntryData> data;
-    data.SetCapacity(aLoadResults.Length());
-    for (const nsSHistory::LoadEntryResult& l : aLoadResults) {
-      data.AppendElement(
-          LoadSHEntryData(static_cast<LegacySHEntry*>(l.mLoadState->SHEntry()),
-                          l.mBrowsingContext, l.mLoadState));
-    }
-
-    *aResult = data;
+    *aResult = LoadSHEntryData(
+        static_cast<LegacySHEntry*>(aLoadResult.mLoadState->SHEntry()),
+        aLoadResult.mBrowsingContext, aLoadResult.mLoadState);
  } else {
    *aResult = aRv;
  }
@ -101,10 +95,12 @@ bool SHistoryParent::RecvPurgeHistory(int32_t aNumEntries, nsresult* aResult) {
 }

 bool SHistoryParent::RecvReloadCurrentEntry(LoadSHEntryResult* aLoadResult) {
-  nsTArray<nsSHistory::LoadEntryResult> loadResults;
-  nsresult rv = mHistory->ReloadCurrentEntry(loadResults);
+  nsSHistory::LoadEntryResult loadResult;
+  nsresult rv = mHistory->ReloadCurrentEntry(loadResult);
  if (NS_SUCCEEDED(rv)) {
-    FillInLoadResult(rv, loadResults, aLoadResult);
+    *aLoadResult = LoadSHEntryData(
+        static_cast<LegacySHEntry*>(loadResult.mLoadState->SHEntry()),
+        loadResult.mBrowsingContext, loadResult.mLoadState);
  } else {
    *aLoadResult = rv;
  }
@ -113,9 +109,9 @@ bool SHistoryParent::RecvReloadCurrentEntry(LoadSHEntryResult* aLoadResult) {

 bool SHistoryParent::RecvGotoIndex(int32_t aIndex,
                                   LoadSHEntryResult* aLoadResult) {
-  nsTArray<nsSHistory::LoadEntryResult> loadResults;
-  nsresult rv = mHistory->GotoIndex(aIndex, loadResults);
-  FillInLoadResult(rv, loadResults, aLoadResult);
+  nsSHistory::LoadEntryResult loadResult;
+  nsresult rv = mHistory->GotoIndex(aIndex, loadResult);
+  FillInLoadResult(rv, loadResult, aLoadResult);
  return true;
 }

@ -197,12 +193,12 @@ bool SHistoryParent::RecvRemoveFrameEntries(PSHEntryParent* aEntry) {

 bool SHistoryParent::RecvReload(const uint32_t& aReloadFlags,
                                LoadSHEntryResult* aLoadResult) {
-  nsTArray<nsSHistory::LoadEntryResult> loadResults;
-  nsresult rv = mHistory->Reload(aReloadFlags, loadResults);
-  if (NS_SUCCEEDED(rv) && loadResults.IsEmpty()) {
+  Maybe<nsSHistory::LoadEntryResult> loadResult;
+  nsresult rv = mHistory->Reload(aReloadFlags, loadResult);
+  if (NS_SUCCEEDED(rv) && !loadResult) {
    *aLoadResult = NS_OK;
  } else {
-    FillInLoadResult(rv, loadResults, aLoadResult);
+    FillInLoadResult(rv, loadResult.ref(), aLoadResult);
  }
  return true;
 }
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@ -830,29 +830,27 @@ nsSHistory::EvictAllContentViewers() {
  return NS_OK;
 }

-static void LoadURIs(nsTArray<nsSHistory::LoadEntryResult>& aLoadResults) {
-  for (nsSHistory::LoadEntryResult& loadEntry : aLoadResults) {
-    loadEntry.mBrowsingContext->LoadURI(nullptr, loadEntry.mLoadState, false);
-  }
+static nsresult LoadURI(nsSHistory::LoadEntryResult& aLoadResult) {
+  return aLoadResult.mBrowsingContext->LoadURI(nullptr, aLoadResult.mLoadState,
+                                               false);
 }

 NS_IMETHODIMP
 nsSHistory::Reload(uint32_t aReloadFlags) {
-  nsTArray<LoadEntryResult> loadResults;
-  nsresult rv = Reload(aReloadFlags, loadResults);
+  Maybe<LoadEntryResult> loadResult;
+  nsresult rv = Reload(aReloadFlags, loadResult);
  NS_ENSURE_SUCCESS(rv, rv);

-  if (loadResults.IsEmpty()) {
+  if (!loadResult) {
    return NS_OK;
  }

-  LoadURIs(loadResults);
-  return NS_OK;
+  return LoadURI(loadResult.ref());
 }

 nsresult nsSHistory::Reload(uint32_t aReloadFlags,
-                            nsTArray<LoadEntryResult>& aLoadResults) {
-  MOZ_ASSERT(aLoadResults.IsEmpty());
+                            Maybe<LoadEntryResult>& aLoadResult) {
+  MOZ_ASSERT(!aLoadResult.isSome());

  uint32_t loadType;
  if (aReloadFlags & nsIWebNavigation::LOAD_FLAGS_BYPASS_PROXY &&
@ -880,9 +878,10 @@ nsresult nsSHistory::Reload(uint32_t aReloadFlags,
    return NS_OK;
  }

-  nsresult rv = LoadEntry(mIndex, loadType, HIST_CMD_RELOAD, aLoadResults);
+  aLoadResult.emplace();
+  nsresult rv = LoadEntry(mIndex, loadType, HIST_CMD_RELOAD, aLoadResult.ref());
  if (NS_FAILED(rv)) {
-    aLoadResults.Clear();
+    aLoadResult.reset();
    return rv;
  }

@ -891,20 +890,18 @@ nsresult nsSHistory::Reload(uint32_t aReloadFlags,

 NS_IMETHODIMP
 nsSHistory::ReloadCurrentEntry() {
-  nsTArray<LoadEntryResult> loadResults;
-  nsresult rv = ReloadCurrentEntry(loadResults);
+  LoadEntryResult loadResult;
+  nsresult rv = ReloadCurrentEntry(loadResult);
  NS_ENSURE_SUCCESS(rv, rv);

-  LoadURIs(loadResults);
-  return NS_OK;
+  return LoadURI(loadResult);
 }

-nsresult nsSHistory::ReloadCurrentEntry(
-    nsTArray<LoadEntryResult>& aLoadResults) {
+nsresult nsSHistory::ReloadCurrentEntry(LoadEntryResult& aLoadResult) {
  // Notify listeners
  NOTIFY_LISTENERS(OnHistoryGotoIndex, ());

-  return LoadEntry(mIndex, LOAD_HISTORY, HIST_CMD_RELOAD, aLoadResults);
+  return LoadEntry(mIndex, LOAD_HISTORY, HIST_CMD_RELOAD, aLoadResult);
 }

 void nsSHistory::EvictOutOfRangeWindowContentViewers(int32_t aIndex) {
@ -1385,12 +1382,11 @@ nsSHistory::UpdateIndex() {

 NS_IMETHODIMP
 nsSHistory::GotoIndex(int32_t aIndex) {
-  nsTArray<LoadEntryResult> loadResults;
-  nsresult rv = GotoIndex(aIndex, loadResults);
+  LoadEntryResult loadResult;
+  nsresult rv = GotoIndex(aIndex, loadResult);
  NS_ENSURE_SUCCESS(rv, rv);

-  LoadURIs(loadResults);
-  return NS_OK;
+  return LoadURI(loadResult);
 }

 NS_IMETHODIMP_(void)
@ -1401,27 +1397,26 @@ nsSHistory::EnsureCorrectEntryAtCurrIndex(nsISHEntry* aEntry) {
  }
 }

-nsresult nsSHistory::GotoIndex(int32_t aIndex,
-                               nsTArray<LoadEntryResult>& aLoadResults) {
-  return LoadEntry(aIndex, LOAD_HISTORY, HIST_CMD_GOTOINDEX, aLoadResults);
+nsresult nsSHistory::GotoIndex(int32_t aIndex, LoadEntryResult& aLoadResult) {
+  return LoadEntry(aIndex, LOAD_HISTORY, HIST_CMD_GOTOINDEX, aLoadResult);
 }

-nsresult nsSHistory::LoadNextPossibleEntry(
-    int32_t aNewIndex, long aLoadType, uint32_t aHistCmd,
-    nsTArray<LoadEntryResult>& aLoadResults) {
+nsresult nsSHistory::LoadNextPossibleEntry(int32_t aNewIndex, long aLoadType,
+                                           uint32_t aHistCmd,
+                                           LoadEntryResult& aLoadResult) {
  mRequestedIndex = -1;
  if (aNewIndex < mIndex) {
-    return LoadEntry(aNewIndex - 1, aLoadType, aHistCmd, aLoadResults);
+    return LoadEntry(aNewIndex - 1, aLoadType, aHistCmd, aLoadResult);
  }
  if (aNewIndex > mIndex) {
-    return LoadEntry(aNewIndex + 1, aLoadType, aHistCmd, aLoadResults);
+    return LoadEntry(aNewIndex + 1, aLoadType, aHistCmd, aLoadResult);
  }
  return NS_ERROR_FAILURE;
 }

 nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,
                               uint32_t aHistCmd,
-                               nsTArray<LoadEntryResult>& aLoadResults) {
+                               LoadEntryResult& aLoadResult) {
  if (!mRootBC) {
    return NS_ERROR_FAILURE;
  }
@ -1462,16 +1457,16 @@ nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,

  if (mRequestedIndex == mIndex) {
    // Possibly a reload case
-    return InitiateLoad(nextEntry, mRootBC, aLoadType, aLoadResults);
+    return InitiateLoad(nextEntry, mRootBC, aLoadType, aLoadResult);
  }

  // Going back or forward.
  bool differenceFound = false;
  nsresult rv = LoadDifferingEntries(prevEntry, nextEntry, mRootBC, aLoadType,
-                                     differenceFound, aLoadResults);
+                                     differenceFound, aLoadResult);
  if (!differenceFound) {
    // We did not find any differences. Go further in the history.
-    return LoadNextPossibleEntry(aIndex, aLoadType, aHistCmd, aLoadResults);
+    return LoadNextPossibleEntry(aIndex, aLoadType, aHistCmd, aLoadResult);
  }

  return rv;
@ -1479,8 +1474,7 @@ nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,

 nsresult nsSHistory::LoadDifferingEntries(
    nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry, BrowsingContext* aParent,
-    long aLoadType, bool& aDifferenceFound,
-    nsTArray<LoadEntryResult>& aLoadResults) {
+    long aLoadType, bool& aDifferenceFound, LoadEntryResult& aLoadResult) {
  if (!aPrevEntry || !aNextEntry || !aParent) {
    return NS_ERROR_FAILURE;
  }
@ -1495,7 +1489,7 @@ nsresult nsSHistory::LoadDifferingEntries(

    // Set the Subframe flag if not navigating the root docshell.
    aNextEntry->SetIsSubFrame(aParent != mRootBC);
-    return InitiateLoad(aNextEntry, aParent, aLoadType, aLoadResults);
+    return InitiateLoad(aNextEntry, aParent, aLoadType, aLoadResult);
  }

  // The entries are the same, so compare any child frames
@ -1549,18 +1543,17 @@ nsresult nsSHistory::LoadDifferingEntries(
    // This will either load a new page to shell or some subshell or
    // do nothing.
    LoadDifferingEntries(pChild, nChild, bcChild, aLoadType, aDifferenceFound,
-                         aLoadResults);
+                         aLoadResult);
  }
  return result;
 }

 nsresult nsSHistory::InitiateLoad(nsISHEntry* aFrameEntry,
                                  BrowsingContext* aFrameBC, long aLoadType,
-                                  nsTArray<LoadEntryResult>& aLoadResults) {
+                                  LoadEntryResult& aLoadResult) {
  NS_ENSURE_STATE(aFrameBC && aFrameEntry);

-  LoadEntryResult* loadResult = aLoadResults.AppendElement();
-  loadResult->mBrowsingContext = aFrameBC;
+  aLoadResult.mBrowsingContext = aFrameBC;

  nsCOMPtr<nsIURI> newURI = aFrameEntry->GetURI();
  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(newURI);
@ -1587,7 +1580,7 @@ nsresult nsSHistory::InitiateLoad(nsISHEntry* aFrameEntry,
  nsCOMPtr<nsIContentSecurityPolicy> csp = aFrameEntry->GetCsp();
  loadState->SetCsp(csp);

-  loadResult->mLoadState = std::move(loadState);
+  aLoadResult.mLoadState = std::move(loadState);

  return NS_OK;
 }
--- a/docshell/shistory/nsSHistory.h
+++ b/docshell/shistory/nsSHistory.h
@ -140,10 +140,9 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,
  // If this doesn't return an error then either aLoadResult is set to nothing,
  // in which case the caller should ignore the load, or it returns a valid
  // LoadEntryResult in aLoadResult which the caller should use to do the load.
-  nsresult Reload(uint32_t aReloadFlags,
-                  nsTArray<LoadEntryResult>& aLoadResults);
-  nsresult ReloadCurrentEntry(nsTArray<LoadEntryResult>& aLoadResults);
-  nsresult GotoIndex(int32_t aIndex, nsTArray<LoadEntryResult>& aLoadResults);
+  nsresult Reload(uint32_t aReloadFlags, Maybe<LoadEntryResult>& aLoadResult);
+  nsresult ReloadCurrentEntry(LoadEntryResult& aLoadResult);
+  nsresult GotoIndex(int32_t aIndex, LoadEntryResult& aLoadResult);

  void WindowIndices(int32_t aIndex, int32_t* aOutStartIndex,
                     int32_t* aOutEndIndex);
@ -161,13 +160,13 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,
  nsresult LoadDifferingEntries(nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry,
                                mozilla::dom::BrowsingContext* aRootBC,
                                long aLoadType, bool& aDifferenceFound,
-                                nsTArray<LoadEntryResult>& aLoadResults);
+                                LoadEntryResult& aLoadResult);
  nsresult InitiateLoad(nsISHEntry* aFrameEntry,
                        mozilla::dom::BrowsingContext* aFrameBC, long aLoadType,
-                        nsTArray<LoadEntryResult>& aLoadResult);
+                        LoadEntryResult& aLoadResult);

  nsresult LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd,
-                     nsTArray<LoadEntryResult>& aLoadResults);
+                     LoadEntryResult& aLoad);

 #ifdef DEBUG
  nsresult PrintHistory();
@ -191,7 +190,7 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,

  nsresult LoadNextPossibleEntry(int32_t aNewIndex, long aLoadType,
                                 uint32_t aHistCmd,
-                                 nsTArray<LoadEntryResult>& aLoadResults);
+                                 LoadEntryResult& aLoadResult);

  // aIndex is the index of the entry which may be removed.
  // If aKeepNext is true, aIndex is compared to aIndex + 1,
--- a/docshell/test/navigation/file_bug1609475.html
+++ b/docshell/test/navigation/file_bug1609475.html
@ -1,49 +0,0 @@
-<html>
-  <head>
-    <script>
-
-    var loadCount = 0;
-    function loadListener(event) {
-      ++loadCount;
-      if (loadCount == 2) {
-        // Use a timer to ensure we don't get extra load events.
-        setTimeout(function() {
-          opener.ok(document.getElementById("i1").contentDocument.documentURI.includes("frame1.html"),
-                    "Should have loaded the initial page to the first iframe");
-          opener.ok(document.getElementById("i2").contentDocument.documentURI.includes("frame1.html"),
-                    "Should have loaded the initial page to the second iframe");
-          opener.nextTest();
-          window.close();
-        }, 1000);
-      } else if (loadCount > 2) {
-        opener.ok(false, "Too many load events");
-      }
-      // if we don't get enough load events, the test will time out.
-    }
-
-    function setupIframe(id) {
-      var ifr = document.getElementById(id);
-      return new Promise(function(resolve) {
-        ifr.onload = function() {
-          // Replace load listener to catch page loads from the session history.
-          ifr.onload = loadListener();
-          // Need to use setTimeout, because triggering loads inside
-          // load event listener has special behavior.
-          setTimeout(resolve);
-        }
-        ifr.contentWindow.location.href = "frame2.html";
-      });
-    }
-
-    async function test() {
-      await setupIframe("i1");
-      await setupIframe("i2");
-      history.go(-2);
-    }
-    </script>
-  </head>
-  <body onload="setTimeout(test)">
-    <iframe id="i1" src="frame1.html"></iframe>
-    <iframe id="i2" src="frame1.html"></iframe>
-  </body>
-</html>
--- a/docshell/test/navigation/mochitest.ini
+++ b/docshell/test/navigation/mochitest.ini
@ -57,7 +57,6 @@ support-files =
  file_bug1375833-frame1.html
  file_bug1375833-frame2.html
  test_bug145971.html
-  file_bug1609475.html

 [test_bug13871.html]
 [test_bug270414.html]
--- a/docshell/test/navigation/test_sessionhistory.html
+++ b/docshell/test/navigation/test_sessionhistory.html
@ -34,7 +34,6 @@ var testFiles =
    "file_bug1300461.html",
    "file_bug1326251.html",
    "file_bug1379762-1.html",
-    "file_bug1609475.html",
  ];
 var testCount = 0; // Used by the test files.