Bug 1376693 - part3: Make callers of nsPrintEngine::Print() and nsPrintEngine::PrintPreview() guarantee that instance of nsPrintEngine won't be deleted during the calls r?dholbert draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 19 Jul 2017 14:06:04 +0900
changeset 611019 8fe2a527efd9e3c6176ad5d28544f30579cf5f81
parent 611018 3898c2d70c71aeb33eecc79d52cf7c7f7199ddf6
child 638045 447ee03e748b82811b037bf85bc8ac70d1a76ef7
push id69097
push usermasayuki@d-toybox.com
push dateWed, 19 Jul 2017 05:32:07 +0000
reviewersdholbert
bugs1376693
milestone56.0a1
Bug 1376693 - part3: Make callers of nsPrintEngine::Print() and nsPrintEngine::PrintPreview() guarantee that instance of nsPrintEngine won't be deleted during the calls r?dholbert This patch makes callers of nsPrintEngine::Print() and nsPrintEngine::PrintPreview() grab the nsPrintEngine instance with local variable before calling them. That guarantees that instance of nPrintEngine won't be deleted during the calls. (We already had a RefPtr in CommonPrint that basically did this. This patch moves it out to the callers to strengthen its guarantee.) MozReview-Commit-ID: 2jlYC4RKAg6
layout/base/nsDocumentViewer.cpp
layout/printing/nsPrintEngine.cpp
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -4011,46 +4011,50 @@ nsDocumentViewer::Print(nsIPrintSettings
     new AutoPrintEventDispatcher(mDocument));
   NS_ENSURE_STATE(!GetIsPrinting());
   // If we are hosting a full-page plugin, tell it to print
   // first. It shows its own native print UI.
   nsCOMPtr<nsIPluginDocument> pDoc(do_QueryInterface(mDocument));
   if (pDoc)
     return pDoc->Print();
 
-  if (!mPrintEngine) {
+  // Our call to nsPrintEngine::Print() may cause mPrintEngine to be
+  // Release()'d in Destroy().  Therefore, we need to grab the instance with
+  // a local variable, so that it won't be deleted during its own method.
+  RefPtr<nsPrintEngine> printEngine = mPrintEngine;
+  if (!printEngine) {
     NS_ENSURE_STATE(mDeviceContext);
-    mPrintEngine = new nsPrintEngine();
-
-    rv = mPrintEngine->Initialize(this, mContainer, mDocument,
-                                  float(mDeviceContext->AppUnitsPerCSSInch()) /
-                                  float(mDeviceContext->AppUnitsPerDevPixel()) /
-                                  mPageZoom,
+    printEngine = new nsPrintEngine();
+
+    rv = printEngine->Initialize(this, mContainer, mDocument,
+                                 float(mDeviceContext->AppUnitsPerCSSInch()) /
+                                 float(mDeviceContext->AppUnitsPerDevPixel()) /
+                                 mPageZoom,
 #ifdef DEBUG
-                                  mDebugFile
+                                 mDebugFile
 #else
-                                  nullptr
+                                 nullptr
 #endif
-                                  );
+                                 );
     if (NS_FAILED(rv)) {
-      mPrintEngine->Destroy();
-      mPrintEngine = nullptr;
+      printEngine->Destroy();
       return rv;
     }
-  }
-  if (mPrintEngine->HasPrintCallbackCanvas()) {
+    mPrintEngine = printEngine;
+  }
+  if (printEngine->HasPrintCallbackCanvas()) {
     // Postpone the 'afterprint' event until after the mozPrintCallback
     // callbacks have been called:
     mAutoBeforeAndAfterPrint = autoBeforeAndAfterPrint;
   }
   dom::Element* root = mDocument->GetRootElement();
   if (root && root->HasAttr(kNameSpaceID_None, nsGkAtoms::mozdisallowselectionprint)) {
-    mPrintEngine->SetDisallowSelectionPrint(true);
-  }
-  rv = mPrintEngine->Print(aPrintSettings, aWebProgressListener);
+    printEngine->SetDisallowSelectionPrint(true);
+  }
+  rv = printEngine->Print(aPrintSettings, aWebProgressListener);
   if (NS_FAILED(rv)) {
     OnDonePrinting();
   }
   return rv;
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::PrintPreview(nsIPrintSettings* aPrintSettings,
@@ -4101,47 +4105,53 @@ nsDocumentViewer::PrintPreview(nsIPrintS
   nsAutoPtr<AutoPrintEventDispatcher> autoBeforeAndAfterPrint;
   if (!mAutoBeforeAndAfterPrint) {
     autoBeforeAndAfterPrint = new AutoPrintEventDispatcher(doc);
   }
   NS_ENSURE_STATE(!GetIsPrinting());
   // beforeprint event may have caused ContentViewer to be shutdown.
   NS_ENSURE_STATE(mContainer);
   NS_ENSURE_STATE(mDeviceContext);
-  if (!mPrintEngine) {
-    mPrintEngine = new nsPrintEngine();
-
-    rv = mPrintEngine->Initialize(this, mContainer, doc,
-                                  float(mDeviceContext->AppUnitsPerCSSInch()) /
-                                  float(mDeviceContext->AppUnitsPerDevPixel()) /
-                                  mPageZoom,
+
+  // Our call to nsPrintEngine::PrintPreview() may cause mPrintEngine to be
+  // Release()'d in Destroy().  Therefore, we need to grab the instance with
+  // a local variable, so that it won't be deleted during its own method.
+  RefPtr<nsPrintEngine> printEngine = mPrintEngine;
+  if (!printEngine) {
+    printEngine = new nsPrintEngine();
+
+    rv = printEngine->Initialize(this, mContainer, doc,
+                                 float(mDeviceContext->AppUnitsPerCSSInch()) /
+                                 float(mDeviceContext->AppUnitsPerDevPixel()) /
+                                 mPageZoom,
 #ifdef DEBUG
-                                  mDebugFile
+                                 mDebugFile
 #else
-                                  nullptr
+                                 nullptr
 #endif
-                                  );
+                                 );
     if (NS_FAILED(rv)) {
-      mPrintEngine->Destroy();
-      mPrintEngine = nullptr;
+      printEngine->Destroy();
       return rv;
     }
+    mPrintEngine = printEngine;
   }
   if (autoBeforeAndAfterPrint &&
-      mPrintEngine->HasPrintCallbackCanvas()) {
+      printEngine->HasPrintCallbackCanvas()) {
     // Postpone the 'afterprint' event until after the mozPrintCallback
     // callbacks have been called:
     mAutoBeforeAndAfterPrint = autoBeforeAndAfterPrint;
   }
   dom::Element* root = doc->GetRootElement();
   if (root && root->HasAttr(kNameSpaceID_None, nsGkAtoms::mozdisallowselectionprint)) {
     PR_PL(("PrintPreview: found mozdisallowselectionprint"));
-    mPrintEngine->SetDisallowSelectionPrint(true);
-  }
-  rv = mPrintEngine->PrintPreview(aPrintSettings, aChildDOMWin, aWebProgressListener);
+    printEngine->SetDisallowSelectionPrint(true);
+  }
+  rv = printEngine->PrintPreview(aPrintSettings, aChildDOMWin,
+                                 aWebProgressListener);
   mPrintPreviewZoomed = false;
   if (NS_FAILED(rv)) {
     OnDonePrinting();
   }
   return rv;
 #else
   return NS_ERROR_FAILURE;
 #endif
@@ -4646,16 +4656,20 @@ nsDocumentViewer::ReturnToGalleyPresenta
 //   up when an error occurred during the start up printing
 //   and print preview
 //
 void
 nsDocumentViewer::OnDonePrinting()
 {
 #if defined(NS_PRINTING) && defined(NS_PRINT_PREVIEW)
   SetPrintRelated();
+  // If Destroy() has been called during calling nsPrintEngine::Print() or
+  // nsPrintEngine::PrintPreview(), mPrintEngine is already nullptr here.
+  // So, the following clean up does nothing in such case.
+  // (Do we need some of this for that case?)
   if (mPrintEngine) {
     RefPtr<nsPrintEngine> pe = mPrintEngine;
     if (GetIsPrintPreview()) {
       pe->DestroyPrintingData();
     } else {
       mPrintEngine = nullptr;
       pe->Destroy();
     }
--- a/layout/printing/nsPrintEngine.cpp
+++ b/layout/printing/nsPrintEngine.cpp
@@ -387,18 +387,23 @@ static void DumpLayoutData(char* aTitleS
 #endif
 
 //--------------------------------------------------------------------------------
 
 nsresult
 nsPrintEngine::CommonPrint(bool                    aIsPrintPreview,
                            nsIPrintSettings*       aPrintSettings,
                            nsIWebProgressListener* aWebProgressListener,
-                           nsIDOMDocument* aDoc) {
-  RefPtr<nsPrintEngine> kungfuDeathGrip = this;
+                           nsIDOMDocument* aDoc)
+{
+  // 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
+  // (on nsDocumentViewer) might be cleared during this function (if we cause
+  // script to run and it cancels the print operation).
+
   nsresult rv = DoCommonPrint(aIsPrintPreview, aPrintSettings,
                               aWebProgressListener, aDoc);
   if (NS_FAILED(rv)) {
     if (aIsPrintPreview) {
       SetIsCreatingPrintPreview(false);
       SetIsPrintPreview(false);
     } else {
       SetIsPrinting(false);