Bug 1536471 - Consolidate calls to nsISHEntry::GetChildAt in nsDocShell::ClearFrameHistory by adding a new sync IPC call nsISHistory::RemoveFrameEntries, r=peterv, r=nika for adding sync IPC messages
authorAnny Gakhokidze <agakhokidze@mozilla.com>
Fri, 22 Mar 2019 15:45:38 -0400
changeset 500491 4c45890e566b34ef07b0cc5670e99a3fec748483
parent 500490 2ca26ca71fa0f4b459f1e02e8a73e537e07e8d20
child 500492 8d5396434bb4a2b53481a1df3b2d65b29f666676
push id114165
push userpvanderbeken@mozilla.com
push dateWed, 06 Nov 2019 09:18:40 +0000
treeherdermozilla-inbound@db1ddab2985d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, nika
bugs1536471
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1536471 - Consolidate calls to nsISHEntry::GetChildAt in nsDocShell::ClearFrameHistory by adding a new sync IPC call nsISHistory::RemoveFrameEntries, r=peterv, r=nika for adding sync IPC messages Currently, nsDocShell repeatedly calls nsISHEntry::GetChildAt, which results in as many IPC sync calls as the number of children a session history entry has. Calling nsISHEntry::GetChildCount and ChildSHistory::Index and incurs additional extra 2 sync IPC calls. With the proposed solution, there will only be 1 sync IPC call. Differential Revision: https://phabricator.services.mozilla.com/D24980
docshell/base/nsDocShell.cpp
docshell/shistory/PSHistory.ipdl
docshell/shistory/SHistoryChild.cpp
docshell/shistory/SHistoryParent.cpp
docshell/shistory/SHistoryParent.h
docshell/shistory/nsISHistory.idl
docshell/shistory/nsSHistory.cpp
ipc/ipdl/sync-messages.ini
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3582,27 +3582,17 @@ nsDocShell::GetDeviceSizeIsPageSize(bool
 }
 
 void nsDocShell::ClearFrameHistory(nsISHEntry* aEntry) {
   RefPtr<ChildSHistory> rootSH = GetRootSessionHistory();
   if (!rootSH || !aEntry) {
     return;
   }
 
-  int32_t count = aEntry->GetChildCount();
-  AutoTArray<nsID, 16> ids;
-  for (int32_t i = 0; i < count; ++i) {
-    nsCOMPtr<nsISHEntry> child;
-    aEntry->GetChildAt(i, getter_AddRefs(child));
-    if (child) {
-      child->GetDocshellID(*ids.AppendElement());
-    }
-  }
-  int32_t index = rootSH->Index();
-  rootSH->LegacySHistory()->RemoveEntries(ids, index);
+  rootSH->LegacySHistory()->RemoveFrameEntries(aEntry);
 }
 
 //-------------------------------------
 //-- Helper Method for Print discovery
 //-------------------------------------
 bool nsDocShell::IsPrintingOrPP(bool aDisplayErrorDialog) {
   if (mIsPrintingOrPP && aDisplayErrorDialog) {
     DisplayLoadError(NS_ERROR_DOCUMENT_IS_PRINTMODE, nullptr, nullptr, nullptr);
--- a/docshell/shistory/PSHistory.ipdl
+++ b/docshell/shistory/PSHistory.ipdl
@@ -46,16 +46,17 @@ parent:
   sync AddEntry(PSHEntry entry, bool persist) returns (nsresult result, int32_t entriesPurged);
   sync UpdateIndex();
   sync ReplaceEntry(int32_t index, PSHEntry entry) returns (nsresult result);
   sync NotifyOnHistoryReload() returns (bool ok);
   sync EvictOutOfRangeContentViewers(int32_t index);
   sync EvictAllContentViewers();
   sync RemoveDynEntries(int32_t index, PSHEntry entry);
   sync RemoveEntries(nsID[] ids, int32_t index) returns (bool didRemove);
+  sync RemoveFrameEntries(PSHEntry entry);
   sync Reload(uint32_t reloadFlags) returns (LoadSHEntryResult load);
   sync GetAllEntries() returns (MaybeNewPSHEntry[] entries);
   sync FindEntryForBFCache(uint64_t sharedID, bool includeCurrentEntry) returns (MaybeNewPSHEntry entries, int32_t startIndex);
   sync Evict(PSHEntry[] entry);
 
   async __delete__();
 };
 
--- a/docshell/shistory/SHistoryChild.cpp
+++ b/docshell/shistory/SHistoryChild.cpp
@@ -324,16 +324,21 @@ NS_IMETHODIMP_(void)
 SHistoryChild::RemoveEntries(nsTArray<nsID>& aIDs, int32_t aStartIndex) {
   bool didRemove = false;
   if (SendRemoveEntries(aIDs, aStartIndex, &didRemove) && didRemove &&
       mRootDocShell) {
     mRootDocShell->DispatchLocationChangeEvent();
   }
 }
 
+NS_IMETHODIMP_(void)
+SHistoryChild::RemoveFrameEntries(nsISHEntry* aEntry) {
+  SendRemoveFrameEntries(static_cast<SHEntryChild*>(aEntry));
+}
+
 NS_IMETHODIMP
 SHistoryChild::Reload(uint32_t aReloadFlags) {
   bool canNavigate = true;
   nsAutoTObserverArray<nsWeakPtr, 2>::EndLimitedIterator iter(mListeners);
   while (iter.HasMore()) {
     nsCOMPtr<nsISHistoryListener> listener = do_QueryReferent(iter.GetNext());
     if (listener) {
       bool canceled = false;
--- a/docshell/shistory/SHistoryParent.cpp
+++ b/docshell/shistory/SHistoryParent.cpp
@@ -141,16 +141,22 @@ bool SHistoryParent::RecvRemoveDynEntrie
 }
 
 bool SHistoryParent::RecvRemoveEntries(nsTArray<nsID>&& aIds, int32_t aIndex,
                                        bool* aDidRemove) {
   mHistory->RemoveEntries(aIds, aIndex, aDidRemove);
   return true;
 }
 
+bool SHistoryParent::RecvRemoveFrameEntries(PSHEntryParent* aEntry) {
+  mHistory->RemoveFrameEntries(
+      static_cast<SHEntryParent*>(aEntry)->GetSHEntry());
+  return true;
+}
+
 bool SHistoryParent::RecvReload(const uint32_t& aReloadFlags,
                                 LoadSHEntryResult* aLoadResult) {
   nsSHistory::LoadEntryResult loadResult;
   nsresult rv = mHistory->Reload(aReloadFlags, loadResult);
   FillInLoadResult(Manager(), rv, loadResult, aLoadResult);
   return true;
 }
 
--- a/docshell/shistory/SHistoryParent.h
+++ b/docshell/shistory/SHistoryParent.h
@@ -67,16 +67,17 @@ class SHistoryParent final : public PSHi
   bool RecvReplaceEntry(int32_t aIndex, PSHEntryParent* aEntry,
                         nsresult* aResult);
   bool RecvNotifyOnHistoryReload(bool* aOk);
   bool RecvEvictOutOfRangeContentViewers(int32_t aIndex);
   bool RecvEvictAllContentViewers();
   bool RecvRemoveDynEntries(int32_t aIndex, PSHEntryParent* aEntry);
   bool RecvRemoveEntries(nsTArray<nsID>&& ids, int32_t aIndex,
                          bool* aDidRemove);
+  bool RecvRemoveFrameEntries(PSHEntryParent* aEntry);
   bool RecvReload(const uint32_t& aReloadFlags, LoadSHEntryResult* aLoadResult);
   bool RecvGetAllEntries(nsTArray<MaybeNewPSHEntry>* aEntries);
   bool RecvFindEntryForBFCache(const uint64_t& aSharedID,
                                const bool& aIncludeCurrentEntry,
                                MaybeNewPSHEntry* aEntry, int32_t* aIndex);
   bool RecvEvict(nsTArray<PSHEntryParent*>&& aEntries);
 
   RefPtr<CanonicalBrowsingContext> mContext;
--- a/docshell/shistory/nsISHistory.idl
+++ b/docshell/shistory/nsISHistory.idl
@@ -243,11 +243,21 @@ interface nsISHistory: nsISupports
 
   /**
    * Removes entries from the history if their docshellID is in
    * aIDs array.
    */
   [noscript, notxpcom]
   void RemoveEntries(in nsDocshellIDArray aIDs, in long aStartIndex);
 
+  /**
+   * Collect docshellIDs from aEntry's children and remove those
+   * entries from history.
+   *
+   * @param aEntry           Children docshellID's will be collected from 
+   *                         this entry and passed to RemoveEntries as aIDs.
+  */
+  [noscript, notxpcom]
+  void RemoveFrameEntries(in nsISHEntry aEntry);
+
   [noscript]
   void Reload(in unsigned long aReloadFlags);
 };
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -1295,16 +1295,29 @@ void nsSHistory::RemoveEntries(nsTArray<
   while (index > minIndex) {
     if (index != mIndex && RemoveDuplicate(index, index < mIndex)) {
       *aDidRemove = true;
     }
     --index;
   }
 }
 
+void nsSHistory::RemoveFrameEntries(nsISHEntry* aEntry) {
+  int32_t count = aEntry->GetChildCount();
+  AutoTArray<nsID, 16> ids;
+  for (int32_t i = 0; i < count; ++i) {
+    nsCOMPtr<nsISHEntry> child;
+    aEntry->GetChildAt(i, getter_AddRefs(child));
+    if (child) {
+      child->GetDocshellID(*ids.AppendElement());
+    }
+  }
+  RemoveEntries(ids, mIndex);
+}
+
 void nsSHistory::RemoveDynEntries(int32_t aIndex, nsISHEntry* aEntry) {
   // Remove dynamic entries which are at the index and belongs to the container.
   nsCOMPtr<nsISHEntry> entry(aEntry);
   if (!entry) {
     GetEntryAtIndex(aIndex, getter_AddRefs(entry));
   }
 
   if (entry) {
--- a/ipc/ipdl/sync-messages.ini
+++ b/ipc/ipdl/sync-messages.ini
@@ -837,16 +837,18 @@ description = Standing up Fission
 [PSHistory::EvictOutOfRangeContentViewers]
 description = Standing up Fission
 [PSHistory::EvictAllContentViewers]
 description = Standing up Fission
 [PSHistory::RemoveDynEntries]
 description = Standing up Fission
 [PSHistory::RemoveEntries]
 description = Standing up Fission
+[PSHistory::RemoveFrameEntries]
+description = Standing up Fission
 [PSHistory::Reload]
 description = Standing up Fission
 [PSHistory::GetAllEntries]
 description = Standing up Fission
 [PSHistory::FindEntryForBFCache]
 description = Standing up Fission
 [PSHistory::Evict]
 description = Standing up Fission