Bug 1659102 - Merge SetActiveSessionHistoryEntryForTop and SetActiveS…

…essionHistoryEntryForFrame. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D91592
iaxat · Sep 29, 2020 · 435c968 · 435c968
1 parent 5c66694
commit 435c968
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 112 deletions.
diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp
@@ -2679,37 +2679,20 @@ bool BrowsingContext::IsPopupAllowed() {
   return false;
 }
 
-void BrowsingContext::SetActiveSessionHistoryEntryForTop(
+void BrowsingContext::SetActiveSessionHistoryEntry(
     const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-    uint32_t aLoadType) {
+    uint32_t aLoadType, int32_t aChildOffset) {
   if (XRE_IsContentProcess()) {
     nsID changeID = {};
     RefPtr<ChildSHistory> shistory = GetChildSessionHistory();
     if (shistory) {
       changeID = shistory->AddPendingHistoryChange(1, 1);
     }
-    ContentChild::GetSingleton()->SendSetActiveSessionHistoryEntryForTop(
-        this, aPreviousScrollPos, *aInfo, aLoadType, changeID);
+    ContentChild::GetSingleton()->SendSetActiveSessionHistoryEntry(
+        this, aPreviousScrollPos, *aInfo, aLoadType, aChildOffset, changeID);
   } else {
-    Canonical()->SetActiveSessionHistoryEntryForTop(aPreviousScrollPos, aInfo,
-                                                    aLoadType, nsID());
-  }
-}
-
-void BrowsingContext::SetActiveSessionHistoryEntryForFrame(
-    const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-    int32_t aChildOffset) {
-  if (XRE_IsContentProcess()) {
-    nsID changeID = {};
-    RefPtr<ChildSHistory> shistory = GetChildSessionHistory();
-    if (shistory) {
-      changeID = shistory->AddPendingHistoryChange(1, 1);
-    }
-    ContentChild::GetSingleton()->SendSetActiveSessionHistoryEntryForFrame(
-        this, aPreviousScrollPos, *aInfo, aChildOffset, changeID);
-  } else {
-    Canonical()->SetActiveSessionHistoryEntryForFrame(aPreviousScrollPos, aInfo,
-                                                      aChildOffset, nsID());
+    Canonical()->SetActiveSessionHistoryEntry(aPreviousScrollPos, aInfo,
+                                              aLoadType, aChildOffset, nsID());
   }
 }
 

diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h
@@ -666,19 +666,11 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
   // context or any of its ancestors.
   bool IsPopupAllowed();
 
-  // Set a new active entry on this browsing context. Should only be called if
-  // IsTop() returns true. This is used for implementing
-  // history.pushState/replaceState.
-  void SetActiveSessionHistoryEntryForTop(
-      const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-      uint32_t aLoadType);
-
-  // Set a new active entry on this browsing context. Should only be called if
-  // IsTop() returns false. This is used for implementing
-  // history.pushState/replaceState.
-  void SetActiveSessionHistoryEntryForFrame(
-      const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-      int32_t aChildOffset);
+  // Set a new active entry on this browsing context. This is used for
+  // implementing history.pushState/replaceState and same document navigations.
+  void SetActiveSessionHistoryEntry(const Maybe<nsPoint>& aPreviousScrollPos,
+                                    SessionHistoryInfo* aInfo,
+                                    uint32_t aLoadType, int32_t aChildOffset);
 
   // Replace the active entry for this browsing context. This is used for
   // implementing history.replaceState.

diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp
@@ -564,48 +564,36 @@ void CanonicalBrowsingContext::NotifyOnHistoryReload(
   // the nsDocShell will create a load state based on its document.
 }
 
-void CanonicalBrowsingContext::SetActiveSessionHistoryEntryForTop(
+void CanonicalBrowsingContext::SetActiveSessionHistoryEntry(
     const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-    uint32_t aLoadType, const nsID& aChangeID) {
+    uint32_t aLoadType, int32_t aChildOffset, const nsID& aChangeID) {
   RefPtr<SessionHistoryEntry> oldActiveEntry = mActiveEntry;
   if (aPreviousScrollPos.isSome() && oldActiveEntry) {
     oldActiveEntry->SetScrollPosition(aPreviousScrollPos.ref().x,
                                       aPreviousScrollPos.ref().y);
   }
-  RefPtr<SessionHistoryEntry> newEntry = new SessionHistoryEntry(aInfo);
-  newEntry->SetDocshellID(GetHistoryID());
-  mActiveEntry = newEntry;
-  Maybe<int32_t> previousEntryIndex, loadedEntryIndex;
-  nsISHistory* shistory = GetSessionHistory();
-  if (shistory) {
-    shistory->AddToRootSessionHistory(
-        true, oldActiveEntry, this, newEntry, aLoadType,
-        nsDocShell::ShouldAddToSessionHistory(aInfo->GetURI(), nullptr),
-        &previousEntryIndex, &loadedEntryIndex);
-    // FIXME Need to do the equivalent of EvictContentViewersOrReplaceEntry.
-    HistoryCommitIndexAndLength(aChangeID);
-  }
-}
+  mActiveEntry = new SessionHistoryEntry(aInfo);
+  mActiveEntry->SetDocshellID(GetHistoryID());
 
-void CanonicalBrowsingContext::SetActiveSessionHistoryEntryForFrame(
-    const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-    int32_t aChildOffset, const nsID& aChangeID) {
-  RefPtr<SessionHistoryEntry> oldActiveEntry = mActiveEntry;
-  if (aPreviousScrollPos.isSome() && oldActiveEntry) {
-    oldActiveEntry->SetScrollPosition(aPreviousScrollPos.ref().x,
-                                      aPreviousScrollPos.ref().y);
-  }
-  RefPtr<SessionHistoryEntry> newEntry = new SessionHistoryEntry(aInfo);
-  newEntry->SetDocshellID(GetHistoryID());
-  mActiveEntry = newEntry;
   nsISHistory* shistory = GetSessionHistory();
-  if (oldActiveEntry) {
+  if (IsTop()) {
     if (shistory) {
-      shistory->AddChildSHEntryHelper(oldActiveEntry, newEntry, Top(), true);
+      Maybe<int32_t> previousEntryIndex, loadedEntryIndex;
+      shistory->AddToRootSessionHistory(
+          true, oldActiveEntry, this, mActiveEntry, aLoadType,
+          nsDocShell::ShouldAddToSessionHistory(aInfo->GetURI(), nullptr),
+          &previousEntryIndex, &loadedEntryIndex);
+    }
+  } else {
+    if (oldActiveEntry) {
+      if (shistory) {
+        shistory->AddChildSHEntryHelper(oldActiveEntry, mActiveEntry, Top(),
+                                        true);
+      }
+    } else if (GetParent() && GetParent()->mActiveEntry) {
+      GetParent()->mActiveEntry->AddChild(mActiveEntry, aChildOffset,
+                                          UseRemoteSubframes());
     }
-  } else if (GetParent() && GetParent()->mActiveEntry) {
-    GetParent()->mActiveEntry->AddChild(newEntry, aChildOffset,
-                                        UseRemoteSubframes());
   }
   // FIXME Need to do the equivalent of EvictContentViewersOrReplaceEntry.
   HistoryCommitIndexAndLength(aChangeID);

diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h
@@ -120,13 +120,10 @@ class CanonicalBrowsingContext final : public BrowsingContext {
                              Maybe<RefPtr<nsDocShellLoadState>>& aLoadState,
                              Maybe<bool>& aReloadActiveEntry);
 
-  void SetActiveSessionHistoryEntryForTop(
-      const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-      uint32_t aLoadType, const nsID& aChangeID);
-
-  void SetActiveSessionHistoryEntryForFrame(
-      const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo* aInfo,
-      int32_t aChildOffset, const nsID& aChangeID);
+  void SetActiveSessionHistoryEntry(const Maybe<nsPoint>& aPreviousScrollPos,
+                                    SessionHistoryInfo* aInfo,
+                                    uint32_t aLoadType, int32_t aChildOffset,
+                                    const nsID& aChangeID);
 
   void ReplaceActiveSessionHistoryEntry(SessionHistoryInfo* aInfo);
 

diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -11537,16 +11537,11 @@ void nsDocShell::UpdateActiveEntry(
   if (replace) {
     mBrowsingContext->ReplaceActiveSessionHistoryEntry(mActiveEntry.get());
   } else {
-    if (mBrowsingContext->IsTop()) {
-      mBrowsingContext->SetActiveSessionHistoryEntryForTop(
-          aPreviousScrollPos, mActiveEntry.get(), mLoadType);
-      // FIXME Do we need to update mPreviousEntryIndex and mLoadedEntryIndex?
-    } else {
-      // FIXME We should probably just compute mChildOffset in the parent
-      //       instead of passing it over IPC here.
-      mBrowsingContext->SetActiveSessionHistoryEntryForFrame(
-          aPreviousScrollPos, mActiveEntry.get(), mChildOffset);
-    }
+    // FIXME We should probably just compute mChildOffset in the parent
+    //       instead of passing it over IPC here.
+    mBrowsingContext->SetActiveSessionHistoryEntry(
+        aPreviousScrollPos, mActiveEntry.get(), mLoadType, mChildOffset);
+    // FIXME Do we need to update mPreviousEntryIndex and mLoadedEntryIndex?
   }
 }
 

diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp
@@ -7042,24 +7042,13 @@ ContentParent::RecvGetLoadingSessionHistoryInfoFromParent(
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult ContentParent::RecvSetActiveSessionHistoryEntryForTop(
+mozilla::ipc::IPCResult ContentParent::RecvSetActiveSessionHistoryEntry(
     const MaybeDiscarded<BrowsingContext>& aContext,
     const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo&& aInfo,
-    uint32_t aLoadType, const nsID& aChangeID) {
+    uint32_t aLoadType, int32_t aChildOffset, const nsID& aChangeID) {
   if (!aContext.IsDiscarded()) {
-    aContext.get_canonical()->SetActiveSessionHistoryEntryForTop(
-        aPreviousScrollPos, &aInfo, aLoadType, aChangeID);
-  }
-  return IPC_OK();
-}
-
-mozilla::ipc::IPCResult ContentParent::RecvSetActiveSessionHistoryEntryForFrame(
-    const MaybeDiscarded<BrowsingContext>& aContext,
-    const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo&& aInfo,
-    int32_t aChildOffset, const nsID& aChangeID) {
-  if (!aContext.IsDiscarded()) {
-    aContext.get_canonical()->SetActiveSessionHistoryEntryForFrame(
-        aPreviousScrollPos, &aInfo, aChildOffset, aChangeID);
+    aContext.get_canonical()->SetActiveSessionHistoryEntry(
+        aPreviousScrollPos, &aInfo, aLoadType, aChildOffset, aChangeID);
   }
   return IPC_OK();
 }

diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h
@@ -1349,15 +1349,10 @@ class ContentParent final
       const MaybeDiscarded<BrowsingContext>& aContext,
       GetLoadingSessionHistoryInfoFromParentResolver&& aResolver);
 
-  mozilla::ipc::IPCResult RecvSetActiveSessionHistoryEntryForTop(
+  mozilla::ipc::IPCResult RecvSetActiveSessionHistoryEntry(
       const MaybeDiscarded<BrowsingContext>& aContext,
       const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo&& aInfo,
-      uint32_t aLoadType, const nsID& aChangeID);
-
-  mozilla::ipc::IPCResult RecvSetActiveSessionHistoryEntryForFrame(
-      const MaybeDiscarded<BrowsingContext>& aContext,
-      const Maybe<nsPoint>& aPreviousScrollPos, SessionHistoryInfo&& aInfo,
-      int32_t aChildOffset, const nsID& aChangeID);
+      uint32_t aLoadType, int32_t aChildOffset, const nsID& aChangeID);
 
   mozilla::ipc::IPCResult RecvReplaceActiveSessionHistoryEntry(
       const MaybeDiscarded<BrowsingContext>& aContext,

diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl
@@ -1697,13 +1697,10 @@ parent:
                              nsID? aAgentClusterId)
       returns (BlobURLDataRequestResult aResult);
 
-    async SetActiveSessionHistoryEntryForTop(MaybeDiscardedBrowsingContext aContext,
+    async SetActiveSessionHistoryEntry(MaybeDiscardedBrowsingContext context,
                                        nsPoint? previousScrollPosition,
                                        SessionHistoryInfo info, uint32_t loadType,
-                                       nsID changeID);
-    async SetActiveSessionHistoryEntryForFrame(
-        MaybeDiscardedBrowsingContext aContext, nsPoint? previousScrollPosition,
-        SessionHistoryInfo info, int32_t childOffset, nsID changeID);
+                                       int32_t childOffset, nsID changeID);
     async ReplaceActiveSessionHistoryEntry(
         MaybeDiscardedBrowsingContext aContext, SessionHistoryInfo info);