From e6788be168d367bba3c97bc6240e3ca4be03883c Mon Sep 17 00:00:00 2001 From: Kashav Madan Date: Sat, 26 Jun 2021 05:49:12 +0000 Subject: [PATCH] Bug 1703692 - Store the latest embedder's permanent key on CanonicalBrowsingContext, r=nika,mccr8 And include it in Session Store flushes to avoid dropping updates in case the browser is unavailable. Differential Revision: https://phabricator.services.mozilla.com/D118385 --- .../components/sessionstore/SessionStore.jsm | 45 +++++++++--- docshell/base/BrowsingContext.cpp | 4 ++ docshell/base/CanonicalBrowsingContext.cpp | 70 ++++++++++++++----- docshell/base/CanonicalBrowsingContext.h | 11 ++- dom/interfaces/base/nsIBrowser.idl | 6 ++ dom/ipc/BrowserParent.cpp | 24 ++++--- dom/ipc/WindowGlobalParent.cpp | 23 +++--- .../sessionstore/SessionStoreFunctions.idl | 7 +- .../sessionstore/SessionStoreFunctions.jsm | 46 ++++++++---- .../sessionstore/SessionStoreListener.cpp | 31 +++++--- 10 files changed, 192 insertions(+), 75 deletions(-) diff --git a/browser/components/sessionstore/SessionStore.jsm b/browser/components/sessionstore/SessionStore.jsm index 0b2323ba5b7e..6386a588c340 100644 --- a/browser/components/sessionstore/SessionStore.jsm +++ b/browser/components/sessionstore/SessionStore.jsm @@ -408,10 +408,16 @@ var SessionStore = { return SessionStoreInternal.reviveAllCrashedTabs(); }, - updateSessionStoreFromTablistener(aBrowser, aBrowsingContext, aData) { + updateSessionStoreFromTablistener( + aBrowser, + aBrowsingContext, + aPermanentKey, + aData + ) { return SessionStoreInternal.updateSessionStoreFromTablistener( aBrowser, aBrowsingContext, + aPermanentKey, aData ); }, @@ -1202,13 +1208,34 @@ var SessionStoreInternal = { Services.obs.notifyObservers(browser, NOTIFY_BROWSER_SHUTDOWN_FLUSH); }, - updateSessionStoreFromTablistener(aBrowser, aBrowsingContext, aData) { - if (aBrowser.permanentKey == undefined) { - return; + updateSessionStoreFromTablistener( + aBrowser, + aBrowsingContext, + aPermanentKey, + aData + ) { + let browser = aBrowser; + + if (!browser || browser.permanentKey === undefined) { + if (!aPermanentKey) { + return; + } + + // A little weird, but this lets us use |aPermanentKey| as an argument + // to functions that take a browser element, since most only need the + // permanent key. + // + // This should only be around as long as we're still depending on + // permanent key in Session Store, see bug 1716788. + browser = { + permanentKey: aPermanentKey, + ownerGlobal: + aBrowsingContext.currentWindowGlobal?.browsingContext?.window, + }; } // Ignore sessionStore update from previous epochs - if (!this.isCurrentEpoch(aBrowser, aData.epoch)) { + if (!this.isCurrentEpoch(browser, aData.epoch)) { return; } @@ -1222,10 +1249,10 @@ var SessionStoreInternal = { aBrowsingContext.sessionHistory ) { let listener = - this._browserSHistoryListener.get(aBrowser.permanentKey) ?? - this.addSHistoryListener(aBrowser, aBrowsingContext); + this._browserSHistoryListener.get(browser.permanentKey) ?? + this.addSHistoryListener(browser, aBrowsingContext); - let historychange = listener.collect(aBrowser, aBrowsingContext, { + let historychange = listener.collect(browser, aBrowsingContext, { collectFull: !!aData.sHistoryNeeded, writeToCache: false, }); @@ -1235,7 +1262,7 @@ var SessionStoreInternal = { } } - this.onTabStateUpdate(aBrowser, aData); + this.onTabStateUpdate(browser, aData); }, /** diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 4378d1df282a..8a70f5f30f9d 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -693,6 +693,10 @@ void BrowsingContext::SetEmbedderElement(Element* aEmbedder) { MOZ_ALWAYS_SUCCEEDS(txn.Commit(this)); } + if (XRE_IsParentProcess() && IsTopContent()) { + Canonical()->MaybeSetPermanentKey(aEmbedder); + } + mEmbedderElement = aEmbedder; if (mEmbedderElement) { diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index e803f94cb678..afe2db005043 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -78,7 +78,8 @@ CanonicalBrowsingContext::CanonicalBrowsingContext(WindowContext* aParentWindow, : BrowsingContext(aParentWindow, aGroup, aBrowsingContextId, aType, std::move(aInit)), mProcessId(aOwnerProcessId), - mEmbedderProcessId(aEmbedderProcessId) { + mEmbedderProcessId(aEmbedderProcessId), + mPermanentKey(JS::NullValue()) { // You are only ever allowed to create CanonicalBrowsingContexts in the // parent process. MOZ_RELEASE_ASSERT(XRE_IsParentProcess()); @@ -86,6 +87,12 @@ CanonicalBrowsingContext::CanonicalBrowsingContext(WindowContext* aParentWindow, // The initial URI in a BrowsingContext is always "about:blank". MOZ_ALWAYS_SUCCEEDS( NS_NewURI(getter_AddRefs(mCurrentRemoteURI), "about:blank")); + + mozilla::HoldJSObjects(this); +} + +CanonicalBrowsingContext::~CanonicalBrowsingContext() { + mozilla::DropJSObjects(this); } /* static */ @@ -288,6 +295,9 @@ void CanonicalBrowsingContext::ReplacedBy( mLoadingEntries.SwapElements(aNewContext->mLoadingEntries); MOZ_ASSERT(!aNewContext->mActiveEntry); mActiveEntry.swap(aNewContext->mActiveEntry); + + aNewContext->mPermanentKey = mPermanentKey; + mPermanentKey = JS::NullValue(); } void CanonicalBrowsingContext::UpdateSecurityState() { @@ -1076,6 +1086,8 @@ void CanonicalBrowsingContext::CanonicalDiscard() { BackgroundSessionStorageManager::RemoveManager(Id()); } + mPermanentKey = JS::NullValue(); + CancelSessionStoreUpdate(); } @@ -1765,6 +1777,19 @@ CanonicalBrowsingContext::ChangeRemoteness( return promise.forget(); } +void CanonicalBrowsingContext::MaybeSetPermanentKey(Element* aEmbedder) { + MOZ_DIAGNOSTIC_ASSERT(IsTop()); + + if (aEmbedder) { + if (nsCOMPtr browser = aEmbedder->AsBrowser()) { + JS::RootedValue key(RootingCx()); + if (NS_SUCCEEDED(browser->GetPermanentKey(&key)) && key.isObject()) { + mPermanentKey = key; + } + } + } +} + MediaController* CanonicalBrowsingContext::GetMediaController() { // We would only create one media controller per tab, so accessing the // controller via the top-level browsing context. @@ -2083,17 +2108,6 @@ void CanonicalBrowsingContext::RestoreState::Resolve() { nsresult CanonicalBrowsingContext::WriteSessionStorageToSessionStore( const nsTArray& aSesssionStorage, uint32_t aEpoch) { - RefPtr windowParent = GetCurrentWindowGlobal(); - - if (!windowParent) { - return NS_OK; - } - - Element* frameElement = windowParent->GetRootOwnerElement(); - if (!frameElement) { - return NS_OK; - } - nsCOMPtr funcs = do_ImportModule("resource://gre/modules/SessionStoreFunctions.jsm"); if (!funcs) { @@ -2119,8 +2133,10 @@ nsresult CanonicalBrowsingContext::WriteSessionStorageToSessionStore( update.setNull(); } - return funcs->UpdateSessionStoreForStorage(frameElement, this, aEpoch, - update); + JS::RootedValue key(jsapi.cx(), Top()->PermanentKey()); + + return funcs->UpdateSessionStoreForStorage(Top()->GetEmbedderElement(), this, + key, aEpoch, update); } void CanonicalBrowsingContext::UpdateSessionStoreSessionStorage( @@ -2407,10 +2423,28 @@ void CanonicalBrowsingContext::SetTouchEventsOverride( SetTouchEventsOverrideInternal(aOverride, aRv); } -NS_IMPL_CYCLE_COLLECTION_INHERITED(CanonicalBrowsingContext, BrowsingContext, - mSessionHistory, mContainerFeaturePolicy, - mCurrentBrowserParent, mWebProgress, - mSessionStoreSessionStorageUpdateTimer) +NS_IMPL_CYCLE_COLLECTION_CLASS(CanonicalBrowsingContext) + +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(CanonicalBrowsingContext, + BrowsingContext) + tmp->mPermanentKey.setNull(); + + NS_IMPL_CYCLE_COLLECTION_UNLINK(mSessionHistory, mContainerFeaturePolicy, + mCurrentBrowserParent, mWebProgress, + mSessionStoreSessionStorageUpdateTimer) +NS_IMPL_CYCLE_COLLECTION_UNLINK_END + +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(CanonicalBrowsingContext, + BrowsingContext) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSessionHistory, mContainerFeaturePolicy, + mCurrentBrowserParent, mWebProgress, + mSessionStoreSessionStorageUpdateTimer) +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END + +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(CanonicalBrowsingContext, + BrowsingContext) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mPermanentKey) +NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_ADDREF_INHERITED(CanonicalBrowsingContext, BrowsingContext) NS_IMPL_RELEASE_INHERITED(CanonicalBrowsingContext, BrowsingContext) diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h index cc30d65654d2..e69ac878514c 100644 --- a/docshell/base/CanonicalBrowsingContext.h +++ b/docshell/base/CanonicalBrowsingContext.h @@ -67,8 +67,8 @@ struct RemotenessChangeOptions { class CanonicalBrowsingContext final : public BrowsingContext { public: NS_DECL_ISUPPORTS_INHERITED - NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(CanonicalBrowsingContext, - BrowsingContext) + NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED( + CanonicalBrowsingContext, BrowsingContext) static already_AddRefed Get(uint64_t aId); static CanonicalBrowsingContext* Cast(BrowsingContext* aContext); @@ -327,6 +327,9 @@ class CanonicalBrowsingContext final : public BrowsingContext { bool IsReplaced() const { return mIsReplaced; } + const JS::Heap& PermanentKey() { return mPermanentKey; } + void MaybeSetPermanentKey(Element* aEmbedder); + protected: // Called when the browsing context is being discarded. void CanonicalDiscard(); @@ -342,7 +345,7 @@ class CanonicalBrowsingContext final : public BrowsingContext { private: friend class BrowsingContext; - ~CanonicalBrowsingContext() = default; + virtual ~CanonicalBrowsingContext(); class PendingRemotenessChange { public: @@ -482,6 +485,8 @@ class CanonicalBrowsingContext final : public BrowsingContext { nsCOMPtr mSessionStoreSessionStorageUpdateTimer; bool mIsReplaced = false; + + JS::Heap mPermanentKey; }; } // namespace dom diff --git a/dom/interfaces/base/nsIBrowser.idl b/dom/interfaces/base/nsIBrowser.idl index d107e7d47eba..973a9244b8f8 100644 --- a/dom/interfaces/base/nsIBrowser.idl +++ b/dom/interfaces/base/nsIBrowser.idl @@ -51,6 +51,12 @@ interface nsIBrowser : nsISupports */ readonly attribute boolean isRemoteBrowser; + /** + * The browser's permanent key. This was added temporarily for Session Store, + * and will be removed in bug 1716788. + */ + readonly attribute jsval permanentKey; + readonly attribute nsIPrincipal contentPrincipal; readonly attribute nsIPrincipal contentPartitionedPrincipal; readonly attribute nsIContentSecurityPolicy csp; diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 4cf1bf35ac89..fd8ffc248ea5 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -2950,18 +2950,26 @@ mozilla::ipc::IPCResult BrowserParent::RecvSessionStoreUpdate( nsCOMPtr funcs = do_ImportModule("resource://gre/modules/SessionStoreFunctions.jsm"); - NS_ENSURE_TRUE(funcs, IPC_OK()); + if (!funcs) { + return IPC_OK(); + } + nsCOMPtr wrapped = do_QueryInterface(funcs); AutoJSAPI jsapi; - MOZ_ALWAYS_TRUE(jsapi.Init(wrapped->GetJSObjectGlobal())); - JS::Rooted dataVal(jsapi.cx()); - bool ok = ToJSValue(jsapi.cx(), data, &dataVal); - NS_ENSURE_TRUE(ok, IPC_OK()); + if (!jsapi.Init(wrapped->GetJSObjectGlobal())) { + return IPC_OK(); + } - nsresult rv = funcs->UpdateSessionStore( - mFrameElement, mBrowsingContext, aEpoch, aNeedCollectSHistory, dataVal); + JS::Rooted update(jsapi.cx()); + if (!ToJSValue(jsapi.cx(), data, &update)) { + return IPC_OK(); + } - NS_ENSURE_SUCCESS(rv, IPC_OK()); + JS::RootedValue key(jsapi.cx(), + mBrowsingContext->Canonical()->Top()->PermanentKey()); + + Unused << funcs->UpdateSessionStore(mFrameElement, mBrowsingContext, key, + aEpoch, aNeedCollectSHistory, update); return IPC_OK(); } diff --git a/dom/ipc/WindowGlobalParent.cpp b/dom/ipc/WindowGlobalParent.cpp index 02191bf48658..65b6ed4f2af9 100644 --- a/dom/ipc/WindowGlobalParent.cpp +++ b/dom/ipc/WindowGlobalParent.cpp @@ -1247,8 +1247,8 @@ nsresult WindowGlobalParent::WriteFormDataAndScrollToSessionStore( return NS_OK; } - Element* frameElement = GetRootOwnerElement(); - if (!frameElement) { + RefPtr context = BrowsingContext(); + if (!context) { return NS_OK; } @@ -1266,7 +1266,7 @@ nsresult WindowGlobalParent::WriteFormDataAndScrollToSessionStore( RootedDictionary windowState(jsapi.cx()); - if (GetPath(GetBrowsingContext(), windowState.mPath)) { + if (GetPath(context, windowState.mPath)) { // If a context in the parent chain from the current context is // missing, do nothing. return NS_OK; @@ -1284,21 +1284,22 @@ nsresult WindowGlobalParent::WriteFormDataAndScrollToSessionStore( } } - windowState.mHasChildren.Construct() = - !GetBrowsingContext()->Children().IsEmpty(); + windowState.mHasChildren.Construct() = !context->Children().IsEmpty(); JS::RootedValue update(jsapi.cx()); if (!ToJSValue(jsapi.cx(), windowState, &update)) { return NS_ERROR_FAILURE; } - return funcs->UpdateSessionStoreForWindow(frameElement, GetBrowsingContext(), + JS::RootedValue key(jsapi.cx(), context->Top()->PermanentKey()); + + return funcs->UpdateSessionStoreForWindow(GetRootOwnerElement(), context, key, aEpoch, update); } nsresult WindowGlobalParent::ResetSessionStore(uint32_t aEpoch) { - Element* frameElement = GetRootOwnerElement(); - if (!frameElement) { + RefPtr context = BrowsingContext(); + if (!context) { return NS_OK; } @@ -1316,7 +1317,7 @@ nsresult WindowGlobalParent::ResetSessionStore(uint32_t aEpoch) { RootedDictionary windowState(jsapi.cx()); - if (GetPath(GetBrowsingContext(), windowState.mPath)) { + if (GetPath(context, windowState.mPath)) { // If a context in the parent chain from the current context is // missing, do nothing. return NS_OK; @@ -1331,7 +1332,9 @@ nsresult WindowGlobalParent::ResetSessionStore(uint32_t aEpoch) { return NS_ERROR_FAILURE; } - return funcs->UpdateSessionStoreForWindow(frameElement, GetBrowsingContext(), + JS::RootedValue key(jsapi.cx(), context->Top()->PermanentKey()); + + return funcs->UpdateSessionStoreForWindow(GetRootOwnerElement(), context, key, aEpoch, update); } diff --git a/toolkit/components/sessionstore/SessionStoreFunctions.idl b/toolkit/components/sessionstore/SessionStoreFunctions.idl index bc06a35ecc42..36445c9649de 100644 --- a/toolkit/components/sessionstore/SessionStoreFunctions.idl +++ b/toolkit/components/sessionstore/SessionStoreFunctions.idl @@ -14,13 +14,14 @@ interface nsISessionStoreFunctions : nsISupports { // aData is a UpdateSessionStoreData dictionary (From SessionStoreUtils.webidl) void UpdateSessionStore( in Element aBrowser, in BrowsingContext aBrowsingContext, - in uint32_t aEpoch, in boolean aCollectSHistory, in jsval aData); + in jsval aPermanentKey, in uint32_t aEpoch, in boolean aCollectSHistory, + in jsval aData); void UpdateSessionStoreForWindow( in Element aBrowser, in BrowsingContext aBrowsingContext, - in uint32_t aEpoch, in jsval aData); + in jsval aPermanentKey, in uint32_t aEpoch, in jsval aData); void UpdateSessionStoreForStorage( in Element aBrowser, in BrowsingContext aBrowsingContext, - in uint32_t aEpoch, in jsval aData); + in jsval aPermanentKey, in uint32_t aEpoch, in jsval aData); }; diff --git a/toolkit/components/sessionstore/SessionStoreFunctions.jsm b/toolkit/components/sessionstore/SessionStoreFunctions.jsm index 70dc27fc7890..910ee73da166 100644 --- a/toolkit/components/sessionstore/SessionStoreFunctions.jsm +++ b/toolkit/components/sessionstore/SessionStoreFunctions.jsm @@ -13,6 +13,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { function UpdateSessionStore( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aCollectSHistory, aData @@ -20,6 +21,7 @@ function UpdateSessionStore( return SessionStoreFuncInternal.updateSessionStore( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aCollectSHistory, aData @@ -29,12 +31,14 @@ function UpdateSessionStore( function UpdateSessionStoreForWindow( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aData ) { return SessionStoreFuncInternal.updateSessionStoreForWindow( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aData ); @@ -43,12 +47,14 @@ function UpdateSessionStoreForWindow( function UpdateSessionStoreForStorage( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aData ) { return SessionStoreFuncInternal.updateSessionStoreForStorage( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aData ); @@ -64,6 +70,7 @@ var SessionStoreFuncInternal = { updateSessionStore: function SSF_updateSessionStore( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aCollectSHistory, aData @@ -76,36 +83,45 @@ var SessionStoreFuncInternal = { currentData.isPrivate = aData.isPrivate; } - SessionStore.updateSessionStoreFromTablistener(aBrowser, aBrowsingContext, { - data: currentData, - epoch: aEpoch, - sHistoryNeeded: aCollectSHistory, - }); + SessionStore.updateSessionStoreFromTablistener( + aBrowser, + aBrowsingContext, + aPermanentKey, + { + data: currentData, + epoch: aEpoch, + sHistoryNeeded: aCollectSHistory, + } + ); }, updateSessionStoreForWindow: function SSF_updateSessionStoreForWindow( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aData ) { - let windowstatechange = aData; - - SessionStore.updateSessionStoreFromTablistener(aBrowser, aBrowsingContext, { - data: { windowstatechange }, - epoch: aEpoch, - }); + SessionStore.updateSessionStoreFromTablistener( + aBrowser, + aBrowsingContext, + aPermanentKey, + { data: { windowstatechange: aData }, epoch: aEpoch } + ); }, updateSessionStoreForStorage: function SSF_updateSessionStoreForWindow( aBrowser, aBrowsingContext, + aPermanentKey, aEpoch, aData ) { - SessionStore.updateSessionStoreFromTablistener(aBrowser, aBrowsingContext, { - data: { storage: aData }, - epoch: aEpoch, - }); + SessionStore.updateSessionStoreFromTablistener( + aBrowser, + aBrowsingContext, + aPermanentKey, + { data: { storage: aData }, epoch: aEpoch } + ); }, }; diff --git a/toolkit/components/sessionstore/SessionStoreListener.cpp b/toolkit/components/sessionstore/SessionStoreListener.cpp index 68dd18f13492..1d5b14752dc8 100644 --- a/toolkit/components/sessionstore/SessionStoreListener.cpp +++ b/toolkit/components/sessionstore/SessionStoreListener.cpp @@ -463,7 +463,8 @@ bool TabListener::UpdateSessionStore(bool aIsFlush) { return false; } - if (!mOwnerContent) { + BrowsingContext* context = mDocShell->GetBrowsingContext(); + if (!context) { return false; } @@ -491,18 +492,30 @@ bool TabListener::UpdateSessionStore(bool aIsFlush) { nsCOMPtr funcs = do_ImportModule("resource://gre/modules/SessionStoreFunctions.jsm"); - NS_ENSURE_TRUE(funcs, false); + if (!funcs) { + return false; + } + nsCOMPtr wrapped = do_QueryInterface(funcs); AutoJSAPI jsapi; - MOZ_ALWAYS_TRUE(jsapi.Init(wrapped->GetJSObjectGlobal())); - JS::Rooted dataVal(jsapi.cx()); - bool ok = ToJSValue(jsapi.cx(), data, &dataVal); - NS_ENSURE_TRUE(ok, false); + if (!jsapi.Init(wrapped->GetJSObjectGlobal())) { + return false; + } + + JS::Rooted update(jsapi.cx()); + if (!ToJSValue(jsapi.cx(), data, &update)) { + return false; + } + + JS::RootedValue key(jsapi.cx(), context->Canonical()->Top()->PermanentKey()); nsresult rv = funcs->UpdateSessionStore( - mOwnerContent, mDocShell->GetBrowsingContext(), mEpoch, - mSessionStore->GetAndClearSHistoryChanged(), dataVal); - NS_ENSURE_SUCCESS(rv, false); + mOwnerContent, context, key, mEpoch, + mSessionStore->GetAndClearSHistoryChanged(), update); + if (NS_FAILED(rv)) { + return false; + } + StopTimerForUpdate(); return true; }