Bug 1303196 - Part 4: Merge mFrameLoader and mOpener into mFrameLoaderOrOpener, r=smaug
authorMichael Layzell <michael@thelayzells.com>
Mon, 17 Oct 2016 10:37:50 -0400
changeset 346556 3515eb43f05a25d02c45a013a5373284ff3fde94
parent 346555 7ee25aab52f5a7ff546313192df4ee1436e3b804
child 346557 11f778f3ea300696e3c900c9583f726809cd8359
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1303196
milestone52.0a1
Bug 1303196 - Part 4: Merge mFrameLoader and mOpener into mFrameLoaderOrOpener, r=smaug MozReview-Commit-ID: DuJRM5KsGhb
dom/base/nsFrameLoader.cpp
dom/base/nsFrameLoader.h
dom/base/nsIFrameLoader.idl
dom/base/nsObjectLoadingContent.cpp
dom/html/nsGenericHTMLFrameElement.cpp
dom/html/nsGenericHTMLFrameElement.h
dom/xul/nsXULElement.cpp
dom/xul/nsXULElement.h
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -129,29 +129,30 @@ typedef FrameMetrics::ViewID ViewID;
 // does not count chrome frames when determining depth, nor does it
 // prevent chrome recursion.  Number is fairly arbitrary, but meant to
 // keep number of shells to a reasonable number on accidental recursion with a
 // small (but not 1) branching factor.  With large branching factors the number
 // of shells can rapidly become huge and run us out of memory.  To solve that,
 // we'd need to re-institute a fixed version of bug 98158.
 #define MAX_DEPTH_CONTENT_FRAMES 10
 
-NS_IMPL_CYCLE_COLLECTION(nsFrameLoader, mDocShell, mMessageManager, mChildMessageManager)
+NS_IMPL_CYCLE_COLLECTION(nsFrameLoader, mDocShell, mMessageManager, mChildMessageManager, mOpener)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsFrameLoader)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsFrameLoader)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsFrameLoader)
   NS_INTERFACE_MAP_ENTRY(nsIFrameLoader)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIFrameLoader)
   NS_INTERFACE_MAP_ENTRY(nsIWebBrowserPersistable)
 NS_INTERFACE_MAP_END
 
-nsFrameLoader::nsFrameLoader(Element* aOwner, bool aNetworkCreated)
+nsFrameLoader::nsFrameLoader(Element* aOwner, nsPIDOMWindowOuter* aOpener, bool aNetworkCreated)
   : mOwnerContent(aOwner)
   , mDetachedSubdocFrame(nullptr)
+  , mOpener(aOpener)
   , mRemoteBrowser(nullptr)
   , mChildID(0)
   , mEventMode(EVENT_MODE_NORMAL_DISPATCH)
   , mIsPrerendered(false)
   , mDepthTooGreat(false)
   , mIsTopLevelContent(false)
   , mDestroyCalled(false)
   , mNeedsAsyncDestroy(false)
@@ -162,28 +163,30 @@ nsFrameLoader::nsFrameLoader(Element* aO
   , mRemoteBrowserShown(false)
   , mRemoteFrame(false)
   , mClipSubdocument(true)
   , mClampScrollPosition(true)
   , mObservingOwnerContent(false)
   , mVisible(true)
 {
   mRemoteFrame = ShouldUseRemoteProcess();
+  MOZ_ASSERT(!mRemoteFrame || !aOpener,
+             "Cannot pass aOpener for a remote frame!");
 }
 
 nsFrameLoader::~nsFrameLoader()
 {
   if (mMessageManager) {
     mMessageManager->Disconnect();
   }
   MOZ_RELEASE_ASSERT(mDestroyCalled);
 }
 
 nsFrameLoader*
-nsFrameLoader::Create(Element* aOwner, bool aNetworkCreated)
+nsFrameLoader::Create(Element* aOwner, nsPIDOMWindowOuter* aOpener, bool aNetworkCreated)
 {
   NS_ENSURE_TRUE(aOwner, nullptr);
   nsIDocument* doc = aOwner->OwnerDoc();
 
   // We never create nsFrameLoaders for elements in resource documents.
   //
   // We never create nsFrameLoaders for elements in data documents, unless the
   // document is a static document.
@@ -203,17 +206,17 @@ nsFrameLoader::Create(Element* aOwner, b
   // since for a static document we know aOwner will end up in a document and
   // the nsFrameLoader will be used for its docShell.)
   //
   NS_ENSURE_TRUE(!doc->IsResourceDoc() &&
                  ((!doc->IsLoadedAsData() && aOwner->IsInComposedDoc()) ||
                   doc->IsStaticDocument()),
                  nullptr);
 
-  return new nsFrameLoader(aOwner, aNetworkCreated);
+  return new nsFrameLoader(aOwner, aOpener, aNetworkCreated);
 }
 
 NS_IMETHODIMP
 nsFrameLoader::LoadFrame()
 {
   NS_ENSURE_TRUE(mOwnerContent, NS_ERROR_NOT_INITIALIZED);
 
   nsAutoString src;
@@ -913,21 +916,28 @@ nsFrameLoader::Hide()
   NS_ASSERTION(baseWin,
                "Found an nsIDocShell which doesn't implement nsIBaseWindow.");
   baseWin->SetVisibility(false);
   baseWin->SetParentWidget(nullptr);
 }
 
 nsresult
 nsFrameLoader::SwapWithOtherRemoteLoader(nsFrameLoader* aOther,
-                                         RefPtr<nsFrameLoader>& aFirstToSwap,
-                                         RefPtr<nsFrameLoader>& aSecondToSwap)
+                                         nsIFrameLoaderOwner* aThisOwner,
+                                         nsIFrameLoaderOwner* aOtherOwner)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
+#ifdef DEBUG
+  RefPtr<nsFrameLoader> first = aThisOwner->GetFrameLoader();
+  RefPtr<nsFrameLoader> second = aOtherOwner->GetFrameLoader();
+  MOZ_ASSERT(first == this, "aThisOwner must own this");
+  MOZ_ASSERT(second == aOther, "aOtherOwner must own aOther");
+#endif
+
   Element* ourContent = mOwnerContent;
   Element* otherContent = aOther->mOwnerContent;
 
   if (!ourContent || !otherContent) {
     // Can't handle this
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
@@ -1065,17 +1075,22 @@ nsFrameLoader::SwapWithOtherRemoteLoader
   if (ourMessageManager) {
     ourMessageManager->SetCallback(aOther);
   }
   if (otherMessageManager) {
     otherMessageManager->SetCallback(this);
   }
   mMessageManager.swap(aOther->mMessageManager);
 
-  aFirstToSwap.swap(aSecondToSwap);
+  // Perform the actual swap of the internal refptrs. We keep a strong reference
+  // to ourselves to make sure we don't die while we overwrite our reference to
+  // ourself.
+  nsCOMPtr<nsIFrameLoader> kungFuDeathGrip(this);
+  aThisOwner->InternalSetFrameLoader(aOther);
+  aOtherOwner->InternalSetFrameLoader(kungFuDeathGrip);
 
   ourFrameFrame->EndSwapDocShells(otherFrame);
 
   ourShell->BackingScaleFactorChanged();
   otherShell->BackingScaleFactorChanged();
 
   ourDoc->FlushPendingNotifications(Flush_Layout);
   otherDoc->FlushPendingNotifications(Flush_Layout);
@@ -1156,22 +1171,26 @@ private:
   RefPtr<nsDocShell> mOtherDocShell;
   nsCOMPtr<EventTarget> mThisEventTarget;
   nsCOMPtr<EventTarget> mOtherEventTarget;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 nsresult
 nsFrameLoader::SwapWithOtherLoader(nsFrameLoader* aOther,
-                                   RefPtr<nsFrameLoader>& aFirstToSwap,
-                                   RefPtr<nsFrameLoader>& aSecondToSwap)
+                                   nsIFrameLoaderOwner* aThisOwner,
+                                   nsIFrameLoaderOwner* aOtherOwner)
 {
-  NS_PRECONDITION((aFirstToSwap == this && aSecondToSwap == aOther) ||
-                  (aFirstToSwap == aOther && aSecondToSwap == this),
-                  "Swapping some sort of random loaders?");
+#ifdef DEBUG
+  RefPtr<nsFrameLoader> first = aThisOwner->GetFrameLoader();
+  RefPtr<nsFrameLoader> second = aOtherOwner->GetFrameLoader();
+  MOZ_ASSERT(first == this, "aThisOwner must own this");
+  MOZ_ASSERT(second == aOther, "aOtherOwner must own aOther");
+#endif
+
   NS_ENSURE_STATE(!mInShow && !aOther->mInShow);
 
   if (IsRemoteFrame() != aOther->IsRemoteFrame()) {
     NS_WARNING("Swapping remote and non-remote frames is not currently supported");
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   Element* ourContent = mOwnerContent;
@@ -1203,17 +1222,17 @@ nsFrameLoader::SwapWithOtherLoader(nsFra
        otherContent->HasAttr(kNameSpaceID_None, nsGkAtoms::mozallowfullscreen)));
   if (ourFullscreenAllowed != otherFullscreenAllowed) {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   // Divert to a separate path for the remaining steps in the remote case
   if (IsRemoteFrame()) {
     MOZ_ASSERT(aOther->IsRemoteFrame());
-    return SwapWithOtherRemoteLoader(aOther, aFirstToSwap, aSecondToSwap);
+    return SwapWithOtherRemoteLoader(aOther, aThisOwner, aOtherOwner);
   }
 
   // Make sure there are no same-origin issues
   bool equal;
   nsresult rv =
     ourContent->NodePrincipal()->Equals(otherContent->NodePrincipal(), &equal);
   if (NS_FAILED(rv) || !equal) {
     // Security problems loom.  Just bail on it all
@@ -1465,17 +1484,22 @@ nsFrameLoader::SwapWithOtherLoader(nsFra
   if (mMessageManager) {
     mMessageManager->SetCallback(aOther);
   }
   if (aOther->mMessageManager) {
     aOther->mMessageManager->SetCallback(this);
   }
   mMessageManager.swap(aOther->mMessageManager);
 
-  aFirstToSwap.swap(aSecondToSwap);
+  // Perform the actual swap of the internal refptrs. We keep a strong reference
+  // to ourselves to make sure we don't die while we overwrite our reference to
+  // ourself.
+  nsCOMPtr<nsIFrameLoader> kungFuDeathGrip(this);
+  aThisOwner->InternalSetFrameLoader(aOther);
+  aOtherOwner->InternalSetFrameLoader(kungFuDeathGrip);
 
   // Drop any cached content viewers in the two session histories.
   nsCOMPtr<nsISHistoryInternal> ourInternalHistory =
     do_QueryInterface(ourHistory);
   nsCOMPtr<nsISHistoryInternal> otherInternalHistory =
     do_QueryInterface(otherHistory);
   if (ourInternalHistory) {
     ourInternalHistory->EvictAllContentViewers();
@@ -2006,16 +2030,22 @@ nsFrameLoader::MaybeCreateDocShell()
   // Tell the window about the frame that hosts it.
   nsCOMPtr<Element> frame_element = mOwnerContent;
   NS_ASSERTION(frame_element, "frame loader owner element not a DOM element!");
 
   nsCOMPtr<nsPIDOMWindowOuter> win_private(mDocShell->GetWindow());
   nsCOMPtr<nsIBaseWindow> base_win(do_QueryInterface(mDocShell));
   if (win_private) {
     win_private->SetFrameElementInternal(frame_element);
+
+    // Set the opener window if we have one provided here
+    if (mOpener) {
+      win_private->SetOpenerWindow(mOpener, true);
+      mOpener = nullptr;
+    }
   }
 
   // This is kinda whacky, this call doesn't really create anything,
   // but it must be called to make sure things are properly
   // initialized.
   if (NS_FAILED(base_win->Create()) || !win_private) {
     // Do not call Destroy() here. See bug 472312.
     NS_WARNING("Something wrong when creating the docshell for a frameloader!");
--- a/dom/base/nsFrameLoader.h
+++ b/dom/base/nsFrameLoader.h
@@ -69,16 +69,17 @@ class nsFrameLoader final : public nsIFr
   friend class AutoResetInShow;
   friend class AutoResetInFrameSwap;
   typedef mozilla::dom::PBrowserParent PBrowserParent;
   typedef mozilla::dom::TabParent TabParent;
   typedef mozilla::layout::RenderFrameParent RenderFrameParent;
 
 public:
   static nsFrameLoader* Create(mozilla::dom::Element* aOwner,
+                               nsPIDOMWindowOuter* aOpener,
                                bool aNetworkCreated);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsFrameLoader, nsIFrameLoader)
   NS_DECL_NSIFRAMELOADER
   NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
   NS_DECL_NSIWEBBROWSERPERSISTABLE
   nsresult CheckForRecursiveLoad(nsIURI* aURI);
@@ -125,22 +126,22 @@ public:
   void Hide();
 
   nsresult CloneForStatic(nsIFrameLoader* aOriginal);
 
   // The guts of an nsIFrameLoaderOwner::SwapFrameLoader implementation.  A
   // frame loader owner needs to call this, and pass in the two references to
   // nsRefPtrs for frame loaders that need to be swapped.
   nsresult SwapWithOtherLoader(nsFrameLoader* aOther,
-                               RefPtr<nsFrameLoader>& aFirstToSwap,
-                               RefPtr<nsFrameLoader>& aSecondToSwap);
+                               nsIFrameLoaderOwner* aThisOwner,
+                               nsIFrameLoaderOwner* aOtherOwner);
 
   nsresult SwapWithOtherRemoteLoader(nsFrameLoader* aOther,
-                                     RefPtr<nsFrameLoader>& aFirstToSwap,
-                                     RefPtr<nsFrameLoader>& aSecondToSwap);
+                                     nsIFrameLoaderOwner* aThisOwner,
+                                     nsIFrameLoaderOwner* aOtherOwner);
 
   /**
    * Return the primary frame for our owning content, or null if it
    * can't be found.
    */
   nsIFrame* GetPrimaryFrameOfOwningContent() const
   {
     return mOwnerContent ? mOwnerContent->GetPrimaryFrame() : nullptr;
@@ -223,17 +224,19 @@ public:
 
   virtual nsIMessageSender* GetProcessMessageManager() const override;
 
   // public because a callback needs these.
   RefPtr<nsFrameMessageManager> mMessageManager;
   nsCOMPtr<nsIInProcessContentFrameMessageManager> mChildMessageManager;
 
 private:
-  nsFrameLoader(mozilla::dom::Element* aOwner, bool aNetworkCreated);
+  nsFrameLoader(mozilla::dom::Element* aOwner,
+                nsPIDOMWindowOuter* aOpener,
+                bool aNetworkCreated);
   ~nsFrameLoader();
 
   void SetOwnerContent(mozilla::dom::Element* aContent);
 
   bool ShouldUseRemoteProcess();
 
   /**
    * Return true if the frame is a remote frame. Return false otherwise
@@ -349,16 +352,19 @@ private:
   nsWeakFrame mDetachedSubdocFrame;
   // Stores the containing document of the frame corresponding to this
   // frame loader. This is reference is kept valid while the subframe's
   // presentation is detached and stored in mDetachedSubdocFrame. This
   // enables us to detect whether the frame has moved documents during
   // a reframe, so that we know not to restore the presentation.
   nsCOMPtr<nsIDocument> mContainerDocWhileDetached;
 
+  // An opener window which should be used when the docshell is created.
+  nsCOMPtr<nsPIDOMWindowOuter> mOpener;
+
   TabParent* mRemoteBrowser;
   uint64_t mChildID;
 
   // See nsIFrameLoader.idl. EVENT_MODE_NORMAL_DISPATCH automatically
   // forwards some input events to out-of-process content.
   uint32_t mEventMode;
 
   // Holds the last known size of the frame.
--- a/dom/base/nsIFrameLoader.idl
+++ b/dom/base/nsIFrameLoader.idl
@@ -250,9 +250,19 @@ interface nsIFrameLoaderOwner : nsISuppo
    * iframes.
    */
   readonly attribute mozIApplication parentApplication;
 
   /**
    * Puts the FrameLoaderOwner in prerendering mode.
    */
   void setIsPrerendered();
+
+  /**
+   * This method is used internally by SwapFrameLoaders to set the frame loader
+   * on the target nsFrameLoader.
+   *
+   * Avoid using this method outside of that context, and instead prefer using
+   * SwapFrameLoaders.
+   */
+  [noscript, notxpcom] void
+  internalSetFrameLoader(in nsIFrameLoader aNewFrameLoader);
 };
--- a/dom/base/nsObjectLoadingContent.cpp
+++ b/dom/base/nsObjectLoadingContent.cpp
@@ -1265,16 +1265,22 @@ nsObjectLoadingContent::PresetOpenerWind
 }
 
 NS_IMETHODIMP
 nsObjectLoadingContent::SetIsPrerendered()
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+void
+nsObjectLoadingContent::InternalSetFrameLoader(nsIFrameLoader* aNewFrameLoader)
+{
+  MOZ_CRASH("You shouldn't be calling this function, it doesn't make any sense on this type.");
+}
+
 NS_IMETHODIMP
 nsObjectLoadingContent::GetActualType(nsACString& aType)
 {
   aType = mContentType;
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -2451,16 +2457,17 @@ nsObjectLoadingContent::LoadObject(bool 
         // We could mFrameLoader->LoadURI(mURI), but UpdateObjectParameters
         // requires documents have a channel, so this is not a valid state.
         NS_NOTREACHED("Attempting to load a document without a channel");
         mType = eType_Null;
         break;
       }
 
       mFrameLoader = nsFrameLoader::Create(thisContent->AsElement(),
+                                           /* aOpener = */ nullptr,
                                            mNetworkCreated);
       if (!mFrameLoader) {
         NS_NOTREACHED("nsFrameLoader::Create failed");
         mType = eType_Null;
         break;
       }
 
       rv = mFrameLoader->CheckForRecursiveLoad(mURI);
@@ -2895,17 +2902,17 @@ nsObjectLoadingContent::CreateStaticClon
     aDest->mPrintFrame = thisObj->mPrintFrame;
   } else {
     aDest->mPrintFrame = const_cast<nsObjectLoadingContent*>(this)->GetExistingFrame();
   }
 
   if (mFrameLoader) {
     nsCOMPtr<nsIContent> content =
       do_QueryInterface(static_cast<nsIImageLoadingContent*>(aDest));
-    nsFrameLoader* fl = nsFrameLoader::Create(content->AsElement(), false);
+    nsFrameLoader* fl = nsFrameLoader::Create(content->AsElement(), nullptr, false);
     if (fl) {
       aDest->mFrameLoader = fl;
       mFrameLoader->CreateStaticClone(fl);
     }
   }
 }
 
 NS_IMETHODIMP
--- a/dom/html/nsGenericHTMLFrameElement.cpp
+++ b/dom/html/nsGenericHTMLFrameElement.cpp
@@ -146,29 +146,22 @@ nsGenericHTMLFrameElement::EnsureFrameLo
 {
   if (!IsInComposedDoc() || mFrameLoader || mFrameLoaderCreationDisallowed) {
     // If frame loader is there, we just keep it around, cached
     return;
   }
 
   // Strangely enough, this method doesn't actually ensure that the
   // frameloader exists.  It's more of a best-effort kind of thing.
-  mFrameLoader = nsFrameLoader::Create(this, mNetworkCreated);
+  mFrameLoader = nsFrameLoader::Create(this,
+                                       nsPIDOMWindowOuter::From(mOpenerWindow),
+                                       mNetworkCreated);
   if (mIsPrerendered) {
     mFrameLoader->SetIsPrerendered();
   }
-  if (mOpenerWindow) {
-    nsCOMPtr<nsIDocShell> docShell;
-    mFrameLoader->GetDocShell(getter_AddRefs(docShell));
-    if (docShell) {
-      nsCOMPtr<nsPIDOMWindowOuter> outerWindow = docShell->GetWindow();
-      outerWindow->SetOpenerWindow(nsPIDOMWindowOuter::From(mOpenerWindow), true);
-      mOpenerWindow = nullptr;
-    }
-  }
 }
 
 nsresult
 nsGenericHTMLFrameElement::CreateRemoteFrameLoader(nsITabParent* aTabParent)
 {
   MOZ_ASSERT(!mFrameLoader);
   EnsureFrameLoader();
   NS_ENSURE_STATE(mFrameLoader);
@@ -226,46 +219,52 @@ nsGenericHTMLFrameElement::GetParentAppl
 void
 nsGenericHTMLFrameElement::PresetOpenerWindow(mozIDOMWindowProxy* aWindow, ErrorResult& aRv)
 {
   MOZ_ASSERT(!mFrameLoader);
   mOpenerWindow = nsPIDOMWindowOuter::From(aWindow);
 }
 
 void
+nsGenericHTMLFrameElement::InternalSetFrameLoader(nsIFrameLoader* aNewFrameLoader)
+{
+  mFrameLoader = static_cast<nsFrameLoader*>(aNewFrameLoader);
+}
+
+void
 nsGenericHTMLFrameElement::SwapFrameLoaders(HTMLIFrameElement& aOtherLoaderOwner,
                                             ErrorResult& rv)
 {
   if (&aOtherLoaderOwner == this) {
     // nothing to do
     return;
   }
 
-  SwapFrameLoaders(aOtherLoaderOwner.mFrameLoader, rv);
+  aOtherLoaderOwner.SwapFrameLoaders(this, rv);
 }
 
 void
 nsGenericHTMLFrameElement::SwapFrameLoaders(nsXULElement& aOtherLoaderOwner,
                                             ErrorResult& rv)
 {
-  aOtherLoaderOwner.SwapFrameLoaders(mFrameLoader, rv);
+  aOtherLoaderOwner.SwapFrameLoaders(this, rv);
 }
 
 void
-nsGenericHTMLFrameElement::SwapFrameLoaders(RefPtr<nsFrameLoader>& aOtherLoader,
+nsGenericHTMLFrameElement::SwapFrameLoaders(nsIFrameLoaderOwner* aOtherLoaderOwner,
                                             mozilla::ErrorResult& rv)
 {
-  if (!mFrameLoader || !aOtherLoader) {
+  RefPtr<nsFrameLoader> loader = GetFrameLoader();
+  RefPtr<nsFrameLoader> otherLoader = aOtherLoaderOwner->GetFrameLoader();
+  if (!loader || !otherLoader) {
     rv.Throw(NS_ERROR_NOT_IMPLEMENTED);
     return;
   }
 
-  rv = mFrameLoader->SwapWithOtherLoader(aOtherLoader,
-                                         mFrameLoader,
-                                         aOtherLoader);
+  rv = loader->SwapWithOtherLoader(otherLoader, this, aOtherLoaderOwner);
 }
 
 NS_IMETHODIMP
 nsGenericHTMLFrameElement::SetIsPrerendered()
 {
   MOZ_ASSERT(!mFrameLoader, "Please call SetIsPrerendered before frameLoader is created");
   mIsPrerendered = true;
   return NS_OK;
@@ -449,17 +448,17 @@ nsGenericHTMLFrameElement::CopyInnerTo(E
 {
   nsresult rv = nsGenericHTMLElement::CopyInnerTo(aDest);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsIDocument* doc = aDest->OwnerDoc();
   if (doc->IsStaticDocument() && mFrameLoader) {
     nsGenericHTMLFrameElement* dest =
       static_cast<nsGenericHTMLFrameElement*>(aDest);
-    nsFrameLoader* fl = nsFrameLoader::Create(dest, false);
+    nsFrameLoader* fl = nsFrameLoader::Create(dest, nullptr, false);
     NS_ENSURE_STATE(fl);
     dest->mFrameLoader = fl;
     static_cast<nsFrameLoader*>(mFrameLoader.get())->CreateStaticClone(fl);
   }
 
   return rv;
 }
 
--- a/dom/html/nsGenericHTMLFrameElement.h
+++ b/dom/html/nsGenericHTMLFrameElement.h
@@ -75,17 +75,17 @@ public:
                                            nsGenericHTMLElement)
 
   void SwapFrameLoaders(mozilla::dom::HTMLIFrameElement& aOtherLoaderOwner,
                         mozilla::ErrorResult& aError);
 
   void SwapFrameLoaders(nsXULElement& aOtherLoaderOwner,
                         mozilla::ErrorResult& aError);
 
-  void SwapFrameLoaders(RefPtr<nsFrameLoader>& aOtherLoader,
+  void SwapFrameLoaders(nsIFrameLoaderOwner* aOtherLoaderOwner,
                         mozilla::ErrorResult& rv);
 
   void PresetOpenerWindow(mozIDOMWindowProxy* aOpenerWindow,
                           mozilla::ErrorResult& aRv);
 
   static bool BrowserFramesEnabled();
 
   /**
--- a/dom/xul/nsXULElement.cpp
+++ b/dom/xul/nsXULElement.cpp
@@ -177,28 +177,27 @@ nsXULElement::~nsXULElement()
 nsXULElement::nsXULSlots::nsXULSlots()
     : nsXULElement::nsDOMSlots()
 {
 }
 
 nsXULElement::nsXULSlots::~nsXULSlots()
 {
     NS_IF_RELEASE(mControllers); // Forces release
-    if (mFrameLoader) {
-        mFrameLoader->Destroy();
+    nsCOMPtr<nsIFrameLoader> frameLoader = do_QueryInterface(mFrameLoaderOrOpener);
+    if (frameLoader) {
+        static_cast<nsFrameLoader*>(frameLoader.get())->Destroy();
     }
 }
 
 void
 nsXULElement::nsXULSlots::Traverse(nsCycleCollectionTraversalCallback &cb)
 {
-    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mFrameLoader");
-    cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIFrameLoader*, mFrameLoader));
-    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mOpener");
-    cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(mozIDOMWindowProxy*, mOpener));
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mFrameLoaderOrOpener");
+    cb.NoteXPCOMChild(mFrameLoaderOrOpener);
 }
 
 nsINode::nsSlots*
 nsXULElement::CreateSlots()
 {
     return new nsXULSlots();
 }
 
@@ -894,25 +893,21 @@ nsXULElement::UnbindFromTree(bool aDeep,
     // which owns this content.  That's a cycle, so we break
     // it here.  (It might be better to break this by releasing
     // mDocument in nsGlobalWindow::SetDocShell, but I'm not
     // sure whether that would fix all possible cycles through
     // mControllers.)
     nsXULSlots* slots = static_cast<nsXULSlots*>(GetExistingDOMSlots());
     if (slots) {
         NS_IF_RELEASE(slots->mControllers);
-        if (slots->mFrameLoader) {
-            // This element is being taken out of the document, destroy the
-            // possible frame loader.
-            // XXXbz we really want to only partially destroy the frame
-            // loader... we don't want to tear down the docshell.  Food for
-            // later bug.
-            slots->mFrameLoader->Destroy();
-            slots->mFrameLoader = nullptr;
+        RefPtr<nsFrameLoader> frameLoader = GetFrameLoader();
+        if (frameLoader) {
+            frameLoader->Destroy();
         }
+        slots->mFrameLoaderOrOpener = nullptr;
     }
 
     nsStyledElement::UnbindFromTree(aDeep, aNullParent);
 }
 
 void
 nsXULElement::RemoveChildAt(uint32_t aIndex, bool aNotify)
 {
@@ -1244,20 +1239,21 @@ nsXULElement::RemoveBroadcaster(const ns
 }
 
 void
 nsXULElement::DestroyContent()
 {
     nsXULSlots* slots = static_cast<nsXULSlots*>(GetExistingDOMSlots());
     if (slots) {
         NS_IF_RELEASE(slots->mControllers);
-        if (slots->mFrameLoader) {
-            slots->mFrameLoader->Destroy();
-            slots->mFrameLoader = nullptr;
+        RefPtr<nsFrameLoader> frameLoader = GetFrameLoader();
+        if (frameLoader) {
+            frameLoader->Destroy();
         }
+        slots->mFrameLoaderOrOpener = nullptr;
     }
 
     nsStyledElement::DestroyContent();
 }
 
 #ifdef DEBUG
 void
 nsXULElement::List(FILE* out, int32_t aIndent) const
@@ -1574,79 +1570,71 @@ nsXULElement::LoadSrc()
         return NS_OK;
     }
     if (!IsInUncomposedDoc() ||
         !OwnerDoc()->GetRootElement() ||
         OwnerDoc()->GetRootElement()->
             NodeInfo()->Equals(nsGkAtoms::overlay, kNameSpaceID_XUL)) {
         return NS_OK;
     }
-    nsXULSlots* slots = static_cast<nsXULSlots*>(Slots());
-    if (!slots->mFrameLoader) {
-        // false as the last parameter so that xul:iframe/browser/editor
-        // session history handling works like dynamic html:iframes.
-        // Usually xul elements are used in chrome, which doesn't have
-        // session history at all.
-        slots->mFrameLoader = nsFrameLoader::Create(this, false);
-        NS_ENSURE_TRUE(slots->mFrameLoader, NS_OK);
-
-        nsCOMPtr<nsPIDOMWindowOuter> opener;
-        if (slots->mOpener) {
-            opener = nsPIDOMWindowOuter::From(slots->mOpener);
-            slots->mOpener = nullptr;
-        } else {
+    RefPtr<nsFrameLoader> frameLoader = GetFrameLoader();
+    if (!frameLoader) {
+        // Check if we have an opener we need to be setting
+        nsXULSlots* slots = static_cast<nsXULSlots*>(Slots());
+        nsCOMPtr<nsPIDOMWindowOuter> opener = do_QueryInterface(slots->mFrameLoaderOrOpener);
+        if (!opener) {
             // If we are a content-primary xul-browser, we want to take the opener property!
             nsCOMPtr<nsIDOMChromeWindow> chromeWindow = do_QueryInterface(OwnerDoc()->GetWindow());
             if (AttrValueIs(kNameSpaceID_None, nsGkAtoms::type,
                             NS_LITERAL_STRING("content-primary"), eIgnoreCase) &&
                 chromeWindow) {
                 nsCOMPtr<mozIDOMWindowProxy> wp;
                 chromeWindow->TakeOpenerForInitialContentBrowser(getter_AddRefs(wp));
                 opener = nsPIDOMWindowOuter::From(wp);
             }
         }
 
-        if (opener) {
-            nsCOMPtr<nsIDocShell> docShell;
-            nsresult rv = slots->mFrameLoader->GetDocShell(getter_AddRefs(docShell));
-            NS_ENSURE_SUCCESS(rv, rv);
-            nsCOMPtr<nsPIDOMWindowOuter> outerWindow = docShell->GetWindow();
-            outerWindow->SetOpenerWindow(opener, true);
-        }
+        // false as the last parameter so that xul:iframe/browser/editor
+        // session history handling works like dynamic html:iframes.
+        // Usually xul elements are used in chrome, which doesn't have
+        // session history at all.
+        frameLoader = nsFrameLoader::Create(this, opener, false);
+        slots->mFrameLoaderOrOpener = static_cast<nsIFrameLoader*>(frameLoader);
+        NS_ENSURE_TRUE(frameLoader, NS_OK);
 
         (new AsyncEventDispatcher(this,
                                   NS_LITERAL_STRING("XULFrameLoaderCreated"),
                                   /* aBubbles */ true))->RunDOMEventWhenSafe();
 
         if (AttrValueIs(kNameSpaceID_None, nsGkAtoms::prerendered,
                         NS_LITERAL_STRING("true"), eIgnoreCase)) {
-            nsresult rv = slots->mFrameLoader->SetIsPrerendered();
+            nsresult rv = frameLoader->SetIsPrerendered();
             NS_ENSURE_SUCCESS(rv,rv);
         }
     }
 
-    return slots->mFrameLoader->LoadFrame();
+    return frameLoader->LoadFrame();
 }
 
 nsresult
 nsXULElement::GetFrameLoaderXPCOM(nsIFrameLoader **aFrameLoader)
 {
     *aFrameLoader = GetFrameLoader().take();
     return NS_OK;
 }
 
 already_AddRefed<nsFrameLoader>
 nsXULElement::GetFrameLoader()
 {
     nsXULSlots* slots = static_cast<nsXULSlots*>(GetExistingSlots());
     if (!slots)
         return nullptr;
 
-    RefPtr<nsFrameLoader> loader = slots->mFrameLoader;
-    return loader.forget();
+    nsCOMPtr<nsIFrameLoader> loader = do_QueryInterface(slots->mFrameLoaderOrOpener);
+    return already_AddRefed<nsFrameLoader>(static_cast<nsFrameLoader*>(loader.forget().take()));
 }
 
 nsresult
 nsXULElement::GetParentApplication(mozIApplication** aApplication)
 {
     if (!aApplication) {
         return NS_ERROR_FAILURE;
     }
@@ -1654,73 +1642,86 @@ nsXULElement::GetParentApplication(mozIA
     *aApplication = nullptr;
     return NS_OK;
 }
 
 void
 nsXULElement::PresetOpenerWindow(mozIDOMWindowProxy* aWindow, ErrorResult& aRv)
 {
     nsXULSlots* slots = static_cast<nsXULSlots*>(Slots());
-    MOZ_ASSERT(!slots->mFrameLoader, "Cannot SetOpenerWindow when a frame loader is present");
-
-    slots->mOpener = aWindow;
+    MOZ_ASSERT(!slots->mFrameLoaderOrOpener, "A frameLoader or opener is present when calling PresetOpenerWindow");
+
+    slots->mFrameLoaderOrOpener = aWindow;
 }
 
 nsresult
 nsXULElement::SetIsPrerendered()
 {
   return SetAttr(kNameSpaceID_None, nsGkAtoms::prerendered, nullptr,
                  NS_LITERAL_STRING("true"), true);
 }
 
 void
+nsXULElement::InternalSetFrameLoader(nsIFrameLoader* aNewFrameLoader)
+{
+    nsXULSlots* slots = static_cast<nsXULSlots*>(GetExistingDOMSlots());
+    MOZ_ASSERT(slots);
+
+    slots->mFrameLoaderOrOpener = aNewFrameLoader;
+}
+
+void
 nsXULElement::SwapFrameLoaders(HTMLIFrameElement& aOtherLoaderOwner,
                                ErrorResult& rv)
 {
-    nsXULSlots *ourSlots = static_cast<nsXULSlots*>(GetExistingDOMSlots());
-    if (!ourSlots) {
+    if (!GetExistingDOMSlots()) {
         rv.Throw(NS_ERROR_NOT_IMPLEMENTED);
         return;
     }
 
-    aOtherLoaderOwner.SwapFrameLoaders(ourSlots->mFrameLoader, rv);
+    nsCOMPtr<nsIFrameLoaderOwner> flo = do_QueryInterface(static_cast<nsIDOMXULElement*>(this));
+    aOtherLoaderOwner.SwapFrameLoaders(flo, rv);
 }
 
 void
 nsXULElement::SwapFrameLoaders(nsXULElement& aOtherLoaderOwner,
                                ErrorResult& rv)
 {
     if (&aOtherLoaderOwner == this) {
         // nothing to do
         return;
     }
 
-    nsXULSlots *otherSlots =
-        static_cast<nsXULSlots*>(aOtherLoaderOwner.GetExistingDOMSlots());
-    if (!otherSlots) {
+    if (!GetExistingDOMSlots()) {
         rv.Throw(NS_ERROR_NOT_IMPLEMENTED);
         return;
     }
 
-    SwapFrameLoaders(otherSlots->mFrameLoader, rv);
+    nsCOMPtr<nsIFrameLoaderOwner> flo = do_QueryInterface(static_cast<nsIDOMXULElement*>(this));
+    aOtherLoaderOwner.SwapFrameLoaders(flo, rv);
 }
 
 void
-nsXULElement::SwapFrameLoaders(RefPtr<nsFrameLoader>& aOtherLoader,
+nsXULElement::SwapFrameLoaders(nsIFrameLoaderOwner* aOtherLoaderOwner,
                                mozilla::ErrorResult& rv)
 {
-    nsXULSlots *ourSlots = static_cast<nsXULSlots*>(GetExistingDOMSlots());
-    if (!ourSlots || !ourSlots->mFrameLoader || !aOtherLoader) {
+    if (!GetExistingDOMSlots()) {
         rv.Throw(NS_ERROR_NOT_IMPLEMENTED);
         return;
     }
 
-    rv = ourSlots->mFrameLoader->SwapWithOtherLoader(aOtherLoader,
-                                                     ourSlots->mFrameLoader,
-                                                     aOtherLoader);
+    RefPtr<nsFrameLoader> loader = GetFrameLoader();
+    RefPtr<nsFrameLoader> otherLoader = aOtherLoaderOwner->GetFrameLoader();
+    if (!loader || !otherLoader) {
+        rv.Throw(NS_ERROR_NOT_IMPLEMENTED);
+        return;
+    }
+
+    nsCOMPtr<nsIFrameLoaderOwner> flo = do_QueryInterface(static_cast<nsIDOMXULElement*>(this));
+    rv = loader->SwapWithOtherLoader(otherLoader, flo, aOtherLoaderOwner);
 }
 
 NS_IMETHODIMP
 nsXULElement::GetParentTree(nsIDOMXULMultiSelectControlElement** aTreeElement)
 {
     for (nsIContent* current = GetParent(); current;
          current = current->GetParent()) {
         if (current->NodeInfo()->Equals(nsGkAtoms::listbox,
--- a/dom/xul/nsXULElement.h
+++ b/dom/xul/nsXULElement.h
@@ -570,21 +570,22 @@ public:
                              const nsAString& aValue);
     already_AddRefed<nsINodeList>
       GetElementsByAttributeNS(const nsAString& aNamespaceURI,
                                const nsAString& aAttribute,
                                const nsAString& aValue,
                                mozilla::ErrorResult& rv);
     // Style() inherited from nsStyledElement
     already_AddRefed<nsFrameLoader> GetFrameLoader();
+    void InternalSetFrameLoader(nsIFrameLoader* aNewFrameLoader);
     void SwapFrameLoaders(mozilla::dom::HTMLIFrameElement& aOtherLoaderOwner,
                           mozilla::ErrorResult& rv);
     void SwapFrameLoaders(nsXULElement& aOtherLoaderOwner,
                           mozilla::ErrorResult& rv);
-    void SwapFrameLoaders(RefPtr<nsFrameLoader>& aOtherLoader,
+    void SwapFrameLoaders(nsIFrameLoaderOwner* aOtherLoaderOwner,
                           mozilla::ErrorResult& rv);
 
     nsINode* GetScopeChainParent() const override
     {
         // For XUL, the parent is the parent element, if any
         Element* parent = GetParentElement();
         return parent ? parent : nsStyledElement::GetScopeChainParent();
     }
@@ -611,18 +612,17 @@ protected:
     class nsXULSlots : public mozilla::dom::Element::nsDOMSlots
     {
     public:
         nsXULSlots();
         virtual ~nsXULSlots();
 
         void Traverse(nsCycleCollectionTraversalCallback &cb);
 
-        RefPtr<nsFrameLoader> mFrameLoader;
-        nsCOMPtr<mozIDOMWindowProxy> mOpener;
+        nsCOMPtr<nsISupports> mFrameLoaderOrOpener;
     };
 
     virtual nsINode::nsSlots* CreateSlots() override;
 
     nsresult LoadSrc();
 
     /**
      * The nearest enclosing content node with a binding