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 319751 3515eb43f05a
parent 319750 7ee25aab52f5
child 319752 11f778f3ea30
push id83239
push usermichael@thelayzells.com
push date2016-10-27 19:53 +0000
treeherdermozilla-inbound@2a31079dae25 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1303196
milestone52.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 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