Bug 1646563 - Remove BrowserId from nsFrameLoaderOwner, r=nika

We can just use BrowsingContext::BrowserId directly, so it's unnecessary to have
the field on nsFrameLoaderOwner as well.

This also makes it so that we only ever generate browser IDs in
BrowsingContext::CreatedDetached.

Differential Revision: https://phabricator.services.mozilla.com/D80121

docshell/base/BrowsingContext.cpp

@@ -189,13 +189,11 @@ bool BrowsingContext::SameOriginWithTop() {
 /* static */
 already_AddRefed<BrowsingContext> BrowsingContext::CreateDetached(
     nsGlobalWindowInner* aParent, BrowsingContext* aOpener,
-    const nsAString& aName, Type aType, uint64_t aBrowserId) {
+    const nsAString& aName, Type aType) {
   if (aParent) {
     MOZ_DIAGNOSTIC_ASSERT(aParent->GetWindowContext());
     MOZ_DIAGNOSTIC_ASSERT(aParent->GetBrowsingContext()->mType == aType);
-    MOZ_DIAGNOSTIC_ASSERT(aParent->GetBrowsingContext()->GetBrowserId() == 0 ||
-                          aParent->GetBrowsingContext()->GetBrowserId() ==
-                              aBrowserId);
+    MOZ_DIAGNOSTIC_ASSERT(aParent->GetBrowsingContext()->GetBrowserId() != 0);
   }
 
   MOZ_DIAGNOSTIC_ASSERT(aType != Type::Chrome || XRE_IsParentProcess());
@@ -251,7 +249,9 @@ already_AddRefed<BrowsingContext> BrowsingContext::CreateDetached(
   context->mFields.SetWithoutSyncing<IDX_OpenerPolicy>(
       nsILoadInfo::OPENER_POLICY_UNSAFE_NONE);
 
-  context->mFields.SetWithoutSyncing<IDX_BrowserId>(aBrowserId);
+  uint64_t browserId =
+      parentBC ? parentBC->GetBrowserId() : nsContentUtils::GenerateBrowserId();
+  context->mFields.SetWithoutSyncing<IDX_BrowserId>(browserId);
 
   if (aOpener && aOpener->SameOriginWithTop()) {
     // We inherit the opener policy if there is a creator and if the creator's
@@ -336,10 +336,8 @@ already_AddRefed<BrowsingContext> BrowsingContext::CreateDetached(
 
 already_AddRefed<BrowsingContext> BrowsingContext::CreateIndependent(
     Type aType) {
-  uint64_t browserId =
-      aType == Type::Content ? nsContentUtils::GenerateBrowserId() : 0;
   RefPtr<BrowsingContext> bc(
-      CreateDetached(nullptr, nullptr, EmptyString(), aType, browserId));
+      CreateDetached(nullptr, nullptr, EmptyString(), aType));
   bc->mWindowless = bc->IsContent();
   bc->EnsureAttached();
   return bc.forget();
@@ -511,29 +509,6 @@ static bool OwnerAllowsFullscreen(const Element& aEmbedder) {
 void BrowsingContext::SetEmbedderElement(Element* aEmbedder) {
   mEmbeddedByThisProcess = true;
 
-  // Update the browser ID on the embedder if necessary. We currently don't care
-  // about browser IDs for chrome-type BrowsingContexts.
-  if (RefPtr<nsFrameLoaderOwner> owner = do_QueryObject(aEmbedder);
-      owner && !IsChrome()) {
-    uint64_t browserId = GetBrowserId();
-    uint64_t frameBrowserId = owner->GetBrowserId();
-
-    MOZ_DIAGNOSTIC_ASSERT(browserId != 0);
-
-    if (frameBrowserId == 0) {
-      // We'll arrive here if we're a top-level BrowsingContext for a window
-      // or tab that was opened in a content process. There should be no
-      // children to update at this point. This ID was generated in
-      // ContentChild::ProvideWindowCommon.
-      MOZ_DIAGNOSTIC_ASSERT(IsTopContent());
-      MOZ_DIAGNOSTIC_ASSERT(Children().IsEmpty());
-      owner->SetBrowserId(browserId);
-    } else {
-      // We would've inherited or generated an ID in CreateBrowsingContext.
-      MOZ_DIAGNOSTIC_ASSERT(browserId == frameBrowserId);
-    }
-  }
-
   // Update embedder-element-specific fields in a shared transaction.
   // Don't do this when clearing our embedder, as we're being destroyed either
   // way.

docshell/base/BrowsingContext.h

@@ -205,7 +205,7 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
   // DocShell, BrowserParent, or BrowserBridgeChild.
   static already_AddRefed<BrowsingContext> CreateDetached(
       nsGlobalWindowInner* aParent, BrowsingContext* aOpener,
-      const nsAString& aName, Type aType, uint64_t aBrowserId);
+      const nsAString& aName, Type aType);
 
   void EnsureAttached();
 

docshell/base/CanonicalBrowsingContext.cpp

@@ -144,11 +144,13 @@ void CanonicalBrowsingContext::MaybeAddAsProgressListener(
 
 void CanonicalBrowsingContext::ReplacedBy(
     CanonicalBrowsingContext* aNewContext) {
+  MOZ_ASSERT(!aNewContext->EverAttached());
   if (mStatusFilter) {
     mStatusFilter->RemoveProgressListener(mWebProgress);
     mStatusFilter = nullptr;
   }
   aNewContext->mWebProgress = std::move(mWebProgress);
+  aNewContext->mFields.SetWithoutSyncing<IDX_BrowserId>(GetBrowserId());
 }
 
 void CanonicalBrowsingContext::

dom/base/nsFrameLoader.cpp

@@ -297,10 +297,6 @@ static already_AddRefed<BrowsingContext> CreateBrowsingContext(
   nsAutoString frameName;
   GetFrameName(aOwner, frameName);
 
-  // By default we just use the same browser ID as the parent.
-  uint64_t browserId = parentBC->GetBrowserId();
-  RefPtr<nsFrameLoaderOwner> owner = do_QueryObject(aOwner);
-
   // Create our BrowsingContext without immediately attaching it. It's possible
   // that no DocShell or remote browser will ever be created for this
   // FrameLoader, particularly if the document that we were created for is not
@@ -308,38 +304,16 @@ static already_AddRefed<BrowsingContext> CreateBrowsingContext(
   // it will wind up attached as a child of the currently active inner window
   // for the BrowsingContext, and cause no end of trouble.
   if (IsTopContent(parentBC, aOwner)) {
-    if (owner && owner->GetBrowserId() != 0) {
-      // This frame has already been assigned an ID. This can happen for example
-      // if a frame is re-inserted into the DOM (i.e. on a remoteness change).
-      browserId = owner->GetBrowserId();
-
-      // This implies that we do not support changing a frame's "type"
-      // attribute. Doing so would mean needing to change the browser ID for the
-      // frame and the intent is to never change this.
-      MOZ_DIAGNOSTIC_ASSERT(browserId != parentBC->GetBrowserId());
-    } else {
-      browserId = nsContentUtils::GenerateBrowserId();
-      if (owner) {
-        owner->SetBrowserId(browserId);
-      }
-    }
-
-    // Create toplevel content without a parent & as Type::Content.
-    return BrowsingContext::CreateDetached(
-        nullptr, opener, frameName, BrowsingContext::Type::Content, browserId);
+    // Create toplevel context without a parent & as Type::Content.
+    return BrowsingContext::CreateDetached(nullptr, opener, frameName,
+                                           BrowsingContext::Type::Content);
   }
 
   MOZ_ASSERT(!aOpenWindowInfo,
              "Can't have openWindowInfo for non-toplevel context");
 
-  if (owner) {
-    MOZ_DIAGNOSTIC_ASSERT(owner->GetBrowserId() == 0 ||
-                          owner->GetBrowserId() == browserId);
-    owner->SetBrowserId(browserId);
-  }
-
   return BrowsingContext::CreateDetached(parentInner, nullptr, frameName,
-                                         parentBC->GetType(), browserId);
+                                         parentBC->GetType());
 }
 
 static bool InitialLoadIsRemote(Element* aOwner) {
@@ -1332,13 +1306,6 @@ nsresult nsFrameLoader::SwapWithOtherRemoteLoader(
   MaybeUpdatePrimaryBrowserParent(eBrowserParentRemoved);
   aOther->MaybeUpdatePrimaryBrowserParent(eBrowserParentRemoved);
 
-  // When embedding the frame in SetOwnerContent, we check that the
-  // BrowsingContext's browser ID matches that of the embedder element, so swap
-  // the IDs here.
-  uint64_t ourBrowserId = aThisOwner->GetBrowserId();
-  aThisOwner->SetBrowserId(aOtherOwner->GetBrowserId());
-  aOtherOwner->SetBrowserId(ourBrowserId);
-
   SetOwnerContent(otherContent);
   aOther->SetOwnerContent(ourContent);
 
@@ -1753,13 +1720,6 @@ nsresult nsFrameLoader::SwapWithOtherLoader(nsFrameLoader* aOther,
       otherDocshell, ourOwner,
       ourBc->IsContent() ? ourChromeEventHandler.get() : nullptr);
 
-  // When embedding the frame in SetOwnerContent, we check that the
-  // BrowsingContext's browser ID matches that of the embedder element, so swap
-  // the IDs here.
-  uint64_t ourBrowserId = aThisOwner->GetBrowserId();
-  aThisOwner->SetBrowserId(aOtherOwner->GetBrowserId());
-  aOtherOwner->SetBrowserId(ourBrowserId);
-
   // Switch the owner content before we start calling AddTreeItemToTreeOwner.
   // Note that we rely on this to deal with setting mObservingOwnerContent to
   // false and calling RemoveMutationObserver as needed.

dom/base/nsFrameLoaderOwner.cpp

@@ -284,9 +284,3 @@ void nsFrameLoaderOwner::SubframeCrashed() {
                          /* inProgress */ false,
                          /* isRemote */ false, frameLoaderInit, IgnoreErrors());
 }
-
-void nsFrameLoaderOwner::UnbindFromTree() {
-  // If we're being adopted into a different document, we'll want to inherit a
-  // browser ID from our new BrowsingContext, so clear our current ID here.
-  mBrowserId = 0;
-}

dom/base/nsFrameLoaderOwner.h

@@ -74,12 +74,6 @@ class nsFrameLoaderOwner : public nsISupports {
 
   void SubframeCrashed();
 
-  // Prepare for a frame to be removed from its current DOM tree.
-  void UnbindFromTree();
-
-  uint64_t GetBrowserId() { return mBrowserId; }
-  void SetBrowserId(uint64_t aBrowserId) { mBrowserId = aBrowserId; }
-
  private:
   bool UseRemoteSubframes();
 
@@ -101,8 +95,6 @@ class nsFrameLoaderOwner : public nsISupports {
                               std::function<void()>& aFrameLoaderInit,
                               mozilla::ErrorResult& aRv);
 
-  uint64_t mBrowserId = 0;
-
  protected:
   virtual ~nsFrameLoaderOwner() = default;
   RefPtr<nsFrameLoader> mFrameLoader;

dom/base/nsObjectLoadingContent.cpp

@@ -568,7 +568,6 @@ nsresult nsObjectLoadingContent::BindToTree(BindContext& aContext,
 }
 
 void nsObjectLoadingContent::UnbindFromTree(bool aNullParent) {
-  nsFrameLoaderOwner::UnbindFromTree();
   nsImageLoadingContent::UnbindFromTree(aNullParent);
 
   nsCOMPtr<Element> thisElement =

dom/html/nsGenericHTMLFrameElement.cpp

@@ -208,7 +208,6 @@ void nsGenericHTMLFrameElement::UnbindFromTree(bool aNullParent) {
     mFrameLoader = nullptr;
   }
 
-  nsFrameLoaderOwner::UnbindFromTree();
   nsGenericHTMLElement::UnbindFromTree(aNullParent);
 }
 

dom/ipc/ContentChild.cpp

@@ -949,9 +949,8 @@ nsresult ContentChild::ProvideWindowCommon(
     openerBC = parent;
   }
 
-  uint64_t browserId(nsContentUtils::GenerateBrowserId());
   RefPtr<BrowsingContext> browsingContext = BrowsingContext::CreateDetached(
-      nullptr, openerBC, aName, BrowsingContext::Type::Content, browserId);
+      nullptr, openerBC, aName, BrowsingContext::Type::Content);
   MOZ_ALWAYS_SUCCEEDS(browsingContext->SetRemoteTabs(true));
   MOZ_ALWAYS_SUCCEEDS(browsingContext->SetRemoteSubframes(useRemoteSubframes));
   MOZ_ALWAYS_SUCCEEDS(browsingContext->SetOriginAttributes(

dom/xul/XULFrameElement.cpp

@@ -69,6 +69,13 @@ Document* XULFrameElement::GetContentDocument() {
   return nullptr;
 }
 
+uint64_t XULFrameElement::BrowserId() {
+  if (auto* bc = mFrameLoader->GetExtantBrowsingContext()) {
+    return bc->GetBrowserId();
+  }
+  return 0;
+}
+
 void XULFrameElement::LoadSrc() {
   if (!IsInUncomposedDoc() || !OwnerDoc()->GetRootElement()) {
     return;
@@ -154,7 +161,6 @@ void XULFrameElement::UnbindFromTree(bool aNullParent) {
   }
   mFrameLoader = nullptr;
 
-  nsFrameLoaderOwner::UnbindFromTree();
   nsXULElement::UnbindFromTree(aNullParent);
 }
 

dom/xul/XULFrameElement.h

@@ -39,7 +39,7 @@ class XULFrameElement final : public nsXULElement, public nsFrameLoaderOwner {
   already_AddRefed<nsIWebNavigation> GetWebNavigation();
   Nullable<WindowProxyHolder> GetContentWindow();
   Document* GetContentDocument();
-  uint64_t BrowserId() { return GetBrowserId(); }
+  uint64_t BrowserId();
 
   void SwapFrameLoaders(mozilla::dom::HTMLIFrameElement& aOtherLoaderOwner,
                         mozilla::ErrorResult& rv);