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

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) {
+  updateSessionStoreFromTablistener(
-    if (aBrowser.permanentKey == undefined) {
+    aBrowser,
-      return;
+    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._browserSHistoryListener.get(browser.permanentKey) ??
-        this.addSHistoryListener(aBrowser, aBrowsingContext);
+        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);
   },
 
   /**

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) {

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<nsIBrowser> 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<SSCacheCopy>& aSesssionStorage, uint32_t aEpoch) {
-  RefPtr<WindowGlobalParent> windowParent = GetCurrentWindowGlobal();
-
-  if (!windowParent) {
-    return NS_OK;
-  }
-
-  Element* frameElement = windowParent->GetRootOwnerElement();
-  if (!frameElement) {
-    return NS_OK;
-  }
-
   nsCOMPtr<nsISessionStoreFunctions> 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,
+  JS::RootedValue key(jsapi.cx(), Top()->PermanentKey());
-                                             update);
+
+  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,
+NS_IMPL_CYCLE_COLLECTION_CLASS(CanonicalBrowsingContext)
-                                   mSessionHistory, mContainerFeaturePolicy,
+
-                                   mCurrentBrowserParent, mWebProgress,
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(CanonicalBrowsingContext,
-                                   mSessionStoreSessionStorageUpdateTimer)
+                                                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)

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,
+  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(
-                                           BrowsingContext)
+      CanonicalBrowsingContext, BrowsingContext)
 
   static already_AddRefed<CanonicalBrowsingContext> 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<JS::Value>& 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<nsITimer> mSessionStoreSessionStorageUpdateTimer;
 
   bool mIsReplaced = false;
+
+  JS::Heap<JS::Value> mPermanentKey;
 };
 
 }  // namespace dom

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;

dom/ipc/BrowserParent.cpp

@@ -2950,18 +2950,26 @@ mozilla::ipc::IPCResult BrowserParent::RecvSessionStoreUpdate(
 
   nsCOMPtr<nsISessionStoreFunctions> funcs =
       do_ImportModule("resource://gre/modules/SessionStoreFunctions.jsm");
-  NS_ENSURE_TRUE(funcs, IPC_OK());
+  if (!funcs) {
+    return IPC_OK();
+  }
+
   nsCOMPtr<nsIXPConnectWrappedJS> wrapped = do_QueryInterface(funcs);
   AutoJSAPI jsapi;
-  MOZ_ALWAYS_TRUE(jsapi.Init(wrapped->GetJSObjectGlobal()));
+  if (!jsapi.Init(wrapped->GetJSObjectGlobal())) {
-  JS::Rooted<JS::Value> dataVal(jsapi.cx());
+    return IPC_OK();
-  bool ok = ToJSValue(jsapi.cx(), data, &dataVal);
+  }
-  NS_ENSURE_TRUE(ok, IPC_OK());
 
-  nsresult rv = funcs->UpdateSessionStore(
+  JS::Rooted<JS::Value> update(jsapi.cx());
-      mFrameElement, mBrowsingContext, aEpoch, aNeedCollectSHistory, dataVal);
+  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();
 }

dom/ipc/WindowGlobalParent.cpp

@@ -1247,8 +1247,8 @@ nsresult WindowGlobalParent::WriteFormDataAndScrollToSessionStore(
     return NS_OK;
   }
 
-  Element* frameElement = GetRootOwnerElement();
+  RefPtr<CanonicalBrowsingContext> context = BrowsingContext();
-  if (!frameElement) {
+  if (!context) {
     return NS_OK;
   }
 
@@ -1266,7 +1266,7 @@ nsresult WindowGlobalParent::WriteFormDataAndScrollToSessionStore(
 
   RootedDictionary<SessionStoreWindowStateChange> 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() =
+  windowState.mHasChildren.Construct() = !context->Children().IsEmpty();
-      !GetBrowsingContext()->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();
+  RefPtr<CanonicalBrowsingContext> context = BrowsingContext();
-  if (!frameElement) {
+  if (!context) {
     return NS_OK;
   }
 
@@ -1316,7 +1317,7 @@ nsresult WindowGlobalParent::ResetSessionStore(uint32_t aEpoch) {
 
   RootedDictionary<SessionStoreWindowStateChange> 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);
 }
 

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);
 };

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, {
+    SessionStore.updateSessionStoreFromTablistener(
-      data: currentData,
+      aBrowser,
-      epoch: aEpoch,
+      aBrowsingContext,
-      sHistoryNeeded: aCollectSHistory,
+      aPermanentKey,
-    });
+      {
+        data: currentData,
+        epoch: aEpoch,
+        sHistoryNeeded: aCollectSHistory,
+      }
+    );
   },
 
   updateSessionStoreForWindow: function SSF_updateSessionStoreForWindow(
     aBrowser,
     aBrowsingContext,
+    aPermanentKey,
     aEpoch,
     aData
   ) {
-    let windowstatechange = aData;
+    SessionStore.updateSessionStoreFromTablistener(
-
+      aBrowser,
-    SessionStore.updateSessionStoreFromTablistener(aBrowser, aBrowsingContext, {
+      aBrowsingContext,
-      data: { windowstatechange },
+      aPermanentKey,
-      epoch: aEpoch,
+      { data: { windowstatechange: aData }, epoch: aEpoch }
-    });
+    );
   },
 
   updateSessionStoreForStorage: function SSF_updateSessionStoreForWindow(
     aBrowser,
     aBrowsingContext,
+    aPermanentKey,
     aEpoch,
     aData
   ) {
-    SessionStore.updateSessionStoreFromTablistener(aBrowser, aBrowsingContext, {
+    SessionStore.updateSessionStoreFromTablistener(
-      data: { storage: aData },
+      aBrowser,
-      epoch: aEpoch,
+      aBrowsingContext,
-    });
+      aPermanentKey,
+      { data: { storage: aData }, epoch: aEpoch }
+    );
   },
 };

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<nsISessionStoreFunctions> funcs =
       do_ImportModule("resource://gre/modules/SessionStoreFunctions.jsm");
-  NS_ENSURE_TRUE(funcs, false);
+  if (!funcs) {
+    return false;
+  }
+
   nsCOMPtr<nsIXPConnectWrappedJS> wrapped = do_QueryInterface(funcs);
   AutoJSAPI jsapi;
-  MOZ_ALWAYS_TRUE(jsapi.Init(wrapped->GetJSObjectGlobal()));
+  if (!jsapi.Init(wrapped->GetJSObjectGlobal())) {
-  JS::Rooted<JS::Value> dataVal(jsapi.cx());
+    return false;
-  bool ok = ToJSValue(jsapi.cx(), data, &dataVal);
+  }
-  NS_ENSURE_TRUE(ok, false);
+
+  JS::Rooted<JS::Value> 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,
+      mOwnerContent, context, key, mEpoch,
-      mSessionStore->GetAndClearSHistoryChanged(), dataVal);
+      mSessionStore->GetAndClearSHistoryChanged(), update);
-  NS_ENSURE_SUCCESS(rv, false);
+  if (NS_FAILED(rv)) {
+    return false;
+  }
+
   StopTimerForUpdate();
   return true;
 }