Bug 1829594 - Fix pre-existing logic bug in nsHideViewer::Run. r=tnikkel,layout-reviewers

This is needed to fix layout/reftests/bugs/1743533-1.html after the
previous patch, which will otherwise perma-pass (which is actually
perma-fail).

The reason is a bug in nsHideViewer::Run, which now gets used for
fission iframes as well.

What happens is that mPresShell->IsDestroying() is true, but
frame->PresShell() is a different press shell in fact (because page mode
re-creates the pres shell).

We should not hide the frame loader there. Hiding it if
mPresShell->IsDestroying() is redundant anyways, even if the frame
belongs to mPresShell, because if there is a frame, that frame going
away will end up in nsHideViewer::Run again, eventually.

Depends on D187650

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

layout/generic/nsSubDocumentFrame.cpp

@@ -915,7 +915,7 @@ class nsHideViewer final : public Runnable {
     //
     // We should find some way to avoid that!
     if (!mPresShell->IsDestroying() && mFrameElement->IsInComposedDoc()) {
-      MOZ_KnownLive(mPresShell)->FlushPendingNotifications(FlushType::Frames);
+      mPresShell->FlushPendingNotifications(FlushType::Frames);
     }
 
     // Either the frame has been constructed by now, or it never will be,
@@ -923,12 +923,11 @@ class nsHideViewer final : public Runnable {
     mFrameLoader->SetDetachedSubdocFrame(nullptr, nullptr);
 
     nsSubDocumentFrame* frame = do_QueryFrame(mFrameElement->GetPrimaryFrame());
-    const bool presShellDestroying = mPresShell->IsDestroying();
-    if (!frame || presShellDestroying) {
+    if (!frame) {
       PropagateIsUnderHiddenEmbedderElement(mFrameLoader, true);
-      if (mHideViewerIfFrameless || presShellDestroying) {
-        // Either the frame element has no nsIFrame or the presshell is being
-        // destroyed. Hide the nsFrameLoader, which destroys the presentation.
+      if (mHideViewerIfFrameless) {
+        // The frame element has no nsIFrame. Hide the nsFrameLoader, which
+        // destroys the presentation.
         mFrameLoader->Hide();
       }
     }
@@ -936,10 +935,10 @@ class nsHideViewer final : public Runnable {
   }
 
  private:
-  nsCOMPtr<nsIContent> mFrameElement;
-  RefPtr<nsFrameLoader> mFrameLoader;
-  RefPtr<PresShell> mPresShell;
-  bool mHideViewerIfFrameless;
+  const nsCOMPtr<nsIContent> mFrameElement;
+  const RefPtr<nsFrameLoader> mFrameLoader;
+  const RefPtr<PresShell> mPresShell;
+  const bool mHideViewerIfFrameless;
 };
 
 static nsView* BeginSwapDocShellsForViews(nsView* aSibling);

layout/reftests/bugs/1743533-1.html

@@ -13,18 +13,8 @@ of a background color because background colors don't show up by default in
 print preview. We're using page mode here and not print preview, so that is
 not technically required here.
 
-The most obvious way to test this would be to compare an oop iframe and an
-in process iframe with the same content to make sure they are the same.
-However, for some reason, in reftest-paged mode a simple in process iframe
-seems to refuse to load the document, I think because of this code
-https://searchfox.org/mozilla-central/rev/3c9369510cb883b9f08d5f9641e1233d2142f025/dom/base/nsFrameLoader.cpp#439
-If the same iframe is oop then the document does load, this behaviour difference
-seems like it might be a bug. If that bug is ever fixed (so that the oop iframe here fails to
-load it's document) then writing a test to exercise the original bug here seems
-like it would be impossible.
-
-So we compare an oop iframe with content in it, to a blank iframe, and then
-we use a high minimum accepted number of pixels differing in the fuzz. This
+We compare an oop iframe with content in it, to a blank iframe, and then we use
+a high minimum accepted number of pixels differing in the fuzz. This
 then makes sure the two documents differ by enough pixels.
 
 And we skip running this test if fission is disabled, as that would just