Bug 1776074 part 4: Use references instead of pointers for nsPrintObject::Init's Document and nsDocShell args (and in callers). r=emilio

This patch doesn't change behavior. It just changes to c++ reference types and
code-comments to indicate where we know that pointers have been null-checked
(and removes some null-checks that now become trivially-unnecessary).

I've added code-comments to justify why we know these args are non-null.
Generally, `nsPrintObject::Init`'s args are null-checked by the caller, except
in one case (in `nsPrintJob::DoCommonPrint`) where the Document* pointer in
question was _not_ directly null-checked by the caller. But fortunately, it is
null-checked earlier, higher up in the call-stack.  So, this patch simply
propagates the C++ reference type-conversion up to that point for additional
clarity.

Differential Revision: https://phabricator.services.mozilla.com/D150078
This commit is contained in:
Daniel Holbert
2022-06-23 22:56:59 +00:00
parent 2578254c73
commit 574940c643
5 changed files with 26 additions and 22 deletions

View File

@@ -2934,7 +2934,9 @@ nsDocumentViewer::Print(nsIPrintSettings* aPrintSettings,
} }
mPrintJob = printJob; mPrintJob = printJob;
rv = printJob->Print(mDocument, aPrintSettings, aRemotePrintJob,
// Note: mDocument was null-checked earlier in this function.
rv = printJob->Print(*mDocument, aPrintSettings, aRemotePrintJob,
aWebProgressListener); aWebProgressListener);
if (NS_WARN_IF(NS_FAILED(rv))) { if (NS_WARN_IF(NS_FAILED(rv))) {
OnDonePrinting(); OnDonePrinting();
@@ -2979,7 +2981,8 @@ nsDocumentViewer::PrintPreview(nsIPrintSettings* aPrintSettings,
} }
mPrintJob = printJob; mPrintJob = printJob;
rv = printJob->PrintPreview(doc, aPrintSettings, aWebProgressListener, // Note: doc is known-non-null via NS_ENSURE_STATE null-check above.
rv = printJob->PrintPreview(*doc, aPrintSettings, aWebProgressListener,
std::move(aCallback)); std::move(aCallback));
if (NS_WARN_IF(NS_FAILED(rv))) { if (NS_WARN_IF(NS_FAILED(rv))) {
OnDonePrinting(); OnDonePrinting();

View File

@@ -196,7 +196,9 @@ void nsPrintJob::BuildNestedPrintObjects(
} }
auto childPO = MakeUnique<nsPrintObject>(); auto childPO = MakeUnique<nsPrintObject>();
nsresult rv = childPO->Init(docShell, doc, aParentPO.get()); // Note: docShell and doc are known-non-null at this point; they've been
// null-checked above (with null leading to 'continue' statements).
nsresult rv = childPO->Init(*docShell, *doc, aParentPO.get());
if (NS_FAILED(rv)) { if (NS_FAILED(rv)) {
MOZ_ASSERT_UNREACHABLE("Init failed?"); MOZ_ASSERT_UNREACHABLE("Init failed?");
} }
@@ -343,7 +345,7 @@ static void DumpLayoutData(const char* aTitleStr, const char* aURLStr,
nsresult nsPrintJob::CommonPrint(bool aIsPrintPreview, nsresult nsPrintJob::CommonPrint(bool aIsPrintPreview,
nsIPrintSettings* aPrintSettings, nsIPrintSettings* aPrintSettings,
nsIWebProgressListener* aWebProgressListener, nsIWebProgressListener* aWebProgressListener,
Document* aSourceDoc) { Document& aSourceDoc) {
// Callers must hold a strong reference to |this| to ensure that we stay // Callers must hold a strong reference to |this| to ensure that we stay
// alive for the duration of this method, because our main owning reference // alive for the duration of this method, because our main owning reference
// (on nsDocumentViewer) might be cleared during this function (if we cause // (on nsDocumentViewer) might be cleared during this function (if we cause
@@ -370,8 +372,8 @@ nsresult nsPrintJob::CommonPrint(bool aIsPrintPreview,
nsresult nsPrintJob::DoCommonPrint(bool aIsPrintPreview, nsresult nsPrintJob::DoCommonPrint(bool aIsPrintPreview,
nsIPrintSettings* aPrintSettings, nsIPrintSettings* aPrintSettings,
nsIWebProgressListener* aWebProgressListener, nsIWebProgressListener* aWebProgressListener,
Document* aDoc) { Document& aDoc) {
MOZ_ASSERT(aDoc->IsStaticDocument()); MOZ_ASSERT(aDoc.IsStaticDocument());
nsresult rv; nsresult rv;
@@ -411,7 +413,9 @@ nsresult nsPrintJob::DoCommonPrint(bool aIsPrintPreview,
{ {
nsAutoScriptBlocker scriptBlocker; nsAutoScriptBlocker scriptBlocker;
mPrintObject = MakeUnique<nsPrintObject>(); mPrintObject = MakeUnique<nsPrintObject>();
rv = mPrintObject->Init(docShell, aDoc); // Note: docShell is implicitly non-null via do_QueryReferent necessarily
// having succeeded (if we got here).
rv = mPrintObject->Init(*docShell, aDoc);
NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_SUCCESS(rv, rv);
mPrintDocList.AppendElement(mPrintObject.get()); mPrintDocList.AppendElement(mPrintObject.get());
@@ -469,14 +473,14 @@ nsresult nsPrintJob::DoCommonPrint(bool aIsPrintPreview,
} }
//--------------------------------------------------------------------------------- //---------------------------------------------------------------------------------
nsresult nsPrintJob::Print(Document* aDoc, nsIPrintSettings* aPrintSettings, nsresult nsPrintJob::Print(Document& aDoc, nsIPrintSettings* aPrintSettings,
RemotePrintJobChild* aRemotePrintJob, RemotePrintJobChild* aRemotePrintJob,
nsIWebProgressListener* aWebProgressListener) { nsIWebProgressListener* aWebProgressListener) {
mRemotePrintJob = aRemotePrintJob; mRemotePrintJob = aRemotePrintJob;
return CommonPrint(false, aPrintSettings, aWebProgressListener, aDoc); return CommonPrint(false, aPrintSettings, aWebProgressListener, aDoc);
} }
nsresult nsPrintJob::PrintPreview(Document* aDoc, nsresult nsPrintJob::PrintPreview(Document& aDoc,
nsIPrintSettings* aPrintSettings, nsIPrintSettings* aPrintSettings,
nsIWebProgressListener* aWebProgressListener, nsIWebProgressListener* aWebProgressListener,
PrintPreviewResolver&& aCallback) { PrintPreviewResolver&& aCallback) {

View File

@@ -99,7 +99,7 @@ class nsPrintJob final : public nsIWebProgressListener,
* PrintPreview calls. * PrintPreview calls.
*/ */
MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult
Print(Document* aSourceDoc, nsIPrintSettings* aPrintSettings, Print(Document& aSourceDoc, nsIPrintSettings* aPrintSettings,
RemotePrintJobChild* aRemotePrintJob, RemotePrintJobChild* aRemotePrintJob,
nsIWebProgressListener* aWebProgressListener); nsIWebProgressListener* aWebProgressListener);
@@ -114,7 +114,7 @@ class nsPrintJob final : public nsIWebProgressListener,
* is actually our docViewer's current document! * is actually our docViewer's current document!
*/ */
MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult
PrintPreview(Document* aSourceDoc, nsIPrintSettings* aPrintSettings, PrintPreview(Document& aSourceDoc, nsIPrintSettings* aPrintSettings,
nsIWebProgressListener* aWebProgressListener, nsIWebProgressListener* aWebProgressListener,
PrintPreviewResolver&& aCallback); PrintPreviewResolver&& aCallback);
@@ -199,11 +199,11 @@ class nsPrintJob final : public nsIWebProgressListener,
MOZ_CAN_RUN_SCRIPT nsresult CommonPrint( MOZ_CAN_RUN_SCRIPT nsresult CommonPrint(
bool aIsPrintPreview, nsIPrintSettings* aPrintSettings, bool aIsPrintPreview, nsIPrintSettings* aPrintSettings,
nsIWebProgressListener* aWebProgressListener, Document* aSourceDoc); nsIWebProgressListener* aWebProgressListener, Document& aSourceDoc);
MOZ_CAN_RUN_SCRIPT nsresult DoCommonPrint( MOZ_CAN_RUN_SCRIPT nsresult DoCommonPrint(
bool aIsPrintPreview, nsIPrintSettings* aPrintSettings, bool aIsPrintPreview, nsIPrintSettings* aPrintSettings,
nsIWebProgressListener* aWebProgressListener, Document* aSourceDoc); nsIWebProgressListener* aWebProgressListener, Document& aSourceDoc);
void FirePrintCompletionEvent(); void FirePrintCompletionEvent();

View File

@@ -49,15 +49,12 @@ nsPrintObject::~nsPrintObject() {
//------------------------------------------------------------------ //------------------------------------------------------------------
nsresult nsPrintObject::Init(nsIDocShell* aDocShell, Document* aDoc, nsresult nsPrintObject::Init(nsIDocShell& aDocShell, Document& aDoc,
nsPrintObject* aParent) { nsPrintObject* aParent) {
NS_ENSURE_STATE(aDocShell); MOZ_ASSERT(aDoc.IsStaticDocument());
NS_ENSURE_STATE(aDoc);
MOZ_ASSERT(aDoc->IsStaticDocument()); mDocShell = &aDocShell;
mDocument = &aDoc;
mDocShell = aDocShell;
mDocument = aDoc;
if (!aParent) { if (!aParent) {
// We are a root nsPrintObject. // We are a root nsPrintObject.
@@ -66,7 +63,7 @@ nsresult nsPrintObject::Init(nsIDocShell* aDocShell, Document* aDoc,
} else { } else {
// We are a nested nsPrintObject. // We are a nested nsPrintObject.
mParent = aParent; mParent = aParent;
nsCOMPtr<nsPIDOMWindowOuter> window = aDoc->GetWindow(); nsCOMPtr<nsPIDOMWindowOuter> window = aDoc.GetWindow();
mContent = window->GetFrameElementInternal(); mContent = window->GetFrameElementInternal();
mFrameType = eIFrame; mFrameType = eIFrame;
} }

View File

@@ -36,7 +36,7 @@ class nsPrintObject final {
* If aParent is nullptr (default), then this instance will be initialized as * If aParent is nullptr (default), then this instance will be initialized as
* a "root" nsPrintObject. Otherwise, this will be a "nested" nsPrintObject. * a "root" nsPrintObject. Otherwise, this will be a "nested" nsPrintObject.
*/ */
nsresult Init(nsIDocShell* aDocShell, mozilla::dom::Document* aDoc, nsresult Init(nsIDocShell& aDocShell, mozilla::dom::Document& aDoc,
nsPrintObject* aParent = nullptr); nsPrintObject* aParent = nullptr);
void DestroyPresentation(); void DestroyPresentation();