Bug 1776289 part 2: Merge nsPrintJob::Initialize into the constructor. r=emilio

This patch doesn't change behavior; it's just collapsing logic from
nsPrintJob's Initialize method (which is now de-facto infallible) into the
constructor.

I'm also removing the "Methods needed by the DocViewer" header-comment since
it's clearly innacurate at this point. It's only surrounding this constructor
and GetSeqFrameAndCountSheets, which is silly since nsDocumentViewer uses more
of our API than that.

(I also placed TODO(dholbert) comments for a few things that looked odd, to
follow up on later. In particular, I noticed that nsDocumentViewer holds a
dedicated stack-owned RefPtr reference to nsPrintJob after creating it, which
superficially looks unnecessary. I don't want to risk changing behvior or
introducing a crash by removing that reference in this refactoring patch, so
I'm leaving that as-is and simply flagging it as suspicious.)

Depends on D150194

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

layout/base/nsDocumentViewer.cpp

@@ -2923,23 +2923,19 @@ nsDocumentViewer::Print(nsIPrintSettings* aPrintSettings,
   }
 
   OnDonePrinting();
-  RefPtr<nsPrintJob> printJob = new nsPrintJob();
 
   // Note: mContainer and mDocument are known to be non-null via null-checks
   // earlier in this function.
-  nsresult rv =
-      printJob->Initialize(*this, *mContainer, *mDocument,
-                           float(AppUnitsPerCSSInch()) /
-                               float(mDeviceContext->AppUnitsPerDevPixel()));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    printJob->Destroy();
-    return rv;
-  }
-
+  // TODO(dholbert) Do we need to bother with this stack-owned local RefPtr?
+  // (Is there an edge case where it's needed to keep the nsPrintJob alive?)
+  RefPtr<nsPrintJob> printJob =
+      new nsPrintJob(*this, *mContainer, *mDocument,
+                     float(AppUnitsPerCSSInch()) /
+                         float(mDeviceContext->AppUnitsPerDevPixel()));
   mPrintJob = printJob;
 
-  rv = printJob->Print(*mDocument, aPrintSettings, aRemotePrintJob,
-                       aWebProgressListener);
+  nsresult rv = printJob->Print(*mDocument, aPrintSettings, aRemotePrintJob,
+                                aWebProgressListener);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     OnDonePrinting();
   }
@@ -2971,22 +2967,18 @@ nsDocumentViewer::PrintPreview(nsIPrintSettings* aPrintSettings,
 
   OnDonePrinting();
 
-  RefPtr<nsPrintJob> printJob = new nsPrintJob();
-
   // Note: mContainer and doc are known to be non-null via null-checks earlier
   // in this function.
-  nsresult rv =
-      printJob->Initialize(*this, *mContainer, *doc,
-                           float(AppUnitsPerCSSInch()) /
-                               float(mDeviceContext->AppUnitsPerDevPixel()));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    printJob->Destroy();
-    return rv;
-  }
+  // TODO(dholbert) Do we need to bother with this stack-owned local RefPtr?
+  // (Is there an edge case where it's needed to keep the nsPrintJob alive?)
+  RefPtr<nsPrintJob> printJob =
+      new nsPrintJob(*this, *mContainer, *doc,
+                     float(AppUnitsPerCSSInch()) /
+                         float(mDeviceContext->AppUnitsPerDevPixel()));
   mPrintJob = printJob;
 
-  rv = printJob->PrintPreview(*doc, aPrintSettings, aWebProgressListener,
-                              std::move(aCallback));
+  nsresult rv = printJob->PrintPreview(
+      *doc, aPrintSettings, aWebProgressListener, std::move(aCallback));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     OnDonePrinting();
   }

layout/printing/nsPrintJob.cpp

@@ -231,8 +231,6 @@ static nsresult GetDefaultPrintSettings(nsIPrintSettings** aSettings) {
 NS_IMPL_ISUPPORTS(nsPrintJob, nsIWebProgressListener, nsISupportsWeakReference)
 
 //-------------------------------------------------------
-nsPrintJob::nsPrintJob() = default;
-
 nsPrintJob::~nsPrintJob() {
   Destroy();  // for insurance
   DisconnectPagePrintTimer();
@@ -258,19 +256,13 @@ void nsPrintJob::DestroyPrintingData() {
   mPrt = nullptr;
 }
 
-//---------------------------------------------------------------------------------
-//-- Section: Methods needed by the DocViewer
-//---------------------------------------------------------------------------------
-
-//--------------------------------------------------------
-nsresult nsPrintJob::Initialize(nsIDocumentViewerPrint& aDocViewerPrint,
-                                nsIDocShell& aDocShell, Document& aOriginalDoc,
-                                float aScreenDPI) {
-  mDocViewerPrint = &aDocViewerPrint;
-  mDocShell = do_GetWeakReference(&aDocShell);
-  mScreenDPI = aScreenDPI;
-
-  // Anything state that we need from aOriginalDoc must be fetched and stored
+nsPrintJob::nsPrintJob(nsIDocumentViewerPrint& aDocViewerPrint,
+                       nsIDocShell& aDocShell, Document& aOriginalDoc,
+                       float aScreenDPI)
+    : mDocViewerPrint(&aDocViewerPrint),
+      mDocShell(do_GetWeakReference(&aDocShell)),
+      mScreenDPI(aScreenDPI) {
+  // Any state that we need from aOriginalDoc must be fetched and stored
   // here, since the document that the user selected to print may mutate
   // across consecutive PrintPreview() calls.
 
@@ -287,8 +279,6 @@ nsresult nsPrintJob::Initialize(nsIDocumentViewerPrint& aDocViewerPrint,
       wbc->IsWindowModal(&mIsForModalWindow);
     }
   }
-
-  return NS_OK;
 }
 
 //-----------------------------------------------------------------
@@ -320,9 +310,6 @@ nsPrintJob::GetSeqFrameAndCountSheets() const {
   // count the total number of sheets
   return {seqFrame, seqFrame->PrincipalChildList().GetLength()};
 }
-//---------------------------------------------------------------------------------
-//-- Done: Methods needed by the DocViewer
-//---------------------------------------------------------------------------------
 
 // Foward decl for Debug Helper Functions
 #ifdef EXTENDED_DEBUG_PRINTING

layout/printing/nsPrintJob.h

@@ -57,15 +57,14 @@ class nsPrintJob final : public nsIWebProgressListener,
   using RemotePrintJobChild = mozilla::layout::RemotePrintJobChild;
 
  public:
-  nsPrintJob();
-
   // nsISupports interface...
   NS_DECL_ISUPPORTS
 
   NS_DECL_NSIWEBPROGRESSLISTENER
 
   /**
-   * Initialize for printing, or for creating a print preview document.
+   * Construct & initialize for printing, or for creating a print preview
+   * document.
    *
    * aDocViewerPrint owns us.
    *
@@ -86,10 +85,12 @@ class nsPrintJob final : public nsIWebProgressListener,
    * with a new docViewer and nsPrintJob, and passes the previous print preview
    * document as aOriginalDoc (it doesn't want to pass the actual original
    * document since it may have mutated)!
+   * TODO(dholbert): This^ note is probably somewhat out of date, in part
+   * because frontend code doesn't create "print preview tabs" anymore,
+   * now that we have a tab-modal print/print-preview dialog...
    */
-  nsresult Initialize(nsIDocumentViewerPrint& aDocViewerPrint,
-                      nsIDocShell& aDocShell, Document& aOriginalDoc,
-                      float aScreenDPI);
+  nsPrintJob(nsIDocumentViewerPrint& aDocViewerPrint, nsIDocShell& aDocShell,
+             Document& aOriginalDoc, float aScreenDPI);
 
   // Our nsIWebBrowserPrint implementation (nsDocumentViewer) defers to the
   // following methods.