Bug 1698235 - Add some error handling when using RemotePrintJobChild …

…which may be destroyed. r=TYLin Differential Revision: https://phabricator.services.mozilla.com/D108711
mozilla · Mar 18, 2021 · 6adb31f · 6adb31f
1 parent ea66000
commit 6adb31f
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 5 deletions.
diff --git a/layout/generic/nsPageSequenceFrame.cpp b/layout/generic/nsPageSequenceFrame.cpp
@@ -683,7 +683,7 @@ nsresult nsPageSequenceFrame::DoPageEnd() {
   if (PresContext()->IsRootPaginatedDocument()) {
     PR_PL(("***************** End Page (DoPageEnd) *****************\n"));
     rv = PresContext()->DeviceContext()->EndPage();
-    NS_ENSURE_SUCCESS(rv, rv);
+    // Fall through to clean up resources/state below even if EndPage failed.
   }
 
   ResetPrintCanvasList();

diff --git a/layout/printing/ipc/RemotePrintJobChild.cpp b/layout/printing/ipc/RemotePrintJobChild.cpp
@@ -43,6 +43,7 @@ mozilla::ipc::IPCResult RemotePrintJobChild::RecvPrintInitializationResult(
 }
 
 PRFileDesc* RemotePrintJobChild::GetNextPageFD() {
+  MOZ_ASSERT(!mDestroyed);
   MOZ_ASSERT(mNextPageFD);
   PRFileDesc* fd = mNextPageFD;
   mNextPageFD = nullptr;
@@ -51,6 +52,7 @@ PRFileDesc* RemotePrintJobChild::GetNextPageFD() {
 
 void RemotePrintJobChild::SetNextPageFD(
     const mozilla::ipc::FileDescriptor& aFd) {
+  MOZ_ASSERT(!mDestroyed);
   auto handle = aFd.ClonePlatformHandle();
   mNextPageFD = PR_ImportFile(PROsfd(handle.release()));
 }
@@ -82,12 +84,14 @@ mozilla::ipc::IPCResult RemotePrintJobChild::RecvAbortPrint(
 }
 
 void RemotePrintJobChild::SetPagePrintTimer(nsPagePrintTimer* aPagePrintTimer) {
+  MOZ_ASSERT(!mDestroyed);
   MOZ_ASSERT(aPagePrintTimer);
 
   mPagePrintTimer = aPagePrintTimer;
 }
 
 void RemotePrintJobChild::SetPrintJob(nsPrintJob* aPrintJob) {
+  MOZ_ASSERT(!mDestroyed);
   MOZ_ASSERT(aPrintJob);
 
   mPrintJob = aPrintJob;

diff --git a/layout/printing/ipc/RemotePrintJobChild.h b/layout/printing/ipc/RemotePrintJobChild.h
@@ -47,6 +47,8 @@ class RemotePrintJobChild final : public PRemotePrintJobChild,
 
   PRFileDesc* GetNextPageFD();
 
+  [[nodiscard]] bool IsDestroyed() const { return mDestroyed; }
+
  private:
   ~RemotePrintJobChild() final;
   void SetNextPageFD(const mozilla::ipc::FileDescriptor& aFd);

diff --git a/widget/nsDeviceContextSpecProxy.cpp b/widget/nsDeviceContextSpecProxy.cpp
@@ -136,6 +136,11 @@ NS_IMETHODIMP
 nsDeviceContextSpecProxy::BeginDocument(const nsAString& aTitle,
                                         const nsAString& aPrintToFileName,
                                         int32_t aStartPage, int32_t aEndPage) {
+  if (!mRemotePrintJob || mRemotePrintJob->IsDestroyed()) {
+    mRemotePrintJob = nullptr;
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   mRecorder = new mozilla::layout::DrawEventRecorderPRFileDesc();
   nsresult rv = mRemotePrintJob->InitializePrint(
       nsString(aTitle), nsString(aPrintToFileName), aStartPage, aEndPage);
@@ -151,29 +156,47 @@ nsDeviceContextSpecProxy::BeginDocument(const nsAString& aTitle,
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::EndDocument() {
-  if (mRemotePrintJob) {
-    Unused << mRemotePrintJob->SendFinalizePrint();
+  if (!mRemotePrintJob || mRemotePrintJob->IsDestroyed()) {
+    mRemotePrintJob = nullptr;
+    return NS_ERROR_NOT_AVAILABLE;
   }
+
+  Unused << mRemotePrintJob->SendFinalizePrint();
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::AbortDocument() {
-  if (mRemotePrintJob) {
-    Unused << mRemotePrintJob->SendAbortPrint(NS_OK);
+  if (!mRemotePrintJob || mRemotePrintJob->IsDestroyed()) {
+    mRemotePrintJob = nullptr;
+    return NS_ERROR_NOT_AVAILABLE;
   }
+
+  Unused << mRemotePrintJob->SendAbortPrint(NS_OK);
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::BeginPage() {
+  if (!mRemotePrintJob || mRemotePrintJob->IsDestroyed()) {
+    mRemotePrintJob = nullptr;
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   mRecorder->OpenFD(mRemotePrintJob->GetNextPageFD());
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDeviceContextSpecProxy::EndPage() {
+  if (!mRemotePrintJob || mRemotePrintJob->IsDestroyed()) {
+    mRemotePrintJob = nullptr;
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   // Send the page recording to the parent.
   mRecorder->Close();
   mRemotePrintJob->ProcessPage(std::move(mRecorder->TakeDependentSurfaces()));