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
--- 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);