From 0b6086ce3e6d89d46765eb24783c45067897c65f Mon Sep 17 00:00:00 2001
From: Marco Castelluccio <mcastelluccio@mozilla.com>
Date: Thu, 25 Jan 2024 19:15:17 +0000
Subject: [PATCH] Bug 1624118 - Fix various issues with view handling when
 loading <object> / <embed> as subdocuments. r=longsonr

These look like two really old bugs.

Part of the issue is that <object> / <embed> manage their frame loader quite
differently from <iframe>. This means that we may have a PresShell /
ViewManager / etc for a frame loader that doesn't yet have a frame associated.

For example, this is the viewport creation for the SVG document that reproduces
the problem:

	#0  0x00005cc60e8302e7 in mozilla::ViewportFrame::SetViewInternal(nsView*) (this=0x68599020, aView=0x683d8f00) at /home/emilio/src/moz/gecko/obj-debug/dist/include/mozilla/ViewportFrame.h:106
	#1  0x00005cc60e842858 in nsIFrame::SetView(nsView*) (this=0x68599020, aView=0x683d8f00) at /home/emilio/src/moz/gecko/layout/generic/nsFrame.cpp:7057
	#2  0x00005cc60e77258a in nsCSSFrameConstructor::ConstructRootFrame() (this=0xc72c715e00) at /home/emilio/src/moz/gecko/layout/base/nsCSSFrameConstructor.cpp:2424
	#3  0x00005cc60e7342f5 in mozilla::PresShell::Initialize() (this=0x6830e000) at /home/emilio/src/moz/gecko/layout/base/PresShell.cpp:1758
	#4  0x00005cc60c9fb02a in nsContentSink::StartLayout(bool) (this=<optimized out>, aIgnorePendingSheets=<optimized out>) at /home/emilio/src/moz/gecko/dom/base/nsContentSink.cpp:1160
	#5  0x00005cc60e2c1581 in nsXMLContentSink::HandleStartElement(char16_t const*, char16_t const**, unsigned int, unsigned int, unsigned int, bool)
	    (this=<optimized out>, aName=<optimized out>, aAtts=0x6fde8200, aAttsCount=<optimized out>, aLineNumber=3, aColumnNumber=<optimized out>, aInterruptable=true) at /home/emilio/src/moz/gecko/dom/xml/nsXMLContentSink.cpp:982
	#6  0x00005cc60e2c17d7 in non-virtual thunk to nsXMLContentSink::HandleStartElement(char16_t const*, char16_t const**, unsigned int, unsigned int, unsigned int) () at /home/emilio/src/moz/gecko/dom/xml/nsXMLContentSink.cpp:889
	#7  0x00005cc60c360307 in nsExpatDriver::HandleStartElement(void*, char16_t const*, char16_t const**) (aUserData=0x224f650d0cc0, aName=0x685aef20 u"http://www.w3.org/2000/svg\xffffdesc", aAtts=0x6fde8200)
	    at /home/emilio/src/moz/gecko/parser/htmlparser/nsExpatDriver.cpp:293
	#8  0x00005cc60ee91cea in doContent (parser=0xc72c70f800, startTagLevel=0, enc=<optimized out>, s=<optimized out>, end=0x5bbd31af5020 "\344\344\344", <incomplete sequence \344>, nextPtr=0x7ffca2653288, haveMore=1 '\001')
	    at /home/emilio/src/moz/gecko/parser/expat/lib/xmlparse.c:2872
	#9  0x00005cc60ee90059 in contentProcessor (parser=0xc72c70f800, start=0xffffffe6 <error: Cannot access memory at address 0xffffffe6>, end=0xc72c511360 "", endPtr=0x1) at /home/emilio/src/moz/gecko/parser/expat/lib/xmlparse.c:2528
	#10 0x00005cc60ee8f8d5 in doProlog
	    (parser=<optimized out>, enc=0x5cc612ce0930 <little2_encoding_ns>, s=0x5bbd31ab508e "<", end=0x5bbd31af5020 "\344\344\344", <incomplete sequence \344>, tok=<optimized out>, next=<optimized out>, nextPtr=0x7ffca2653288, haveMore=1 '\001', allowClosingDoctype=1 '\001') at /home/emilio/src/moz/gecko/parser/expat/lib/xmlparse.c:4591
	#11 0x00005cc60ee8d86e in prologProcessor (parser=0xc72c70f800, s=0x5bbd31ab5020 "<", end=0x5bbd31af5020 "\344\344\344", <incomplete sequence \344>, nextPtr=0x7ffca2653288) at /home/emilio/src/moz/gecko/parser/expat/lib/xmlparse.c:4311
	#12 0x00005cc60ee8cf45 in MOZ_XML_Parse (parser=0xc72c70f800, s=0x5bbd31ab5020 "<", len=262144, isFinal=0) at /home/emilio/src/moz/gecko/parser/expat/lib/xmlparse.c:1894
	#13 0x00005cc60c3627bc in nsExpatDriver::ParseBuffer(char16_t const*, unsigned int, bool, unsigned int*)
	    (this=0x224f650d0cc0, aBuffer=0x5bbd31ab5020 u"<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<svg height=\"2970\" width=\"2100\" viewBox=\"0 0 2100 2970\" version=\"1.1\" xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlin"..., aLength=131072, aIsFinal=false, aConsumed=<optimized out>) at /home/emilio/src/moz/gecko/parser/htmlparser/nsExpatDriver.cpp:875
	#14 0x00005cc60c362c91 in nsExpatDriver::ConsumeToken(nsScanner&, bool&) (this=<optimized out>, aScanner=..., aFlushTokens=<optimized out>) at /home/emilio/src/moz/gecko/parser/htmlparser/nsExpatDriver.cpp:970
	#15 0x00005cc60c3666a8 in nsParser::Tokenize(bool) (this=0x224f65038e80, aIsFinalChunk=false) at /home/emilio/src/moz/gecko/parser/htmlparser/nsParser.cpp:1410
	#16 0x00005cc60c36595e in nsParser::ResumeParse(bool, bool, bool) (this=0x224f65038e80, allowIteration=true, aIsFinalChunk=false, aCanInterrupt=<optimized out>) at /home/emilio/src/moz/gecko/parser/htmlparser/nsParser.cpp:961
	#17 0x00005cc60c366c86 in nsParser::OnDataAvailable(nsIRequest*, nsIInputStream*, unsigned long, unsigned int) (this=0x224f65038e80, request=<optimized out>, pIStream=0x6fdfc430, sourceOffset=<optimized out>, aLength=131072)
	    at /home/emilio/src/moz/gecko/parser/htmlparser/nsParser.cpp:1317
	#18 0x00005cc60c897cc2 in nsObjectLoadingContent::OnDataAvailable(nsIRequest*, nsIInputStream*, unsigned long, unsigned int) (this=<optimized out>, aRequest=0x68483080, aInputStream=0x6fdfc430, aOffset=0, aCount=131072)
	    at /home/emilio/src/moz/gecko/dom/base/nsObjectLoadingContent.cpp:1055
	#19 0x00005cc60b9d18f8 in mozilla::net::HttpChannelChild::DoOnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int)
	    (this=0x68483000, aRequest=0x68483080, aContext=<optimized out>, aStream=0x6fdfc430, aOffset=0, aCount=743510880) at /home/emilio/src/moz/gecko/netwerk/protocol/http/HttpChannelChild.cpp:968
	#20 0x00005cc60b9d5cbf in mozilla::net::HttpChannelChild::OnTransportAndData(nsresult const&, nsresult const&, unsigned long const&, unsigned int const&, nsTString<char> const&)
	    (this=0x68483000, aChannelStatus=<optimized out>, aTransportStatus=0x683be5bc: -2142568440, aOffset=<optimized out>, aCount=<optimized out>, aData=...) at /home/emilio/src/moz/gecko/netwerk/protocol/http/HttpChannelChild.cpp:867
	#21 0x00005cc60badb535 in mozilla::net::ChannelEventQueue::FlushQueue() (this=0xc72ce2cae0) at /home/emilio/src/moz/gecko/netwerk/ipc/ChannelEventQueue.cpp:90
	#22 0x00005cc60b976fda in mozilla::net::ChannelEventQueue::MaybeFlushQueue() (this=0xc72ce2cae0) at /home/emilio/src/moz/gecko/obj-debug/dist/include/mozilla/net/ChannelEventQueue.h:350
	#23 0x00005cc60baec442 in mozilla::net::ChannelEventQueue::CompleteResume() (this=0xc72ce2cae0) at /home/emilio/src/moz/gecko/obj-debug/dist/include/mozilla/net/ChannelEventQueue.h:329
	#24 mozilla::net::ChannelEventQueue::ResumeInternal()::CompleteResumeRunnable::Run() (this=<optimized out>) at /home/emilio/src/moz/gecko/netwerk/ipc/ChannelEventQueue.cpp:148
	#25 0x00005cc60b53d4f1 in mozilla::SchedulerGroup::Runnable::Run() (this=0x685b0200) at /home/emilio/src/moz/gecko/xpcom/threads/SchedulerGroup.cpp:282
	#26 0x00005cc60b54ff80 in nsThread::ProcessNextEvent(bool, bool*) (this=0x3dd7f4f3020, aMayWait=<optimized out>, aResult=0x7ffca2653ea7) at /home/emilio/src/moz/gecko/xpcom/threads/nsThread.cpp:1220
	#27 0x00005cc60b552f0d in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x68599020, aMayWait=true) at /home/emilio/src/moz/gecko/xpcom/threads/nsThreadUtils.cpp:481

The parent view may not be set properly if the frame is not current by the time
it is created. For example in this case the parent for the root view is
non-null and comes from the following MakeWindow call:

	#0  nsDocumentViewer::MakeWindow(nsSize const&, nsView*) (this=0xc72ca72cd0, aSize=..., aContainerView=0x683ba500) at /home/emilio/src/moz/gecko/layout/base/nsDocumentViewer.cpp:2368
	#1  0x00005cc60e789b50 in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::dom::WindowGlobalChild*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool)
	    (this=0xc72ca72cd0, aParentWidget=<optimized out>, aState=0x0, aActor=0x0, aBounds=..., aDoCreation=true, aNeedMakeCX=<optimized out>, aForceSetNewDocument=<optimized out>)
	    at /home/emilio/src/moz/gecko/layout/base/nsDocumentViewer.cpp:933
	#2  0x00005cc60e789959 in nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::dom::WindowGlobalChild*)
	    (this=0xc72ca72cd0, aParentWidget=0x7ffca2651020, aBounds=..., aActor=0x7f6216725c00) at /home/emilio/src/moz/gecko/layout/base/nsDocumentViewer.cpp:762
	#3  0x00005cc60f4f584f in nsDocShell::SetupNewViewer(nsIContentViewer*, mozilla::dom::WindowGlobalChild*) (this=0x684db000, aNewViewer=<optimized out>, aWindowActor=<optimized out>)
	    at /home/emilio/src/moz/gecko/docshell/base/nsDocShell.cpp:8017
	#4  0x00005cc60f4f5204 in nsDocShell::Embed(nsIContentViewer*, mozilla::dom::WindowGlobalChild*) (this=0x684db000, aContentViewer=0x7ffca2651020, aWindowActor=0x683ba500) at /home/emilio/src/moz/gecko/docshell/base/nsDocShell.cpp:5745
	#5  0x00005cc60f4dbc7b in nsDocShell::CreateContentViewer(nsTSubstring<char> const&, nsIRequest*, nsIStreamListener**) (this=0x684db000, aContentType=..., aRequest=0x68483080, aContentHandler=<optimized out>)
	    at /home/emilio/src/moz/gecko/docshell/base/nsDocShell.cpp:7819
	#6  0x00005cc60f4dab99 in nsDSURIContentListener::DoContent(nsTSubstring<char> const&, bool, nsIRequest*, nsIStreamListener**, bool*)
	    (this=0x683056a0, aContentType=..., aIsContentPreferred=<optimized out>, aRequest=0x68483080, aContentHandler=0xc72c5e8608, aAbortProcess=0x7ffca265139f) at /home/emilio/src/moz/gecko/docshell/base/nsDSURIContentListener.cpp:181
	#7  0x00005cc60c2fd8f5 in nsDocumentOpenInfo::TryContentListener(nsIURIContentListener*, nsIChannel*) (this=0xc72c5e85e0, aListener=0x683056a0, aChannel=<optimized out>) at /home/emilio/src/moz/gecko/uriloader/base/nsURILoader.cpp:632
	#8  0x00005cc60c2fccd1 in nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*) (this=0xc72c5e85e0, request=0x68483080, aCtxt=<optimized out>) at /home/emilio/src/moz/gecko/uriloader/base/nsURILoader.cpp:313
	#9  0x00005cc60c2fc5aa in nsDocumentOpenInfo::OnStartRequest(nsIRequest*) (this=<optimized out>, request=0x68483080) at /home/emilio/src/moz/gecko/uriloader/base/nsURILoader.cpp:191
	#10 0x00005cc60c8975c4 in nsObjectLoadingContent::LoadObject(bool, bool, nsIRequest*) (this=0x4b1b3938b6a8, aNotify=<optimized out>, aForceLoad=<optimized out>, aLoadingChannel=0x68483080)
	    at /home/emilio/src/moz/gecko/dom/base/nsObjectLoadingContent.cpp:2218
	#11 0x00005cc60c89681f in nsObjectLoadingContent::OnStartRequest(nsIRequest*) (this=0x4b1b3938b6a8, aRequest=0x68483080) at /home/emilio/src/moz/gecko/dom/base/nsObjectLoadingContent.cpp:1006
	#12 0x00005cc60b9d1020 in mozilla::net::HttpChannelChild::DoOnStartRequest(nsIRequest*, nsISupports*) (this=0x68483000, aRequest=0x68483080, aContext=<optimized out>)
	    at /home/emilio/src/moz/gecko/netwerk/protocol/http/HttpChannelChild.cpp:708
	#13 0x00005cc60b9d481b in mozilla::net::HttpChannelChild::OnStartRequest(nsresult const&, mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, mozilla::net::ParentLoadInfoForwarderArgs const&, bool const&, bool const&, bool const&, unsigned long const&, int const&, unsigned int const&, nsTString<char> const&, nsTString<char> const&, mozilla::net::NetAddr const&, mozilla::net::NetAddr const&, unsigned int const&, nsTString<char> const&, long const&, bool const&, bool const&, bool const&, mozilla::net::ResourceTimingStructArgs const&, bool const&, mozilla::Maybe<unsigned int> const&, bool const&, nsILoadInfo::CrossOriginOpenerPolicy const&)

However, even though aContainerView is non-null, the view is incorrect, it's
the view for the _old_ frame.

The view parent/child relationship gets cleared properly in:

	#1  0x00005cc60e8e82bf in BeginSwapDocShellsForViews (aSibling=0x0) at /home/emilio/src/moz/gecko/layout/generic/nsSubDocumentFrame.cpp:1027
	warning: Source file is more recent than executable.
	#2  0x00005cc60e8e810b in nsSubDocumentFrame::DestroyFrom (this=0x6cd04eaa45a8, aDestructRoot=0x6cd04eaa45a8, aPostDestroyData=...) at /home/emilio/src/moz/gecko/layout/generic/nsSubDocumentFrame.cpp:943
	#3  0x00005cc60e7b71a3 in nsIFrame::Destroy (this=0x6cd04eaa45a8) at /home/emilio/src/moz/gecko/layout/generic/nsIFrame.h:657
	#4  0x00005cc60e80baac in nsBlockFrame::RemoveFrame (this=0x4b1b39362d88, aListID=<optimized out>, aOldFrame=0x6cd04eaa45a8) at /home/emilio/src/moz/gecko/layout/generic/nsBlockFrame.cpp:5597
	#5  0x00005cc60e8df29f in nsPlaceholderFrame::DestroyFrom (this=0x4b1b39363240, aDestructRoot=0x4b1b39363240, aPostDestroyData=...) at /home/emilio/src/moz/gecko/layout/generic/nsPlaceholderFrame.cpp:181
	#6  0x00005cc60e80cf19 in nsBlockFrame::DoRemoveFrameInternal (this=<optimized out>, aDeletedFrame=0x0, aFlags=<optimized out>, aPostDestroyData=...) at /home/emilio/src/moz/gecko/layout/generic/nsBlockFrame.cpp:6265
	#7  0x00005cc60e82d947 in nsBlockFrame::DoRemoveFrame (this=0x4b1b39362d88, aDeletedFrame=0x683d8f00, aFlags=244338087) at /home/emilio/src/moz/gecko/layout/generic/nsBlockFrame.h:528
	#8  0x00005cc60e80ba3a in nsBlockFrame::RemoveFrame (this=0x4b1b39362d88, aListID=<optimized out>, aOldFrame=0x4b1b39363240) at /home/emilio/src/moz/gecko/layout/generic/nsBlockFrame.cpp:5581
	#9  0x00005cc60e77fd5c in nsCSSFrameConstructor::ContentRemoved (this=<optimized out>, aChild=0x4b1b3938b600, aOldNextSibling=<optimized out>, aFlags=<optimized out>)
	    at /home/emilio/src/moz/gecko/layout/base/nsCSSFrameConstructor.cpp:7583
	#10 0x00005cc60e779a35 in nsCSSFrameConstructor::RecreateFramesForContent (this=0x6fdf9800, aContent=0x4b1b3938b600, aInsertionKind=nsCSSFrameConstructor::InsertionKind::Sync)
	    at /home/emilio/src/moz/gecko/layout/base/nsCSSFrameConstructor.cpp:8593
	#11 0x00005cc60e751745 in mozilla::RestyleManager::ProcessRestyledFrames (this=<optimized out>, aChangeList=...) at /home/emilio/src/moz/gecko/layout/base/RestyleManager.cpp:1484

But the temporary state is stored in the _old_ frame-loader, so when we create
the new frame, we get to nsSubDocumentFrame::Init, and find nothing, and thus
go through nsFrameLoader::Show. But we do have a pres-shell, and
nsFrameLoader::Show just early-returns then, and thus we end up with a detached
pres shell which is not hooked to the view tree and thus not painted...

So there are multiple potential fixes. First (this is the approach this patch
takes):

 * Make nsHideViewer not fail to hide a presentation when the frame loader
   has changed. This is not an issue per se, but leaves stale views / etc
   living for too long which is not nice.

 * Fix up the Show() code path to handle this case properly by parenting the
   pres-shell and initializing the docshell properly.

Second potential fix would be to store the temporary state somewhere else than
the frame loader (like the element). This may be a less invasive change
somehow, but it looks pretty fishy to me, and not particularly better...

Terribly sorry about the lack of test-case, but this is racy as crazy and I had
a lot of trouble to even reproduce it in a debug build. This needs the
PresShell creation for the subdocument to happen right after setting .data on
the <object>, but before processing its reframe.

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

UltraBlame original commit: 50b298a5272a5c248c5f428cfb03087d919f0a7b
---
 dom/base/nsFrameLoader.cpp            | 30 +++++++++++++++++++++++----
 layout/generic/nsSubDocumentFrame.cpp | 25 +++++++++++-----------
 layout/generic/nsSubDocumentFrame.h   |  4 ++++
 layout/reftests/bugs/1624118-ref.html |  2 ++
 layout/reftests/bugs/1624118.html     | 18 ++++++++++++++++
 layout/reftests/bugs/reftest.list     |  1 +
 6 files changed, 63 insertions(+), 17 deletions(-)
 create mode 100644 layout/reftests/bugs/1624118-ref.html
 create mode 100644 layout/reftests/bugs/1624118.html

diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp
index 773552c1f50a..a95babb1bdeb 100644
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -47,6 +47,7 @@
 #include "nsPIWindowRoot.h"
 #include "nsLayoutUtils.h"
 #include "nsView.h"
+#include "nsViewManager.h"
 #include "nsBaseWidget.h"
 #include "nsQueryObject.h"
 #include "ReferrerInfo.h"
@@ -992,6 +993,14 @@ bool nsFrameLoader::Show(nsSubDocumentFrame* frame) {
   ds->SetScrollbarPreference(GetScrollbarPreference(mOwnerContent));
   const bool marginsChanged =
       ds->UpdateFrameMargins(GetMarginAttributes(mOwnerContent));
+
+  nsView* view = frame->EnsureInnerView();
+  if (!view) {
+    return false;
+  }
+
+  
+  
   if (PresShell* presShell = ds->GetPresShell()) {
     
     if (marginsChanged) {
@@ -1000,11 +1009,21 @@ bool nsFrameLoader::Show(nsSubDocumentFrame* frame) {
                                     NS_FRAME_IS_DIRTY);
       }
     }
-    return true;
-  }
+    nsView* childView = presShell->GetViewManager()->GetRootView();
+    MOZ_DIAGNOSTIC_ASSERT(childView);
+    if (childView->GetParent() == view) {
+      
+      
+      return true;
+    }
 
-  nsView* view = frame->EnsureInnerView();
-  if (!view) return false;
+    
+    
+    MOZ_DIAGNOSTIC_ASSERT(!view->GetFirstChild());
+    MOZ_DIAGNOSTIC_ASSERT(!childView->GetParent(), "Stale view?");
+    nsSubDocumentFrame::InsertViewsInReverseOrder(childView, view);
+    nsSubDocumentFrame::EndSwapDocShellsForViews(view->GetFirstChild());
+  }
 
   RefPtr<nsDocShell> baseWindow = GetDocShell();
   baseWindow->InitWindow(nullptr, view->GetWidget(), 0, 0, size.width,
@@ -1016,6 +1035,9 @@ bool nsFrameLoader::Show(nsSubDocumentFrame* frame) {
   
   
   
+  
+  
+  
   if (RefPtr<PresShell> presShell = GetDocShell()->GetPresShell()) {
     Document* doc = presShell->GetDocument();
     nsHTMLDocument* htmlDoc =
diff --git a/layout/generic/nsSubDocumentFrame.cpp b/layout/generic/nsSubDocumentFrame.cpp
index 80e198f726d1..954ab78f69b5 100644
--- a/layout/generic/nsSubDocumentFrame.cpp
+++ b/layout/generic/nsSubDocumentFrame.cpp
@@ -116,10 +116,6 @@ class AsyncFrameInit : public Runnable {
   WeakFrame mFrame;
 };
 
-static void InsertViewsInReverseOrder(nsView* aSibling, nsView* aParent);
-
-static void EndSwapDocShellsForViews(nsView* aView);
-
 void nsSubDocumentFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
                               nsIFrame* aPrevInFlow) {
   MOZ_ASSERT(aContent);
@@ -148,8 +144,8 @@ void nsSubDocumentFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
     nsView* detachedView = detachedFrame ? detachedFrame->GetView() : nullptr;
     if (detachedView) {
       
-      ::InsertViewsInReverseOrder(detachedView, mInnerView);
-      ::EndSwapDocShellsForViews(mInnerView->GetFirstChild());
+      InsertViewsInReverseOrder(detachedView, mInnerView);
+      EndSwapDocShellsForViews(mInnerView->GetFirstChild());
     } else if (hadFrame) {
       
       frameloader->Hide();
@@ -921,7 +917,7 @@ class nsHideViewer final : public Runnable {
     mFrameLoader->SetDetachedSubdocFrame(nullptr);
 
     nsSubDocumentFrame* frame = do_QueryFrame(mFrameElement->GetPrimaryFrame());
-    if (!frame) {
+    if (!frame || frame->FrameLoader() != mFrameLoader) {
       PropagateIsUnderHiddenEmbedderElement(mFrameLoader, true);
       if (mHideViewerIfFrameless) {
         
@@ -1080,7 +1076,9 @@ static nsView* BeginSwapDocShellsForViews(nsView* aSibling) {
   return removedViews;
 }
 
-static void InsertViewsInReverseOrder(nsView* aSibling, nsView* aParent) {
+
+void nsSubDocumentFrame::InsertViewsInReverseOrder(nsView* aSibling,
+                                                   nsView* aParent) {
   MOZ_ASSERT(aParent, "null view");
   MOZ_ASSERT(!aParent->GetFirstChild(), "inserting into non-empty list");
 
@@ -1115,8 +1113,8 @@ nsresult nsSubDocumentFrame::BeginSwapDocShells(nsIFrame* aOther) {
     nsView* otherSubdocViews = other->mInnerView->GetFirstChild();
     nsView* otherRemovedViews = ::BeginSwapDocShellsForViews(otherSubdocViews);
 
-    ::InsertViewsInReverseOrder(ourRemovedViews, other->mInnerView);
-    ::InsertViewsInReverseOrder(otherRemovedViews, mInnerView);
+    InsertViewsInReverseOrder(ourRemovedViews, other->mInnerView);
+    InsertViewsInReverseOrder(otherRemovedViews, mInnerView);
   }
   mFrameLoader.swap(other->mFrameLoader);
   return NS_OK;
@@ -1147,7 +1145,8 @@ static CallState EndSwapDocShellsForDocument(Document& aDocument) {
   return CallState::Continue;
 }
 
-static void EndSwapDocShellsForViews(nsView* aSibling) {
+
+void nsSubDocumentFrame::EndSwapDocShellsForViews(nsView* aSibling) {
   for (; aSibling; aSibling = aSibling->GetNextSibling()) {
     if (Document* doc = ::GetDocumentFromView(aSibling)) {
       ::EndSwapDocShellsForDocument(*doc);
@@ -1178,10 +1177,10 @@ void nsSubDocumentFrame::EndSwapDocShells(nsIFrame* aOther) {
   AutoWeakFrame weakOther(aOther);
 
   if (mInnerView) {
-    ::EndSwapDocShellsForViews(mInnerView->GetFirstChild());
+    EndSwapDocShellsForViews(mInnerView->GetFirstChild());
   }
   if (other->mInnerView) {
-    ::EndSwapDocShellsForViews(other->mInnerView->GetFirstChild());
+    EndSwapDocShellsForViews(other->mInnerView->GetFirstChild());
   }
 
   
diff --git a/layout/generic/nsSubDocumentFrame.h b/layout/generic/nsSubDocumentFrame.h
index 4dc28e88ed44..886e4f1656dd 100644
--- a/layout/generic/nsSubDocumentFrame.h
+++ b/layout/generic/nsSubDocumentFrame.h
@@ -96,6 +96,10 @@ class nsSubDocumentFrame final : public nsAtomicContainerFrame,
   nsIDocShell* GetDocShell() const;
   nsresult BeginSwapDocShells(nsIFrame* aOther);
   void EndSwapDocShells(nsIFrame* aOther);
+
+  static void InsertViewsInReverseOrder(nsView* aSibling, nsView* aParent);
+  static void EndSwapDocShellsForViews(nsView* aView);
+
   nsView* EnsureInnerView();
   nsPoint GetExtraOffset() const;
   nsIFrame* GetSubdocumentRootFrame();
diff --git a/layout/reftests/bugs/1624118-ref.html b/layout/reftests/bugs/1624118-ref.html
new file mode 100644
index 000000000000..3c10ed7406fb
--- /dev/null
+++ b/layout/reftests/bugs/1624118-ref.html
@@ -0,0 +1,2 @@
+<!doctype html>
+<object type="image/svg+xml" data="243519-8.svg"></object>
diff --git a/layout/reftests/bugs/1624118.html b/layout/reftests/bugs/1624118.html
new file mode 100644
index 000000000000..a6b4a1465af4
--- /dev/null
+++ b/layout/reftests/bugs/1624118.html
@@ -0,0 +1,18 @@
+<!doctype html>
+<html class="reftest-wait">
+<script>
+  let loadedOnce = false;
+  function objectLoaded() {
+    let firstTime = !loadedOnce;
+    loadedOnce = true;
+    let object = document.querySelector("object");
+    if (!firstTime) {
+      document.documentElement.className = "";
+      return;
+    }
+    // The svgs used in this test are arbitrary, but should obviously match the
+    // reference and should be different from each other.
+    object.data = "243519-8.svg";
+  }
+</script>
+<object onload="objectLoaded()" type="image/svg+xml" data="315920-20.svg"></object>
diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list
index 7630cb9d6d93..0e6c7d1bc7e4 100644
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -2074,6 +2074,7 @@ skip-if(!OSX) != 1608124-2.html 1608124-2-notref.html
 fuzzy(0-145,0-48536) == 1614788-1.svg 1614788-1-ref.svg # large fuzz necesary to test bug: 239,202824 is how much it differs when bug exists
 == blob-fallback-clip.html blob-fallback-clip-ref.html
 fuzzy-if(OSX,0-42,0-4) fuzzy-if(winWidget,249-249,999-999) == 1617515-1.html 1617515-1-ref.html
+== 1624118.html 1624118-ref.html
 fuzzy(0-2,0-21184) == 1626259-1.html 1626259-1-ref.html
 fuzzy(0-2,0-21184) == 1626259-2.html 1626259-2-ref.html
 != 1642583-1.html 1642583-1-ref.html