Backed out changeset 6b39ac73fb99 (bug 1804803) for causing build bustages CLOSED TREE

2022-12-21 14:37:39 +02:00 · 2022-12-21 14:37:39 +02:00 · acaf348a8b
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@ -560,14 +560,18 @@ void CanonicalBrowsingContext::GetLoadingSessionHistoryInfoFromParent(

 UniquePtr<LoadingSessionHistoryInfo>
 CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad(
-    nsDocShellLoadState* aLoadState, SessionHistoryEntry* existingEntry,
-    nsIChannel* aChannel) {
+    nsDocShellLoadState* aLoadState, nsIChannel* aChannel) {
  RefPtr<SessionHistoryEntry> entry;
  const LoadingSessionHistoryInfo* existingLoadingInfo =
      aLoadState->GetLoadingSessionHistoryInfo();
-  MOZ_ASSERT(!existingEntry == !existingLoadingInfo);
  if (existingLoadingInfo) {
-    entry = existingEntry;
+    entry = SessionHistoryEntry::GetByLoadId(existingLoadingInfo->mLoadId);
+    MOZ_LOG(gSHLog, LogLevel::Verbose,
+            ("SHEntry::GetByLoadId(%" PRIu64 ") -> %p",
+             existingLoadingInfo->mLoadId, entry.get()));
+    if (!entry) {
+      return nullptr;
+    }

    // If the entry was updated, update also the LoadingSessionHistoryInfo.
    UniquePtr<LoadingSessionHistoryInfo> lshi =
@ -604,8 +608,7 @@ CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad(
        LoadingSessionHistoryEntry{loadingInfo->mLoadId, entry});
  }

-  MOZ_ASSERT(SessionHistoryEntry::GetByLoadId(loadingInfo->mLoadId)->mEntry ==
-             entry);
+  MOZ_ASSERT(SessionHistoryEntry::GetByLoadId(loadingInfo->mLoadId) == entry);

  return loadingInfo;
 }
@ -986,15 +989,13 @@ void CanonicalBrowsingContext::SessionHistoryCommit(
  // aSessionHistoryEntryId?
 }

-already_AddRefed<nsDocShellLoadState> CanonicalBrowsingContext::CreateLoadInfo(
+static already_AddRefed<nsDocShellLoadState> CreateLoadInfo(
    SessionHistoryEntry* aEntry) {
  const SessionHistoryInfo& info = aEntry->Info();
  RefPtr<nsDocShellLoadState> loadState(new nsDocShellLoadState(info.GetURI()));
  info.FillLoadInfo(*loadState);
  UniquePtr<LoadingSessionHistoryInfo> loadingInfo;
  loadingInfo = MakeUnique<LoadingSessionHistoryInfo>(aEntry);
-  mLoadingEntries.AppendElement(
-      LoadingSessionHistoryEntry{loadingInfo->mLoadId, aEntry});
  loadState->SetLoadingSessionHistoryInfo(std::move(loadingInfo));

  return loadState.forget();
@ -1027,10 +1028,10 @@ void CanonicalBrowsingContext::NotifyOnHistoryReload(
    aLoadState.emplace(CreateLoadInfo(loadingEntry.mEntry));
    aReloadActiveEntry.emplace(false);
    if (aForceReload) {
-      SessionHistoryEntry::LoadingEntry* entry =
+      SessionHistoryEntry* entry =
          SessionHistoryEntry::GetByLoadId(loadingEntry.mLoadId);
      if (entry) {
-        shistory->RemoveFrameEntries(entry->mEntry);
+        shistory->RemoveFrameEntries(entry);
      }
    }
  }
--- a/docshell/base/CanonicalBrowsingContext.h
+++ b/docshell/base/CanonicalBrowsingContext.h
@ -128,8 +128,7 @@ class CanonicalBrowsingContext final : public BrowsingContext {
  void SetActiveSessionHistoryEntry(SessionHistoryEntry* aEntry);

  UniquePtr<LoadingSessionHistoryInfo> CreateLoadingSessionHistoryEntryForLoad(
-      nsDocShellLoadState* aLoadState, SessionHistoryEntry* aExistingEntry,
-      nsIChannel* aChannel);
+      nsDocShellLoadState* aLoadState, nsIChannel* aChannel);

  UniquePtr<LoadingSessionHistoryInfo> ReplaceLoadingSessionHistoryEntryForLoad(
      LoadingSessionHistoryInfo* aInfo, nsIChannel* aNewChannel);
@ -485,9 +484,6 @@ class CanonicalBrowsingContext final : public BrowsingContext {
                                                     aHasPostData);
  }

-  already_AddRefed<nsDocShellLoadState> CreateLoadInfo(
-      SessionHistoryEntry* aEntry);
-
  // XXX(farre): Store a ContentParent pointer here rather than mProcessId?
  // Indicates which process owns the docshell.
  uint64_t mProcessId;
--- a/docshell/base/crashtests/1804803.html
+++ b/docshell/base/crashtests/1804803.html
@ -1,13 +0,0 @@
-<html class="reftest-wait">
-<script>
-function test() {
-  // If the reload in the iframe succeeds we might crash, so wait for it.
-  setTimeout(function() {
-    document.documentElement.className = "";
-  }, 500);
-}
-</script>
-<body onload="test()">
-    <iframe src="1804803.sjs"></iframe>
-</body>
-</html>
--- a/docshell/base/crashtests/1804803.sjs
+++ b/docshell/base/crashtests/1804803.sjs
@ -1,18 +0,0 @@
-function handleRequest(request, response) {
-  let counter = Number(getState("load"));
-  const reload = counter == 0 ? "self.history.go(0);" : "";
-  setState("load", String(++counter));
-  const document = `
-<script>
-    document.addEventListener('DOMContentLoaded', () => {
-      ${reload}
-      let url = window.location.href;
-      for (let i = 0; i < 50; i++) {
-        self.history.pushState({x: i}, '', url + "#" + i);
-      }
-    });
-</script>
-`;
-
-  response.write(document);
-}
--- a/docshell/base/crashtests/crashtests.list
+++ b/docshell/base/crashtests/crashtests.list
@ -22,4 +22,3 @@ skip-if(Android) pref(dom.disable_open_during_load,false) load 1667491.html
 pref(dom.disable_open_during_load,false) load 1690169-1.html
 load 1672873.html
 load 1753136.html
-HTTP load 1804803.html
--- a/docshell/shistory/SessionHistoryEntry.cpp
+++ b/docshell/shistory/SessionHistoryEntry.cpp
@ -395,7 +395,7 @@ LoadingSessionHistoryInfo::LoadingSessionHistoryInfo(
      mLoadIsFromSessionHistory(aInfo->mLoadIsFromSessionHistory),
      mOffset(aInfo->mOffset),
      mLoadingCurrentEntry(aInfo->mLoadingCurrentEntry) {
-  MOZ_ASSERT(SessionHistoryEntry::GetByLoadId(mLoadId)->mEntry == aEntry);
+  MOZ_ASSERT(SessionHistoryEntry::GetByLoadId(mLoadId) == aEntry);
 }

 LoadingSessionHistoryInfo::LoadingSessionHistoryInfo(
@ -416,14 +416,29 @@ LoadingSessionHistoryInfo::CreateLoadInfo() const {

 static uint32_t gEntryID;

-SessionHistoryEntry::LoadingEntry* SessionHistoryEntry::GetByLoadId(
-    uint64_t aLoadId) {
+SessionHistoryEntry* SessionHistoryEntry::GetByLoadId(uint64_t aLoadId) {
  MOZ_ASSERT(XRE_IsParentProcess());
  if (!sLoadIdToEntry) {
    return nullptr;
  }

-  return sLoadIdToEntry->Lookup(aLoadId).DataPtrOrNull();
+  if (auto entry = sLoadIdToEntry->Lookup(aLoadId)) {
+    return entry->mEntry;
+  }
+  return nullptr;
+}
+
+const SessionHistoryInfo*
+SessionHistoryEntry::GetInfoSnapshotForValidationByLoadId(uint64_t aLoadId) {
+  MOZ_ASSERT(XRE_IsParentProcess());
+  if (!sLoadIdToEntry) {
+    return nullptr;
+  }
+
+  if (auto entry = sLoadIdToEntry->Lookup(aLoadId)) {
+    return entry->mInfoSnapshotForValidation.get();
+  }
+  return nullptr;
 }

 void SessionHistoryEntry::SetByLoadId(uint64_t aLoadId,
--- a/docshell/shistory/SessionHistoryEntry.h
+++ b/docshell/shistory/SessionHistoryEntry.h
@ -368,16 +368,6 @@ class SessionHistoryEntry : public nsISHEntry {
  NS_DECL_NSISHENTRY
  NS_DECLARE_STATIC_IID_ACCESSOR(NS_SESSIONHISTORYENTRY_IID)

-  bool IsInSessionHistory() {
-    SessionHistoryEntry* entry = this;
-    while (SessionHistoryEntry* parent =
-               static_cast<SessionHistoryEntry*>(entry->mParent)) {
-      entry = parent;
-    }
-    return entry->SharedInfo()->mSHistory &&
-           entry->SharedInfo()->mSHistory->IsAlive();
-  }
-
  void ReplaceWith(const SessionHistoryEntry& aSource);

  const SessionHistoryInfo& Info() const { return *mInfo; }
@ -418,18 +408,11 @@ class SessionHistoryEntry : public nsISHEntry {

  void SetWireframe(const Maybe<Wireframe>& aWireframe);

-  struct LoadingEntry {
-    // A pointer to the entry being loaded. Will be cleared by the
-    // SessionHistoryEntry destructor, at latest.
-    SessionHistoryEntry* mEntry;
-    // Snapshot of the entry's SessionHistoryInfo when the load started, to be
-    // used for validation purposes only.
-    UniquePtr<SessionHistoryInfo> mInfoSnapshotForValidation;
-  };
-
  // Get an entry based on LoadingSessionHistoryInfo's mLoadId. Parent process
  // only.
-  static LoadingEntry* GetByLoadId(uint64_t aLoadId);
+  static SessionHistoryEntry* GetByLoadId(uint64_t aLoadId);
+  static const SessionHistoryInfo* GetInfoSnapshotForValidationByLoadId(
+      uint64_t aLoadId);
  static void SetByLoadId(uint64_t aLoadId, SessionHistoryEntry* aEntry);
  static void RemoveLoadId(uint64_t aLoadId);

@ -449,6 +432,15 @@ class SessionHistoryEntry : public nsISHEntry {

  HistoryEntryCounterForBrowsingContext mBCHistoryLength;

+  struct LoadingEntry {
+    // A pointer to the entry being loaded. Will be cleared by the
+    // SessionHistoryEntry destructor, at latest.
+    SessionHistoryEntry* mEntry;
+    // Snapshot of the entry's SessionHistoryInfo when the load started, to be
+    // used for validation purposes only.
+    UniquePtr<SessionHistoryInfo> mInfoSnapshotForValidation;
+  };
+
  static nsTHashMap<nsUint64HashKey, LoadingEntry>* sLoadIdToEntry;
 };

--- a/netwerk/ipc/DocumentLoadListener.cpp
+++ b/netwerk/ipc/DocumentLoadListener.cpp
@ -13,7 +13,6 @@
 #include "mozilla/LoadInfo.h"
 #include "mozilla/MozPromiseInlines.h"  // For MozPromise::FromDomPromise
 #include "mozilla/NullPrincipal.h"
-#include "mozilla/ResultVariant.h"
 #include "mozilla/ScopeExit.h"
 #include "mozilla/StaticPrefs_extensions.h"
 #include "mozilla/StaticPrefs_fission.h"
@ -526,31 +525,29 @@ bool CheckRecursiveLoad(CanonicalBrowsingContext* aLoadingContext,

 // Check that the load state, potentially received from a child process, appears
 // to be performing a load of the specified LoadingSessionHistoryInfo.
-// Returns a Result<…> containing the SessionHistoryEntry found for the
-// LoadingSessionHistoryInfo as success value if the validation succeeded, or a
-// static (telemetry-safe) string naming what did not match as a failure value
-// if the validation failed.
-static Result<SessionHistoryEntry*, const char*> ValidateHistoryLoad(
+// Returns a static (telemetry-safe) string naming what did not match, or
+// nullptr if it succeeds.
+static const char* ValidateHistoryLoad(
    CanonicalBrowsingContext* aLoadingContext,
    nsDocShellLoadState* aLoadState) {
  MOZ_ASSERT(SessionHistoryInParent());
  MOZ_ASSERT(aLoadState->LoadIsFromSessionHistory());

  if (!aLoadState->GetLoadingSessionHistoryInfo()) {
-    return Err("Missing LoadingSessionHistoryInfo");
+    return "Missing LoadingSessionHistoryInfo";
  }

-  SessionHistoryEntry::LoadingEntry* loading = SessionHistoryEntry::GetByLoadId(
-      aLoadState->GetLoadingSessionHistoryInfo()->mLoadId);
-  if (!loading) {
-    return Err("Missing SessionHistoryEntry");
+  const SessionHistoryInfo* snapshot =
+      SessionHistoryEntry::GetInfoSnapshotForValidationByLoadId(
+          aLoadState->GetLoadingSessionHistoryInfo()->mLoadId);
+  if (!snapshot) {
+    return "Invalid LoadId";
  }

-  SessionHistoryInfo* snapshot = loading->mInfoSnapshotForValidation.get();
  // History loads do not inherit principal.
  if (aLoadState->HasInternalLoadFlags(
          nsDocShell::INTERNAL_LOAD_FLAGS_INHERIT_PRINCIPAL)) {
-    return Err("LOAD_FLAGS_INHERIT_PRINCIPAL");
+    return "LOAD_FLAGS_INHERIT_PRINCIPAL";
  }

  auto uriEq = [](nsIURI* a, nsIURI* b) -> bool {
@ -563,34 +560,34 @@ static Result<SessionHistoryEntry*, const char*> ValidateHistoryLoad(

  // XXX: Needing to do all of this validation manually is kinda gross.
  if (!uriEq(snapshot->GetURI(), aLoadState->URI())) {
-    return Err("URI");
+    return "URI";
  }
  if (!uriEq(snapshot->GetOriginalURI(), aLoadState->OriginalURI())) {
-    return Err("OriginalURI");
+    return "OriginalURI";
  }
  if (!aLoadState->ResultPrincipalURIIsSome() ||
      !uriEq(snapshot->GetResultPrincipalURI(),
             aLoadState->ResultPrincipalURI())) {
-    return Err("ResultPrincipalURI");
+    return "ResultPrincipalURI";
  }
  if (!uriEq(snapshot->GetUnstrippedURI(), aLoadState->GetUnstrippedURI())) {
-    return Err("UnstrippedURI");
+    return "UnstrippedURI";
  }
  if (!principalEq(snapshot->GetTriggeringPrincipal(),
                   aLoadState->TriggeringPrincipal())) {
-    return Err("TriggeringPrincipal");
+    return "TriggeringPrincipal";
  }
  if (!principalEq(snapshot->GetPrincipalToInherit(),
                   aLoadState->PrincipalToInherit())) {
-    return Err("PrincipalToInherit");
+    return "PrincipalToInherit";
  }
  if (!principalEq(snapshot->GetPartitionedPrincipalToInherit(),
                   aLoadState->PartitionedPrincipalToInherit())) {
-    return Err("PartitionedPrincipalToInherit");
+    return "PartitionedPrincipalToInherit";
  }

  // Everything matches!
-  return loading->mEntry;
+  return nullptr;
 }

 auto DocumentLoadListener::Open(nsDocShellLoadState* aLoadState,
@ -630,13 +627,10 @@ auto DocumentLoadListener::Open(nsDocShellLoadState* aLoadState,
  //
  // NOTE: Keep this check in-sync with the check in
  // `nsDocShellLoadState::GetEffectiveTriggeringRemoteType()`!
-  RefPtr<SessionHistoryEntry> existingEntry;
  if (SessionHistoryInParent() && aLoadState->LoadIsFromSessionHistory() &&
      aLoadState->LoadType() != LOAD_ERROR_PAGE) {
-    Result<SessionHistoryEntry*, const char*> result =
-        ValidateHistoryLoad(loadingContext, aLoadState);
-    if (result.isErr()) {
-      const char* mismatch = result.unwrapErr();
+    if (const char* mismatch =
+            ValidateHistoryLoad(loadingContext, aLoadState)) {
      LOG(
          ("DocumentLoadListener::Open with invalid loading history entry "
           "[this=%p, mismatch=%s]",
@ -651,21 +645,6 @@ auto DocumentLoadListener::Open(nsDocShellLoadState* aLoadState,
      mParentChannelListener = nullptr;
      return nullptr;
    }
-
-    existingEntry = result.unwrap();
-    if (!existingEntry->IsInSessionHistory()) {
-      SessionHistoryEntry::RemoveLoadId(
-          aLoadState->GetLoadingSessionHistoryInfo()->mLoadId);
-      LOG(
-          ("DocumentLoadListener::Open with disconnected history entry "
-           "[this=%p]",
-           this));
-
-      *aRv = NS_BINDING_ABORTED;
-      mParentChannelListener = nullptr;
-      mChannel = nullptr;
-      return nullptr;
-    }
  }

  if (aLoadState->GetRemoteTypeOverride()) {
@ -722,9 +701,14 @@ auto DocumentLoadListener::Open(nsDocShellLoadState* aLoadState,
    // It's hard to know at this point whether session history will be enabled
    // in the browsing context, so we always create an entry for a load here.
    mLoadingSessionHistoryInfo =
-        documentContext->CreateLoadingSessionHistoryEntryForLoad(
-            aLoadState, existingEntry, mChannel);
-    MOZ_ASSERT(mLoadingSessionHistoryInfo);
+        documentContext->CreateLoadingSessionHistoryEntryForLoad(aLoadState,
+                                                                 mChannel);
+    if (!mLoadingSessionHistoryInfo) {
+      *aRv = NS_BINDING_ABORTED;
+      mParentChannelListener = nullptr;
+      mChannel = nullptr;
+      return nullptr;
+    }
  }

  nsCOMPtr<nsIURI> uriBeingLoaded;