[tor-commits] [tor-browser] 16/37: Bug 1765951 - Stop storing BC pointer in nsSHistory. r=smaug, a=RyanVM

gitolite role git at cupani.torproject.org
Wed Jun 22 18:27:25 UTC 2022


This is an automated email from the git hooks/post-receive script.

richard pushed a commit to branch tor-browser-91.11.0esr-11.5-1
in repository tor-browser.

commit 38d04993eea071abc316c7a678e3c4265a885f28
Author: Peter Van der Beken <peterv at propagandism.org>
AuthorDate: Thu Jun 9 07:17:29 2022 +0000

    Bug 1765951 - Stop storing BC pointer in nsSHistory. r=smaug, a=RyanVM
    
    Differential Revision: https://phabricator.services.mozilla.com/D148748
---
 docshell/base/CanonicalBrowsingContext.cpp |  2 +-
 docshell/shistory/nsSHistory.cpp           | 48 ++++++++++++++++--------------
 docshell/shistory/nsSHistory.h             | 23 +++++++++-----
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp
index 749591918f8cd..3820a192dcf50 100644
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -1023,7 +1023,7 @@ void CanonicalBrowsingContext::RemoveFromSessionHistory(const nsID& aChangeID) {
     AutoTArray<nsID, 16> ids({GetHistoryID()});
     shistory->RemoveEntries(ids, shistory->GetIndexOfEntry(root), &didRemove);
     if (didRemove) {
-      BrowsingContext* rootBC = shistory->GetBrowsingContext();
+      RefPtr<BrowsingContext> rootBC = shistory->GetBrowsingContext();
       if (rootBC) {
         if (!rootBC->IsInProcess()) {
           Unused << rootBC->Canonical()
diff --git a/docshell/shistory/nsSHistory.cpp b/docshell/shistory/nsSHistory.cpp
index e0380dfd2c31f..7d542185ecf37 100644
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -167,11 +167,9 @@ class MOZ_STACK_CLASS SHistoryChangeNotifier {
       MOZ_ASSERT(mSHistory->HasOngoingUpdate());
       mSHistory->SetHasOngoingUpdate(false);
 
-      if (mozilla::SessionHistoryInParent() &&
-          mSHistory->GetBrowsingContext()) {
-        mSHistory->GetBrowsingContext()
-            ->Canonical()
-            ->HistoryCommitIndexAndLength();
+      RefPtr<BrowsingContext> rootBC = mSHistory->GetBrowsingContext();
+      if (mozilla::SessionHistoryInParent() && rootBC) {
+        rootBC->Canonical()->HistoryCommitIndexAndLength();
       }
     }
   }
@@ -254,7 +252,7 @@ void nsSHistory::EvictContentViewerForEntry(nsISHEntry* aEntry) {
 }
 
 nsSHistory::nsSHistory(BrowsingContext* aRootBC)
-    : mRootBC(aRootBC),
+    : mRootBC(aRootBC->Id()),
       mHasOngoingUpdate(false),
       mIndex(-1),
       mRequestedIndex(-1),
@@ -724,13 +722,13 @@ void nsSHistory::HandleEntriesToSwapInDocShell(
   }
 }
 
-void nsSHistory::UpdateRootBrowsingContextState() {
-  if (mRootBC && mRootBC->EverAttached()) {
+void nsSHistory::UpdateRootBrowsingContextState(BrowsingContext* aRootBC) {
+  if (aRootBC && aRootBC->EverAttached()) {
     bool sameDocument = IsEmptyOrHasEntriesForSingleTopLevelPage();
-    if (sameDocument != mRootBC->GetIsSingleToplevelInHistory()) {
+    if (sameDocument != aRootBC->GetIsSingleToplevelInHistory()) {
       // If the browsing context is discarded then its session history is
       // invalid and will go away.
-      Unused << mRootBC->SetIsSingleToplevelInHistory(sameDocument);
+      Unused << aRootBC->SetIsSingleToplevelInHistory(sameDocument);
     }
   }
 }
@@ -808,7 +806,8 @@ nsSHistory::AddEntry(nsISHEntry* aSHEntry, bool aPersist) {
 
   // If we have a root docshell, update the docshell id of the root shentry to
   // match the id of that docshell
-  if (mRootBC) {
+  RefPtr<BrowsingContext> rootBC = GetBrowsingContext();
+  if (rootBC) {
     aSHEntry->SetDocshellID(mRootDocShellID);
   }
 
@@ -1051,8 +1050,9 @@ nsSHistory::PurgeHistory(int32_t aNumEntries) {
     }
   }
 
-  if (mRootBC) {
-    mRootBC->PreOrderWalk([&docshellIDToEntry](BrowsingContext* aBC) {
+  RefPtr<BrowsingContext> rootBC = GetBrowsingContext();
+  if (rootBC) {
+    rootBC->PreOrderWalk([&docshellIDToEntry](BrowsingContext* aBC) {
       SessionHistoryEntry* entry = docshellIDToEntry.Get(aBC->GetHistoryID());
       Unused << aBC->SetHistoryEntryCount(
           entry ? uint32_t(entry->BCHistoryLength()) : 0);
@@ -1068,11 +1068,11 @@ nsSHistory::PurgeHistory(int32_t aNumEntries) {
   mRequestedIndex -= aNumEntries;
   mRequestedIndex = std::max(mRequestedIndex, -1);
 
-  if (mRootBC && mRootBC->GetDocShell()) {
-    mRootBC->GetDocShell()->HistoryPurged(aNumEntries);
+  if (rootBC && rootBC->GetDocShell()) {
+    rootBC->GetDocShell()->HistoryPurged(aNumEntries);
   }
 
-  UpdateRootBrowsingContextState();
+  UpdateRootBrowsingContextState(rootBC);
 
   return NS_OK;
 }
@@ -1838,8 +1838,11 @@ NS_IMETHODIMP_(void)
 nsSHistory::RemoveEntries(nsTArray<nsID>& aIDs, int32_t aStartIndex) {
   bool didRemove;
   RemoveEntries(aIDs, aStartIndex, &didRemove);
-  if (didRemove && mRootBC && mRootBC->GetDocShell()) {
-    mRootBC->GetDocShell()->DispatchLocationChangeEvent();
+  if (didRemove) {
+    RefPtr<BrowsingContext> rootBC = GetBrowsingContext();
+    if (rootBC && rootBC->GetDocShell()) {
+      rootBC->GetDocShell()->DispatchLocationChangeEvent();
+    }
   }
 }
 
@@ -1976,7 +1979,8 @@ nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,
                                bool aSameEpoch, bool aUserActivation) {
   MOZ_LOG(gSHistoryLog, LogLevel::Debug,
           ("LoadEntry(%d, 0x%lx, %u)", aIndex, aLoadType, aHistCmd));
-  if (!mRootBC) {
+  RefPtr<BrowsingContext> rootBC = GetBrowsingContext();
+  if (!rootBC) {
     return NS_ERROR_FAILURE;
   }
 
@@ -2043,13 +2047,13 @@ nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,
 
   if (mRequestedIndex == mIndex) {
     // Possibly a reload case
-    InitiateLoad(nextEntry, mRootBC, aLoadType, aLoadResults, aUserActivation);
+    InitiateLoad(nextEntry, rootBC, aLoadType, aLoadResults, aUserActivation);
     return NS_OK;
   }
 
   // Going back or forward.
   bool differenceFound = LoadDifferingEntries(
-      prevEntry, nextEntry, mRootBC, aLoadType, aLoadResults, aUserActivation);
+      prevEntry, nextEntry, rootBC, aLoadType, aLoadResults, aUserActivation);
   if (!differenceFound) {
     // We did not find any differences. Go further in the history.
     return LoadNextPossibleEntry(aIndex, aLoadType, aHistCmd, aLoadResults,
@@ -2072,7 +2076,7 @@ bool nsSHistory::LoadDifferingEntries(nsISHEntry* aPrevEntry,
   // Check the IDs to verify if the pages are different.
   if (prevID != nextID) {
     // Set the Subframe flag if not navigating the root docshell.
-    aNextEntry->SetIsSubFrame(aParent != mRootBC);
+    aNextEntry->SetIsSubFrame(aParent->Id() != mRootBC);
     InitiateLoad(aNextEntry, aParent, aLoadType, aLoadResults, aUserActivation);
     return true;
   }
diff --git a/docshell/shistory/nsSHistory.h b/docshell/shistory/nsSHistory.h
index 2bf059847dfe8..6269039b521a6 100644
--- a/docshell/shistory/nsSHistory.h
+++ b/docshell/shistory/nsSHistory.h
@@ -173,16 +173,18 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,
 
   int32_t Length() { return int32_t(mEntries.Length()); }
   int32_t Index() { return mIndex; }
-  mozilla::dom::BrowsingContext* GetBrowsingContext() { return mRootBC; }
+  already_AddRefed<mozilla::dom::BrowsingContext> GetBrowsingContext() {
+    return mozilla::dom::BrowsingContext::Get(mRootBC);
+  }
   bool HasOngoingUpdate() { return mHasOngoingUpdate; }
   void SetHasOngoingUpdate(bool aVal) { mHasOngoingUpdate = aVal; }
 
   void SetBrowsingContext(mozilla::dom::BrowsingContext* aRootBC) {
-    if (mRootBC == aRootBC) {
-      return;
+    uint64_t newID = aRootBC ? aRootBC->Id() : 0;
+    if (mRootBC != newID) {
+      mRootBC = newID;
+      UpdateRootBrowsingContextState(aRootBC);
     }
-    mRootBC = aRootBC;
-    UpdateRootBrowsingContextState();
   }
 
   int32_t GetIndexForReplace() {
@@ -194,7 +196,10 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,
 
   // Update the root browsing context state when adding, removing or
   // replacing entries.
-  void UpdateRootBrowsingContextState();
+  void UpdateRootBrowsingContextState() {
+    RefPtr<mozilla::dom::BrowsingContext> rootBC(GetBrowsingContext());
+    UpdateRootBrowsingContextState(rootBC);
+  }
 
   void GetEpoch(uint64_t& aEpoch,
                 mozilla::Maybe<mozilla::dom::ContentParentId>& aId) const {
@@ -212,12 +217,14 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,
  protected:
   virtual ~nsSHistory();
 
-  // Weak reference. Do not refcount this.
-  mozilla::dom::BrowsingContext* mRootBC;
+  uint64_t mRootBC;
 
  private:
   friend class nsSHistoryObserver;
 
+  void UpdateRootBrowsingContextState(
+      mozilla::dom::BrowsingContext* aBrowsingContext);
+
   bool LoadDifferingEntries(nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry,
                             mozilla::dom::BrowsingContext* aParent,
                             long aLoadType,

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the tor-commits mailing list