Bug 1593246 - Part 1: Move SessionStorageManager to BrowsingContext r=dom-workers-and-storage-reviewers,sg,smaug
☠☠ backed out by 922cb699c351 ☠ ☠
authorYaron Tausky <ytausky@mozilla.com>
Mon, 09 Dec 2019 13:04:51 +0000
changeset 506146 d9f0d827e28d957bdf6e3bb98818f74050e4a3f9
parent 506145 faba09a058503b02e2bfcdfc2cd383a1d65c2f56
child 506147 2663311a1b628c835b7c557d453797080c92d25d
push id102683
push userytausky@mozilla.com
push dateMon, 09 Dec 2019 18:14:29 +0000
treeherderautoland@fd389138a684 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-workers-and-storage-reviewers, sg, smaug
bugs1593246
milestone73.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 1593246 - Part 1: Move SessionStorageManager to BrowsingContext r=dom-workers-and-storage-reviewers,sg,smaug With Fission enabled we do not necessarily have access to the nsDocShell that holds the top-level browsing context, so the BrowsingContext is a better place to store information that needs to be accessible to nested browsing contexts. Differential Revision: https://phabricator.services.mozilla.com/D55276
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
dom/base/nsGlobalWindowInner.cpp
toolkit/components/sessionstore/SessionStoreUtils.cpp
toolkit/components/windowwatcher/nsWindowWatcher.cpp
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -14,16 +14,17 @@
 #include "mozilla/dom/ContentChild.h"
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Location.h"
 #include "mozilla/dom/LocationBinding.h"
 #include "mozilla/dom/PopupBlocker.h"
 #include "mozilla/dom/ScriptSettings.h"
+#include "mozilla/dom/SessionStorageManager.h"
 #include "mozilla/dom/StructuredCloneTags.h"
 #include "mozilla/dom/UserActivationIPCUtils.h"
 #include "mozilla/dom/WindowBinding.h"
 #include "mozilla/dom/WindowGlobalChild.h"
 #include "mozilla/dom/WindowGlobalParent.h"
 #include "mozilla/dom/WindowProxyHolder.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/ClearOnShutdown.h"
@@ -670,16 +671,24 @@ bool BrowsingContext::CanAccess(Browsing
     if (RefPtr<BrowsingContext> opener = aTarget->GetOpener()) {
       return CanAccess(opener, false);
     }
   }
 
   return false;
 }
 
+RefPtr<SessionStorageManager> BrowsingContext::SessionStorageManager() {
+  RefPtr<dom::SessionStorageManager>& manager = Top()->mSessionStorageManager;
+  if (!manager) {
+    manager = new dom::SessionStorageManager();
+  }
+  return manager;
+}
+
 BrowsingContext::~BrowsingContext() {
   MOZ_DIAGNOSTIC_ASSERT(!mParent || !mParent->mChildren.Contains(this));
   MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->Toplevels().Contains(this));
   MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->IsContextCached(this));
 
   mDeprioritizedLoadRunner.clear();
 
   if (sBrowsingContexts) {
@@ -807,26 +816,26 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Br
   if (tmp->mIsPopupSpam) {
     PopupBlocker::UnregisterOpenPopupSpam();
     // NOTE: Doesn't use SetIsPopupSpam, as it will be set all processes
     // automatically.
     tmp->mIsPopupSpam = false;
   }
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell, mChildren, mParent, mGroup,
-                                  mEmbedderElement)
+                                  mEmbedderElement, mSessionStorageManager)
   if (XRE_IsParentProcess()) {
     CanonicalBrowsingContext::Cast(tmp)->Unlink();
   }
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(BrowsingContext)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocShell, mChildren, mParent, mGroup,
-                                    mEmbedderElement)
+                                    mEmbedderElement, mSessionStorageManager)
   if (XRE_IsParentProcess()) {
     CanonicalBrowsingContext::Cast(tmp)->Traverse(cb);
   }
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 class RemoteLocationProxy
     : public RemoteObjectProxy<BrowsingContext::LocationProxy,
                                Location_Binding::sCrossOriginAttributes,
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -12,16 +12,17 @@
 #include "mozilla/Maybe.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/Tuple.h"
 #include "mozilla/WeakPtr.h"
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/LoadURIOptionsBinding.h"
 #include "mozilla/dom/LocationBase.h"
 #include "mozilla/dom/FeaturePolicyUtils.h"
+#include "mozilla/dom/SessionStorageManager.h"
 #include "mozilla/dom/UserActivation.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsID.h"
 #include "nsIDocShell.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "nsWrapperCache.h"
@@ -498,16 +499,18 @@ class BrowsingContext : public nsISuppor
   // Performs access control to check that 'this' can access 'aTarget'.
   bool CanAccess(BrowsingContext* aTarget, bool aConsiderOpener = true);
 
   // The runnable will be called once there is idle time, or the top level
   // page has been loaded or if a timeout has fired.
   // Must be called only on the top level BrowsingContext.
   void AddDeprioritizedLoadRunner(nsIRunnable* aRunner);
 
+  RefPtr<SessionStorageManager> SessionStorageManager();
+
  protected:
   virtual ~BrowsingContext();
   BrowsingContext(BrowsingContext* aParent, BrowsingContextGroup* aGroup,
                   uint64_t aBrowsingContextId, Type aType);
 
  private:
   // Find the special browsing context if aName is '_self', '_parent',
   // '_top', but not '_blank'. The latter is handled in FindWithName
@@ -648,16 +651,18 @@ class BrowsingContext : public nsISuppor
       return NS_OK;
     }
 
    private:
     RefPtr<nsIRunnable> mInner;
   };
 
   mozilla::LinkedList<DeprioritizedLoadRunner> mDeprioritizedLoadRunner;
+
+  RefPtr<dom::SessionStorageManager> mSessionStorageManager;
 };
 
 /**
  * Gets a WindowProxy object for a BrowsingContext that lives in a different
  * process (creating the object if it doesn't already exist). The WindowProxy
  * object will be in the compartment that aCx is currently in. This should only
  * be called if aContext doesn't hold a docshell, otherwise the BrowsingContext
  * lives in this process, and a same-process WindowProxy should be used (see
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -524,18 +524,17 @@ void nsDocShell::DestroyChildren() {
     if (shell) {
       shell->SetTreeOwner(nullptr);
     }
   }
 
   nsDocLoader::DestroyChildren();
 }
 
-NS_IMPL_CYCLE_COLLECTION_INHERITED(nsDocShell, nsDocLoader,
-                                   mSessionStorageManager, mScriptGlobal,
+NS_IMPL_CYCLE_COLLECTION_INHERITED(nsDocShell, nsDocLoader, mScriptGlobal,
                                    mInitialClientSource, mSessionHistory,
                                    mBrowsingContext, mChromeEventHandler)
 
 NS_IMPL_ADDREF_INHERITED(nsDocShell, nsDocLoader)
 NS_IMPL_RELEASE_INHERITED(nsDocShell, nsDocLoader)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsDocShell)
   NS_INTERFACE_MAP_ENTRY(nsIDocShell)
@@ -544,17 +543,16 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
   NS_INTERFACE_MAP_ENTRY(nsIBaseWindow)
   NS_INTERFACE_MAP_ENTRY(nsIScrollable)
   NS_INTERFACE_MAP_ENTRY(nsIRefreshURI)
   NS_INTERFACE_MAP_ENTRY(nsIWebProgressListener)
   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
   NS_INTERFACE_MAP_ENTRY(nsIWebPageDescriptor)
   NS_INTERFACE_MAP_ENTRY(nsIAuthPromptProvider)
   NS_INTERFACE_MAP_ENTRY(nsILoadContext)
-  NS_INTERFACE_MAP_ENTRY(nsIDOMStorageManager)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsINetworkInterceptController,
                                      mInterceptController)
   NS_INTERFACE_MAP_ENTRY(nsIDeprecationWarner)
 NS_INTERFACE_MAP_END_INHERITING(nsDocLoader)
 
 NS_IMETHODIMP
 nsDocShell::GetInterface(const nsIID& aIID, void** aSink) {
   MOZ_ASSERT(aSink, "null out param");
@@ -2243,41 +2241,16 @@ nsDocShell::GetWindowDraggingAllowed(boo
     // Top level chrome window
     *aValue = true;
   } else {
     *aValue = mWindowDraggingAllowed;
   }
   return NS_OK;
 }
 
-nsIDOMStorageManager* nsDocShell::TopSessionStorageManager() {
-  nsresult rv;
-
-  nsCOMPtr<nsIDocShellTreeItem> topItem;
-  rv = GetInProcessSameTypeRootTreeItem(getter_AddRefs(topItem));
-  if (NS_FAILED(rv)) {
-    return nullptr;
-  }
-
-  if (!topItem) {
-    return nullptr;
-  }
-
-  nsDocShell* topDocShell = static_cast<nsDocShell*>(topItem.get());
-  if (topDocShell != this) {
-    return topDocShell->TopSessionStorageManager();
-  }
-
-  if (!mSessionStorageManager) {
-    mSessionStorageManager = new SessionStorageManager();
-  }
-
-  return mSessionStorageManager;
-}
-
 NS_IMETHODIMP
 nsDocShell::GetCurrentDocumentChannel(nsIChannel** aResult) {
   NS_IF_ADDREF(*aResult = GetCurrentDocChannel());
   return NS_OK;
 }
 
 nsIChannel* nsDocShell::GetCurrentDocChannel() {
   if (mContentViewer) {
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -23,17 +23,16 @@
 #include "mozilla/dom/ChildSHistory.h"
 #include "mozilla/dom/WindowProxyHolder.h"
 
 #include "nsIAuthPromptProvider.h"
 #include "nsIBaseWindow.h"
 #include "nsIDeprecationWarner.h"
 #include "nsIDocShell.h"
 #include "nsIDocShellTreeItem.h"
-#include "nsIDOMStorageManager.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsILoadContext.h"
 #include "nsILoadURIDelegate.h"
 #include "nsINetworkInterceptController.h"
 #include "nsIRefreshURI.h"
 #include "nsIScrollable.h"
 #include "nsIWebNavigation.h"
 #include "nsIWebPageDescriptor.h"
@@ -118,17 +117,16 @@ class nsDocShell final : public nsDocLoa
                          public nsIWebNavigation,
                          public nsIBaseWindow,
                          public nsIScrollable,
                          public nsIRefreshURI,
                          public nsIWebProgressListener,
                          public nsIWebPageDescriptor,
                          public nsIAuthPromptProvider,
                          public nsILoadContext,
-                         public nsIDOMStorageManager,
                          public nsINetworkInterceptController,
                          public nsIDeprecationWarner,
                          public mozilla::SupportsWeakPtr<nsDocShell> {
  public:
   enum InternalLoad : uint32_t {
     INTERNAL_LOAD_FLAGS_NONE = 0x0,
     INTERNAL_LOAD_FLAGS_INHERIT_PRINCIPAL = 0x1,
     INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER = 0x2,
@@ -196,18 +194,16 @@ class nsDocShell final : public nsDocLoa
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSIWEBPROGRESSLISTENER
   NS_DECL_NSIREFRESHURI
   NS_DECL_NSIWEBPAGEDESCRIPTOR
   NS_DECL_NSIAUTHPROMPTPROVIDER
   NS_DECL_NSINETWORKINTERCEPTCONTROLLER
   NS_DECL_NSIDEPRECATIONWARNER
 
-  NS_FORWARD_SAFE_NSIDOMSTORAGEMANAGER(TopSessionStorageManager())
-
   // Create a new nsDocShell object, initializing it.
   static already_AddRefed<nsDocShell> Create(
       mozilla::dom::BrowsingContext* aBrowsingContext,
       uint64_t aContentWindowID = 0);
 
   NS_IMETHOD Stop() override {
     // Need this here because otherwise nsIWebNavigation::Stop
     // overrides the docloader's Stop()
@@ -988,17 +984,16 @@ class nsDocShell final : public nsDocLoa
   bool ShouldDiscardLayoutState(nsIHttpChannel* aChannel);
   bool HasUnloadedParent();
   bool JustStartedNetworkLoad();
   bool IsPrintingOrPP(bool aDisplayErrorDialog = true);
   bool IsNavigationAllowed(bool aDisplayPrintErrorDialog = true,
                            bool aCheckIfUnloadFired = true);
   uint32_t GetInheritedFrameType();
   nsIScrollableFrame* GetRootScrollFrame();
-  nsIDOMStorageManager* TopSessionStorageManager();
   nsIChannel* GetCurrentDocChannel();
   nsresult EnsureScriptEnvironment();
   nsresult EnsureEditorData();
   nsresult EnsureTransferableHookData();
   nsresult EnsureFind();
   nsresult EnsureCommandHandler();
   nsresult RefreshURIFromQueue();
   nsresult Embed(nsIContentViewer* aContentViewer,
@@ -1090,17 +1085,16 @@ class nsDocShell final : public nsDocLoa
   mozilla::UniquePtr<mozilla::dom::ClientSource> mInitialClientSource;
   nsCOMPtr<nsINetworkInterceptController> mInterceptController;
   RefPtr<nsDOMNavigationTiming> mTiming;
   RefPtr<nsDSURIContentListener> mContentListener;
   RefPtr<nsGlobalWindowOuter> mScriptGlobal;
   nsCOMPtr<nsIPrincipal> mParentCharsetPrincipal;
   nsCOMPtr<nsIMutableArray> mRefreshURIList;
   nsCOMPtr<nsIMutableArray> mSavedRefreshURIList;
-  nsCOMPtr<nsIDOMStorageManager> mSessionStorageManager;
   uint64_t mContentWindowID;
   nsCOMPtr<nsIContentViewer> mContentViewer;
   nsCOMPtr<nsIWidget> mParentWidget;
   RefPtr<mozilla::dom::ChildSHistory> mSessionHistory;
   nsCOMPtr<nsIWebBrowserFind> mFind;
   RefPtr<nsCommandManager> mCommandManager;
   RefPtr<mozilla::dom::BrowsingContext> mBrowsingContext;
 
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -4309,19 +4309,19 @@ already_AddRefed<nsICSSDeclaration> nsGl
     ErrorResult& aError) {
   FORWARD_TO_OUTER_OR_THROW(GetComputedStyleHelperOuter,
                             (aElt, aPseudoElt, aDefaultStylesOnly), aError,
                             nullptr);
 }
 
 Storage* nsGlobalWindowInner::GetSessionStorage(ErrorResult& aError) {
   nsIPrincipal* principal = GetPrincipal();
-  nsIDocShell* docShell = GetDocShell();
-
-  if (!principal || !docShell || !Storage::StoragePrefIsEnabled()) {
+  BrowsingContext* browsingContext = GetBrowsingContext();
+
+  if (!principal || !browsingContext || !Storage::StoragePrefIsEnabled()) {
     return nullptr;
   }
 
   if (mSessionStorage) {
     MOZ_LOG(gDOMLeakPRLogInner, LogLevel::Debug,
             ("nsGlobalWindowInner %p has %p sessionStorage", this,
              mSessionStorage.get()));
     bool canAccess = principal->Subsumes(mSessionStorage->Principal());
@@ -4391,22 +4391,20 @@ Storage* nsGlobalWindowInner::GetSession
     // The rationale is similar for the 3rd case.
     if (access == StorageAccess::eDeny &&
         rejectedReason !=
             nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN) {
       aError.Throw(NS_ERROR_DOM_SECURITY_ERR);
       return nullptr;
     }
 
-    nsresult rv;
-
-    nsCOMPtr<nsIDOMStorageManager> storageManager =
-        do_QueryInterface(docShell, &rv);
-    if (NS_FAILED(rv)) {
-      aError.Throw(rv);
+    const RefPtr<SessionStorageManager> storageManager =
+        browsingContext->SessionStorageManager();
+    if (!storageManager) {
+      aError.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
       return nullptr;
     }
 
     RefPtr<Storage> storage;
     // No StoragePrincipal for sessions.
     aError = storageManager->CreateStorage(this, principal, principal,
                                            documentURI, IsPrivateBrowsing(),
                                            getter_AddRefs(storage));
@@ -4984,19 +4982,18 @@ void nsGlobalWindowInner::ObserveStorage
   eventType.AssignLiteral("storage");
 
   if (!NS_strcmp(aStorageType, u"sessionStorage")) {
     RefPtr<Storage> changingStorage = aEvent->GetStorageArea();
     MOZ_ASSERT(changingStorage);
 
     bool check = false;
 
-    nsCOMPtr<nsIDOMStorageManager> storageManager =
-        do_QueryInterface(GetDocShell());
-    if (storageManager) {
+    if (const RefPtr<SessionStorageManager> storageManager =
+            GetBrowsingContext()->SessionStorageManager()) {
       nsresult rv =
           storageManager->CheckStorage(principal, changingStorage, &check);
       if (NS_FAILED(rv)) {
         return;
       }
     }
 
     if (!check) {
--- a/toolkit/components/sessionstore/SessionStoreUtils.cpp
+++ b/toolkit/components/sessionstore/SessionStoreUtils.cpp
@@ -1075,18 +1075,18 @@ bool SessionStoreUtils::RestoreFormData(
   return true;
 }
 
 /* Read entries in the session storage data contained in a tab's history. */
 static void ReadAllEntriesFromStorage(nsPIDOMWindowOuter* aWindow,
                                       nsTArray<nsCString>& aOrigins,
                                       nsTArray<nsString>& aKeys,
                                       nsTArray<nsString>& aValues) {
-  nsCOMPtr<nsIDocShell> docShell = aWindow->GetDocShell();
-  if (!docShell) {
+  BrowsingContext* const browsingContext = aWindow->GetBrowsingContext();
+  if (!browsingContext) {
     return;
   }
 
   Document* doc = aWindow->GetDoc();
   if (!doc) {
     return;
   }
 
@@ -1103,17 +1103,18 @@ static void ReadAllEntriesFromStorage(ns
   nsAutoCString origin;
   nsresult rv = principal->GetOrigin(origin);
   if (NS_FAILED(rv) || aOrigins.Contains(origin)) {
     // Don't read a host twice.
     return;
   }
 
   /* Completed checking for recursion and is about to read storage*/
-  nsCOMPtr<nsIDOMStorageManager> storageManager = do_QueryInterface(docShell);
+  const RefPtr<SessionStorageManager> storageManager =
+      browsingContext->SessionStorageManager();
   if (!storageManager) {
     return;
   }
   RefPtr<Storage> storage;
   storageManager->GetStorage(aWindow->GetCurrentInnerWindow(), principal,
                              storagePrincipal, false, getter_AddRefs(storage));
   if (!storage) {
     return;
@@ -1204,20 +1205,24 @@ void SessionStoreUtils::RestoreSessionSt
     //
     // If changing this logic, make sure to also change the principal
     // computation logic in SessionStore::_sendRestoreHistory.
 
     // OriginAttributes are always after a '^' character
     int32_t pos = entry.mKey.RFindChar('^');
     nsCOMPtr<nsIPrincipal> principal = BasePrincipal::CreateContentPrincipal(
         NS_ConvertUTF16toUTF8(Substring(entry.mKey, 0, pos)));
-    nsresult rv;
-    nsCOMPtr<nsIDOMStorageManager> storageManager =
-        do_QueryInterface(aDocShell, &rv);
-    if (NS_FAILED(rv)) {
+    BrowsingContext* const browsingContext =
+        nsDocShell::Cast(aDocShell)->GetBrowsingContext();
+    if (!browsingContext) {
+      return;
+    }
+    const RefPtr<SessionStorageManager> storageManager =
+        browsingContext->SessionStorageManager();
+    if (!storageManager) {
       return;
     }
     RefPtr<Storage> storage;
     // There is no need to pass documentURI, it's only used to fill documentURI
     // property of domstorage event, which in this case has no consumer.
     // Prevention of events in case of missing documentURI will be solved in a
     // followup bug to bug 600307.
     // Null window because the current window doesn't match the principal yet
--- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp
+++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp
@@ -1231,19 +1231,20 @@ nsresult nsWindowWatcher::OpenWindowInte
 
     // Should this pay attention to errors returned by LoadURI?
     newBC->LoadURI(parentBC, loadState);
   }
 
   // Copy the current session storage for the current domain. Don't perform the
   // copy if we're forcing noopener, however.
   if (!aForceNoOpener && subjectPrincipal && parentDocShell) {
-    nsCOMPtr<nsIDOMStorageManager> parentStorageManager =
-        do_QueryInterface(parentDocShell);
-    nsCOMPtr<nsIDOMStorageManager> newStorageManager(newDocShell);
+    const RefPtr<SessionStorageManager> parentStorageManager =
+        parentDocShell->GetBrowsingContext()->SessionStorageManager();
+    const RefPtr<SessionStorageManager> newStorageManager =
+        newDocShell->GetBrowsingContext()->SessionStorageManager();
 
     if (parentStorageManager && newStorageManager) {
       RefPtr<Storage> storage;
       nsCOMPtr<nsPIDOMWindowInner> pInnerWin =
           parentWindow->GetCurrentInnerWindow();
       parentStorageManager->GetStorage(
           pInnerWin, subjectPrincipal, subjectPrincipal,
           isPrivateBrowsingWindow, getter_AddRefs(storage));