From 80aa10467aad41dbd1224fb2848a36601daf6980 Mon Sep 17 00:00:00 2001 From: Peter Van der Beken Date: Wed, 13 Mar 2024 10:56:33 +0000 Subject: [PATCH] Bug 1883278 - Force-enable platform collection code of session store when SHIP is enabled. r=farre,geckoview-reviewers,sessionstore-reviewers If session history in the parent is enabled then session store only works correctly if the platform collection code is turned on. Differential Revision: https://phabricator.services.mozilla.com/D203375 --- docshell/base/CanonicalBrowsingContext.cpp | 2 +- docshell/base/nsDocShell.cpp | 4 ++-- dom/base/nsFrameLoader.cpp | 2 +- dom/ipc/BrowserChild.cpp | 2 +- dom/storage/SessionStorageManager.cpp | 3 ++- .../chrome/geckoview/SessionStateAggregator.js | 3 +-- modules/libpref/init/StaticPrefList.yaml | 13 ++++++++----- .../sessionstore/SessionStoreChangeListener.cpp | 3 +-- toolkit/xre/nsAppRunner.cpp | 12 ++++++++++++ xpcom/system/nsIXULRuntime.idl | 9 +++++++++ 10 files changed, 38 insertions(+), 15 deletions(-) diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index 587784cf78eb..3636d4a0e2f0 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -2607,7 +2607,7 @@ void CanonicalBrowsingContext::UpdateSessionStoreForStorage( } void CanonicalBrowsingContext::MaybeScheduleSessionStoreUpdate() { - if (!StaticPrefs::browser_sessionstore_platform_collection_AtStartup()) { + if (!SessionStorePlatformCollection()) { return; } diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 12602485df65..3404597343e0 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -5666,7 +5666,7 @@ nsDocShell::OnStateChange(nsIWebProgress* aProgress, nsIRequest* aRequest, mBusyFlags = (BusyFlags)(BUSY_FLAGS_BUSY | BUSY_FLAGS_BEFORE_PAGE_LOAD); if ((aStateFlags & STATE_RESTORING) == 0) { - if (StaticPrefs::browser_sessionstore_platform_collection_AtStartup()) { + if (SessionStorePlatformCollection()) { if (IsForceReloadType(mLoadType)) { if (WindowContext* windowContext = mBrowsingContext->GetCurrentWindowContext()) { @@ -6393,7 +6393,7 @@ nsresult nsDocShell::EndPageLoad(nsIWebProgress* aProgress, // incorrectly overrides session store data from the following load. return NS_OK; } - if (StaticPrefs::browser_sessionstore_platform_collection_AtStartup()) { + if (SessionStorePlatformCollection()) { if (WindowContext* windowContext = mBrowsingContext->GetCurrentWindowContext()) { using Change = SessionStoreChangeListener::Change; diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 5712bc944736..eca528f2588b 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -3016,7 +3016,7 @@ nsresult nsFrameLoader::EnsureMessageManager() { NS_ENSURE_TRUE(mChildMessageManager, NS_ERROR_UNEXPECTED); // Set up session store - if (StaticPrefs::browser_sessionstore_platform_collection_AtStartup()) { + if (SessionStorePlatformCollection()) { if (XRE_IsParentProcess() && mIsTopLevelContent) { mSessionStoreChild = SessionStoreChild::GetOrCreate( GetExtantBrowsingContext(), mOwnerContent); diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 63b524ebe7e0..bdd10fdcb285 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -463,7 +463,7 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent, mIPCOpen = true; - if (StaticPrefs::browser_sessionstore_platform_collection_AtStartup()) { + if (SessionStorePlatformCollection()) { mSessionStoreChild = SessionStoreChild::GetOrCreate(mBrowsingContext); } diff --git a/dom/storage/SessionStorageManager.cpp b/dom/storage/SessionStorageManager.cpp index 020083730ab5..a64d5784267d 100644 --- a/dom/storage/SessionStorageManager.cpp +++ b/dom/storage/SessionStorageManager.cpp @@ -26,6 +26,7 @@ #include "mozilla/ipc/BackgroundChild.h" #include "mozilla/ipc/BackgroundParent.h" #include "mozilla/ipc/PBackgroundChild.h" +#include "nsIXULRuntime.h" #include "nsTHashMap.h" #include "nsThreadUtils.h" @@ -918,7 +919,7 @@ void BackgroundSessionStorageManager::SetCurrentBrowsingContextId( } void BackgroundSessionStorageManager::MaybeScheduleSessionStoreUpdate() { - if (!StaticPrefs::browser_sessionstore_platform_collection_AtStartup()) { + if (!SessionStorePlatformCollection()) { return; } diff --git a/mobile/android/chrome/geckoview/SessionStateAggregator.js b/mobile/android/chrome/geckoview/SessionStateAggregator.js index 53ae9e9167ba..e41c31ba062a 100644 --- a/mobile/android/chrome/geckoview/SessionStateAggregator.js +++ b/mobile/android/chrome/geckoview/SessionStateAggregator.js @@ -29,7 +29,6 @@ const DEFAULT_INTERVAL_MS = 1500; const TIMEOUT_DISABLED_PREF = "browser.sessionstore.debug.no_auto_updates"; const PREF_INTERVAL = "browser.sessionstore.interval"; -const PREF_SESSION_COLLECTION = "browser.sessionstore.platform_collection"; class Handler { constructor(store) { @@ -607,7 +606,7 @@ class SessionStateAggregator extends GeckoViewChildModule { this.messageQueue, ]; - if (!Services.prefs.getBoolPref(PREF_SESSION_COLLECTION, false)) { + if (!Services.appinfo.sessionStorePlatformCollection) { this.handlers.push( new FormDataListener(this), new ScrollPositionListener(this) diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 42e9093fb1d6..b235ac3dcbf9 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -1655,15 +1655,18 @@ value: 15000 mirror: always -# Platform collection of data for session store -- name: browser.sessionstore.platform_collection +# Disable collection of data for session store using the native collector code, +# instead use the older implementation that's not compatible with session +# history in the parent (and thus Fission). +- name: browser.sessionstore.disable_platform_collection type: bool -#if defined(ANDROID) || defined(MOZ_THUNDERBIRD) - value: false -#else +#if defined(MOZ_THUNDERBIRD) value: true +#else + value: false #endif mirror: once + do_not_use_directly: true # Causes SessionStore to ignore non-final update messages from # browser tabs that were not caused by a flush from the parent. diff --git a/toolkit/components/sessionstore/SessionStoreChangeListener.cpp b/toolkit/components/sessionstore/SessionStoreChangeListener.cpp index 23527018d9b6..753c7324fd74 100644 --- a/toolkit/components/sessionstore/SessionStoreChangeListener.cpp +++ b/toolkit/components/sessionstore/SessionStoreChangeListener.cpp @@ -134,8 +134,7 @@ SessionStoreChangeListener::HandleEvent(dom::Event* aEvent) { /* static */ already_AddRefed SessionStoreChangeListener::Create(BrowsingContext* aBrowsingContext) { - MOZ_RELEASE_ASSERT( - StaticPrefs::browser_sessionstore_platform_collection_AtStartup()); + MOZ_RELEASE_ASSERT(SessionStorePlatformCollection()); if (!aBrowsingContext) { return nullptr; } diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index faa13b16304a..0d7a8fd0fcfa 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -1024,6 +1024,12 @@ bool SessionHistoryInParent() { fission_disableSessionHistoryInParent_AtStartup_DoNotUseDirectly(); } +bool SessionStorePlatformCollection() { + return SessionHistoryInParent() && + !StaticPrefs:: + browser_sessionstore_disable_platform_collection_AtStartup_DoNotUseDirectly(); +} + bool BFCacheInParent() { return SessionHistoryInParent() && StaticPrefs::fission_bfcacheInParent_DoNotUseDirectly(); @@ -1428,6 +1434,12 @@ nsXULAppInfo::GetSessionHistoryInParent(bool* aResult) { return NS_OK; } +NS_IMETHODIMP +nsXULAppInfo::GetSessionStorePlatformCollection(bool* aResult) { + *aResult = SessionStorePlatformCollection(); + return NS_OK; +} + NS_IMETHODIMP nsXULAppInfo::GetBrowserTabsRemoteAutostart(bool* aResult) { *aResult = BrowserTabsRemoteAutostart(); diff --git a/xpcom/system/nsIXULRuntime.idl b/xpcom/system/nsIXULRuntime.idl index 8beadae9f102..9d187bcbba22 100644 --- a/xpcom/system/nsIXULRuntime.idl +++ b/xpcom/system/nsIXULRuntime.idl @@ -30,6 +30,10 @@ bool FissionExperimentEnrolled(); // fission.disableSessionHistoryInParent is false. bool SessionHistoryInParent(); +// Returns true if SessionHistoryInParent() is true and +// browser.sessionstore.disable_platform_collection is false. +bool SessionStorePlatformCollection(); + // Returns true if SessionHistoryInParent() returns true and // fission.bfcacheInParent is true. bool BFCacheInParent(); @@ -168,6 +172,11 @@ interface nsIXULRuntime : nsISupports */ readonly attribute boolean sessionHistoryInParent; + /** + * Whether Gecko code drives session store collection data. + */ + readonly attribute boolean sessionStorePlatformCollection; + /** * Whether to write console errors to a log file. If a component * encounters startup errors that might prevent the app from showing