Bug 1309272, part 5 - Rework the macOS printing code to get rid of the hacks that create a new PrintTarget for each page. r=lsalzman
☠☠ backed out by bc39d8cde9ab ☠ ☠
authorJonathan Watt <jwatt@jwatt.org>
Mon, 21 Nov 2016 15:07:09 +0000
changeset 324467 a9f6271115de9f3cb7e3bea8f4fcbb04d41ace08
parent 324466 125e6fb37319d4767df61d82af7014094163dccb
child 324468 31fdab31ab5128a693c0f58fc1b3a2ec86ccb698
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewerslsalzman
bugs1309272
milestone53.0a1
Bug 1309272, part 5 - Rework the macOS printing code to get rid of the hacks that create a new PrintTarget for each page. r=lsalzman
gfx/src/nsDeviceContext.cpp
gfx/src/nsDeviceContext.h
gfx/thebes/PrintTargetCG.cpp
gfx/thebes/PrintTargetCG.h
gfx/thebes/PrintTargetCG.mm
gfx/thebes/moz.build
widget/cocoa/nsDeviceContextSpecX.mm
--- a/gfx/src/nsDeviceContext.cpp
+++ b/gfx/src/nsDeviceContext.cpp
@@ -235,21 +235,17 @@ nsDeviceContext::FontMetricsDeleted(cons
         mFontCache->FontMetricsDeleted(aFontMetrics);
     }
     return NS_OK;
 }
 
 bool
 nsDeviceContext::IsPrinterContext()
 {
-  return mPrintTarget != nullptr
-#ifdef XP_MACOSX
-         || mCachedPrintTarget != nullptr
-#endif
-         ;
+  return mPrintTarget != nullptr;
 }
 
 void
 nsDeviceContext::SetDPI(double* aScale)
 {
     float dpi = -1.0f;
 
     // Use the printing DC to determine DPI values, if we have one.
@@ -342,60 +338,52 @@ nsDeviceContext::CreateReferenceRenderin
 }
 
 already_AddRefed<gfxContext>
 nsDeviceContext::CreateRenderingContextCommon(bool aWantReferenceContext)
 {
     MOZ_ASSERT(IsPrinterContext());
     MOZ_ASSERT(mWidth > 0 && mHeight > 0);
 
-    RefPtr<PrintTarget> printingTarget = mPrintTarget;
-#ifdef XP_MACOSX
-    // CreateRenderingContext() can be called (on reflow) after EndPage()
-    // but before BeginPage().  On OS X (and only there) mPrintTarget
-    // will in this case be null, because OS X printing surfaces are
-    // per-page, and therefore only truly valid between calls to BeginPage()
-    // and EndPage().  But we can get away with fudging things here, if need
-    // be, by using a cached copy.
-    if (!printingTarget) {
-      printingTarget = mCachedPrintTarget;
-    }
-#endif
-
     // This will usually be null, depending on the pref print.print_via_parent.
     RefPtr<DrawEventRecorder> recorder;
     mDeviceContextSpec->GetDrawEventRecorder(getter_AddRefs(recorder));
 
     RefPtr<gfx::DrawTarget> dt;
     if (aWantReferenceContext) {
-      dt = printingTarget->GetReferenceDrawTarget(recorder);
+      dt = mPrintTarget->GetReferenceDrawTarget(recorder);
     } else {
-      dt = printingTarget->MakeDrawTarget(gfx::IntSize(mWidth, mHeight), recorder);
+      dt = mPrintTarget->MakeDrawTarget(gfx::IntSize(mWidth, mHeight), recorder);
     }
 
     if (!dt || !dt->IsValid()) {
       gfxCriticalNote
         << "Failed to create draw target in device context sized "
-        << mWidth << "x" << mHeight << " and pointers "
-        << hexa(mPrintTarget) << " and " << hexa(printingTarget);
+        << mWidth << "x" << mHeight << " and pointer "
+        << hexa(mPrintTarget);
       return nullptr;
     }
 
 #ifdef XP_MACOSX
+    // The CGContextRef provided by PMSessionGetCGGraphicsContext is
+    // write-only, so we need to prevent gfxContext::PushGroupAndCopyBackground
+    // trying to read from it or else we'll crash.
+    // XXXjwatt Consider adding a MakeDrawTarget override to PrintTargetCG and
+    // moving this AddUserData call there.
     dt->AddUserData(&gfxContext::sDontUseAsSourceKey, dt, nullptr);
 #endif
     dt->AddUserData(&sDisablePixelSnapping, (void*)0x1, nullptr);
 
     RefPtr<gfxContext> pContext = gfxContext::CreateOrNull(dt);
     MOZ_ASSERT(pContext); // already checked draw target above
 
     gfxMatrix transform;
-    if (printingTarget->RotateNeededForLandscape()) {
+    if (mPrintTarget->RotateNeededForLandscape()) {
       // Rotate page 90 degrees to draw landscape page on portrait paper
-      IntSize size = printingTarget->GetSize();
+      IntSize size = mPrintTarget->GetSize();
       transform.Translate(gfxPoint(0, size.width));
       gfxMatrix rotate(0, -1,
                        1,  0,
                        0,  0);
       transform = rotate * transform;
     }
     transform.Scale(mPrintingScale, mPrintingScale);
 
@@ -504,79 +492,62 @@ nsDeviceContext::BeginDocument(const nsA
 }
 
 
 nsresult
 nsDeviceContext::EndDocument(void)
 {
     nsresult rv = NS_OK;
 
-    if (mPrintTarget) {
-        rv = mPrintTarget->EndPrinting();
-        if (NS_SUCCEEDED(rv))
-            mPrintTarget->Finish();
+    rv = mPrintTarget->EndPrinting();
+    if (NS_SUCCEEDED(rv)) {
+        mPrintTarget->Finish();
     }
 
     if (mDeviceContextSpec)
         mDeviceContextSpec->EndDocument();
 
+    mPrintTarget = nullptr;
+
     return rv;
 }
 
 
 nsresult
 nsDeviceContext::AbortDocument(void)
 {
     nsresult rv = mPrintTarget->AbortPrinting();
 
     if (mDeviceContextSpec)
         mDeviceContextSpec->EndDocument();
 
+    mPrintTarget = nullptr;
+
     return rv;
 }
 
 
 nsresult
 nsDeviceContext::BeginPage(void)
 {
     nsresult rv = NS_OK;
 
     if (mDeviceContextSpec)
         rv = mDeviceContextSpec->BeginPage();
 
     if (NS_FAILED(rv)) return rv;
 
-#ifdef XP_MACOSX
-    // We need to get a new surface for each page on the Mac, as the
-    // CGContextRefs are only good for one page.
-    mPrintTarget = mDeviceContextSpec->MakePrintTarget();
-#endif
-
-    rv = mPrintTarget->BeginPage();
-
-    return rv;
+    return mPrintTarget->BeginPage();
 }
 
 nsresult
 nsDeviceContext::EndPage(void)
 {
     nsresult rv = mPrintTarget->EndPage();
 
-#ifdef XP_MACOSX
-    // We need to release the CGContextRef in the surface here, plus it's
-    // not something you would want anyway, as these CGContextRefs are only
-    // good for one page.  But we need to keep a cached reference to it, since
-    // CreateRenderingContext() may try to access it when mPrintTarget
-    // would normally be null.  See bug 665218.  If we just stop nulling out
-    // mPrintTarget here (and thereby make that our cached copy), we'll
-    // break all our null checks on mPrintTarget.  See bug 684622.
-    mCachedPrintTarget = mPrintTarget;
-    mPrintTarget = nullptr;
-#endif
-
     if (mDeviceContextSpec)
         mDeviceContextSpec->EndPage();
 
     return rv;
 }
 
 void
 nsDeviceContext::ComputeClientRectUsingScreen(nsRect* outRect)
@@ -656,20 +627,16 @@ nsDeviceContext::FindScreen(nsIScreen** 
     if (!(*outScreen)) {
         mScreenManager->GetPrimaryScreen(outScreen);
     }
 }
 
 bool
 nsDeviceContext::CalcPrintingSize()
 {
-    if (!mPrintTarget) {
-        return (mWidth > 0 && mHeight > 0);
-    }
-
     gfxSize size = mPrintTarget->GetSize();
     // For printing, CSS inches and physical inches are identical
     // so it doesn't matter which we use here
     mWidth = NSToCoordRound(size.width * AppUnitsPerPhysicalInch()
                             / POINTS_PER_INCH_FLOAT);
     mHeight = NSToCoordRound(size.height * AppUnitsPerPhysicalInch()
                              / POINTS_PER_INCH_FLOAT);
 
--- a/gfx/src/nsDeviceContext.h
+++ b/gfx/src/nsDeviceContext.h
@@ -295,17 +295,14 @@ private:
     float    mFullZoom;
     float    mPrintingScale;
 
     RefPtr<nsFontCache>            mFontCache;
     nsCOMPtr<nsIWidget>            mWidget;
     nsCOMPtr<nsIScreenManager>     mScreenManager;
     nsCOMPtr<nsIDeviceContextSpec> mDeviceContextSpec;
     RefPtr<PrintTarget>            mPrintTarget;
-#ifdef XP_MACOSX
-    RefPtr<PrintTarget>            mCachedPrintTarget;
-#endif
 #ifdef DEBUG
     bool mIsInitialized;
 #endif
 };
 
 #endif /* _NS_DEVICECONTEXT_H_ */
--- a/gfx/thebes/PrintTargetCG.h
+++ b/gfx/thebes/PrintTargetCG.h
@@ -9,34 +9,33 @@
 #include <Carbon/Carbon.h>
 #include "PrintTarget.h"
 
 namespace mozilla {
 namespace gfx {
 
 /**
  * CoreGraphics printing target.
- *
- * Note that a CGContextRef obtained from PMSessionGetCGGraphicsContext is
- * valid only for the current page.  As a consequence instances of this class
- * should only be used to print a single page.
  */
 class PrintTargetCG final : public PrintTarget
 {
 public:
   static already_AddRefed<PrintTargetCG>
-  CreateOrNull(const IntSize& aSize, gfxImageFormat aFormat);
+  CreateOrNull(PMPrintSession aPrintSession, const IntSize& aSize);
 
-  static already_AddRefed<PrintTargetCG>
-  CreateOrNull(CGContextRef aContext, const IntSize& aSize);
+  virtual nsresult BeginPage() final;
+  virtual nsresult EndPage() final;
 
   virtual already_AddRefed<DrawTarget>
   GetReferenceDrawTarget(DrawEventRecorder* aRecorder) final;
 
 private:
-  PrintTargetCG(cairo_surface_t* aCairoSurface,
+  PrintTargetCG(PMPrintSession aPrintSession,
                 const IntSize& aSize);
+  ~PrintTargetCG();
+
+  PMPrintSession mPrintSession;
 };
 
 } // namespace gfx
 } // namespace mozilla
 
 #endif /* MOZILLA_GFX_PRINTTARGETCG_H */
rename from gfx/thebes/PrintTargetCG.cpp
rename to gfx/thebes/PrintTargetCG.mm
--- a/gfx/thebes/PrintTargetCG.cpp
+++ b/gfx/thebes/PrintTargetCG.mm
@@ -3,71 +3,54 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "PrintTargetCG.h"
 
 #include "cairo.h"
 #include "cairo-quartz.h"
 #include "mozilla/gfx/HelpersCairo.h"
+#include "nsObjCExceptions.h"
 
 namespace mozilla {
 namespace gfx {
 
-PrintTargetCG::PrintTargetCG(cairo_surface_t* aCairoSurface,
+PrintTargetCG::PrintTargetCG(PMPrintSession aPrintSession,
                              const IntSize& aSize)
-  : PrintTarget(aCairoSurface, aSize)
+  : PrintTarget(/* aCairoSurface */ nullptr, aSize)
+  , mPrintSession(aPrintSession)
 {
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
+
+  ::PMRetain(mPrintSession);
+
   // TODO: Add memory reporting like gfxQuartzSurface.
   //RecordMemoryUsed(mSize.height * 4 + sizeof(gfxQuartzSurface));
+
+  NS_OBJC_END_TRY_ABORT_BLOCK;
+}
+
+PrintTargetCG::~PrintTargetCG()
+{
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
+
+  if (mPrintSession)
+    ::PMRelease(mPrintSession);
+
+  NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
 /* static */ already_AddRefed<PrintTargetCG>
-PrintTargetCG::CreateOrNull(const IntSize& aSize, gfxImageFormat aFormat)
+PrintTargetCG::CreateOrNull(PMPrintSession aPrintSession, const IntSize& aSize)
 {
   if (!Factory::CheckSurfaceSize(aSize)) {
     return nullptr;
   }
 
-  unsigned int width = static_cast<unsigned int>(aSize.width);
-  unsigned int height = static_cast<unsigned int>(aSize.height);
-
-  cairo_format_t cformat = GfxFormatToCairoFormat(aFormat);
-  cairo_surface_t* surface =
-    cairo_quartz_surface_create(cformat, width, height);
-
-  if (cairo_surface_status(surface)) {
-    return nullptr;
-  }
-
-  // The new object takes ownership of our surface reference.
-  RefPtr<PrintTargetCG> target = new PrintTargetCG(surface, aSize);
-
-  return target.forget();
-}
-
-/* static */ already_AddRefed<PrintTargetCG>
-PrintTargetCG::CreateOrNull(CGContextRef aContext, const IntSize& aSize)
-{
-  if (!Factory::CheckSurfaceSize(aSize)) {
-    return nullptr;
-  }
-
-  unsigned int width = static_cast<unsigned int>(aSize.width);
-  unsigned int height = static_cast<unsigned int>(aSize.height);
-
-  cairo_surface_t* surface =
-    cairo_quartz_surface_create_for_cg_context(aContext, width, height);
-
-  if (cairo_surface_status(surface)) {
-    return nullptr;
-  }
-
-  // The new object takes ownership of our surface reference.
-  RefPtr<PrintTargetCG> target = new PrintTargetCG(surface, aSize);
+  RefPtr<PrintTargetCG> target = new PrintTargetCG(aPrintSession, aSize);
 
   return target.forget();
 }
 
 static size_t
 PutBytesNull(void* info, const void* buffer, size_t count)
 {
   return count;
@@ -111,10 +94,54 @@ PrintTargetCG::GetReferenceDrawTarget(Dr
       }
     }
 
     mRefDT = dt.forget();
   }
   return do_AddRef(mRefDT);
 }
 
+nsresult
+PrintTargetCG::BeginPage()
+{
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
+
+  CGContextRef context;
+  // This call will fail if we are not called between the PMSessionBeginPage/
+  // PMSessionEndPage calls:
+  ::PMSessionGetCGGraphicsContext(mPrintSession, &context);
+
+  if (!context) {
+    return NS_ERROR_FAILURE;
+  }
+
+  unsigned int width = static_cast<unsigned int>(mSize.width);
+  unsigned int height = static_cast<unsigned int>(mSize.height);
+
+  // Initially, origin is at bottom-left corner of the paper.
+  // Here, we translate it to top-left corner of the paper.
+  CGContextTranslateCTM(context, 0, height);
+  CGContextScaleCTM(context, 1.0, -1.0);
+
+  cairo_surface_t* surface =
+    cairo_quartz_surface_create_for_cg_context(context, width, height);
+
+  if (cairo_surface_status(surface)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  mCairoSurface = surface;
+
+  return PrintTarget::BeginPage();
+
+  NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
+}
+
+nsresult
+PrintTargetCG::EndPage()
+{
+  cairo_surface_finish(mCairoSurface);
+  mCairoSurface = nullptr;
+  return PrintTarget::EndPage();
+}
+
 } // namespace gfx
 } // namespace mozilla
--- a/gfx/thebes/moz.build
+++ b/gfx/thebes/moz.build
@@ -84,17 +84,17 @@ elif CONFIG['MOZ_WIDGET_TOOLKIT'] == 'co
         'PrintTargetCG.h',
     ]
     SOURCES += [
         'gfxCoreTextShaper.cpp',
         'gfxMacFont.cpp',
         'gfxPlatformMac.cpp',
         'gfxQuartzNativeDrawing.cpp',
         'gfxQuartzSurface.cpp',
-        'PrintTargetCG.cpp',
+        'PrintTargetCG.mm',
     ]
 elif 'gtk' in CONFIG['MOZ_WIDGET_TOOLKIT']:
     EXPORTS += [
         'gfxFontconfigFonts.h',
         'gfxFT2FontBase.h',
         'gfxGdkNativeRenderer.h',
         'gfxPlatformGtk.h',
     ]
--- a/widget/cocoa/nsDeviceContextSpecX.mm
+++ b/widget/cocoa/nsDeviceContextSpecX.mm
@@ -134,32 +134,16 @@ void nsDeviceContextSpecX::GetPaperRect(
     *aTop = paperRect.top, *aLeft = paperRect.left;
     *aBottom = paperRect.bottom, *aRight = paperRect.right;
 
     NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
 already_AddRefed<PrintTarget> nsDeviceContextSpecX::MakePrintTarget()
 {
-    NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
-
     double top, left, bottom, right;
     GetPaperRect(&top, &left, &bottom, &right);
     const double width = right - left;
     const double height = bottom - top;
     IntSize size = IntSize::Floor(width, height);
 
-    CGContextRef context;
-    ::PMSessionGetCGGraphicsContext(mPrintSession, &context);
-
-    if (context) {
-        // Initially, origin is at bottom-left corner of the paper.
-        // Here, we translate it to top-left corner of the paper.
-        CGContextTranslateCTM(context, 0, height);
-        CGContextScaleCTM(context, 1.0, -1.0);
-        return PrintTargetCG::CreateOrNull(context, size);
-    }
-
-    // Apparently we do need this branch - bug 368933.
-    return PrintTargetCG::CreateOrNull(size, SurfaceFormat::A8R8G8B8_UINT32);
-
-    NS_OBJC_END_TRY_ABORT_BLOCK_NSNULL;
+    return PrintTargetCG::CreateOrNull(mPrintSession, size);
 }