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

--HG--
rename : gfx/thebes/PrintTargetCG.cpp => gfx/thebes/PrintTargetCG.mm

CLOBBER

@@ -22,4 +22,4 @@
 # changes to stick? As of bug 928195, this shouldn't be necessary! Please
 # don't change CLOBBER for WebIDL changes any more.
 
-Merge day clobber
+Bug 1309272, part 5 renames gfx/thebes/PrintTargetCG.cpp (to .mm) which results in an object file of the same name, requiring a clobber.

gfx/src/nsDeviceContext.cpp

@@ -240,11 +240,7 @@ nsDeviceContext::FontMetricsDeleted(const nsFontMetrics* aFontMetrics)
 bool
 nsDeviceContext::IsPrinterContext()
 {
-  return mPrintTarget != nullptr
-#ifdef XP_MACOSX
-         || mCachedPrintTarget != nullptr
-#endif
-         ;
+  return mPrintTarget != nullptr;
 }
 
 void
@@ -347,39 +343,31 @@ 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);
@@ -388,9 +376,9 @@ nsDeviceContext::CreateRenderingContextCommon(bool aWantReferenceContext)
     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,
@@ -509,15 +497,16 @@ 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;
 }
 
@@ -530,6 +519,8 @@ nsDeviceContext::AbortDocument(void)
     if (mDeviceContextSpec)
         mDeviceContextSpec->EndDocument();
 
+    mPrintTarget = nullptr;
+
     return rv;
 }
 
@@ -544,15 +535,7 @@ nsDeviceContext::BeginPage(void)
 
     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
@@ -560,18 +543,6 @@ 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();
 
@@ -661,10 +632,6 @@ nsDeviceContext::FindScreen(nsIScreen** 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

gfx/src/nsDeviceContext.h

@@ -300,9 +300,6 @@ private:
     nsCOMPtr<nsIScreenManager>     mScreenManager;
     nsCOMPtr<nsIDeviceContextSpec> mDeviceContextSpec;
     RefPtr<PrintTarget>            mPrintTarget;
-#ifdef XP_MACOSX
-    RefPtr<PrintTarget>            mCachedPrintTarget;
-#endif
 #ifdef DEBUG
     bool mIsInitialized;
 #endif

gfx/thebes/PrintTargetCG.h

@@ -14,26 +14,25 @@ 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

gfx/thebes/PrintTargetCG.mm

@@ -8,61 +8,44 @@
 #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();
 }
@@ -116,5 +99,49 @@ PrintTargetCG::GetReferenceDrawTarget(DrawEventRecorder* aRecorder)
   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

gfx/thebes/moz.build

@@ -89,7 +89,7 @@ elif CONFIG['MOZ_WIDGET_TOOLKIT'] == 'cocoa':
         'gfxPlatformMac.cpp',
         'gfxQuartzNativeDrawing.cpp',
         'gfxQuartzSurface.cpp',
-        'PrintTargetCG.cpp',
+        'PrintTargetCG.mm',
     ]
 elif 'gtk' in CONFIG['MOZ_WIDGET_TOOLKIT']:
     EXPORTS += [

widget/cocoa/nsDeviceContextSpecX.mm

@@ -139,27 +139,11 @@ void nsDeviceContextSpecX::GetPaperRect(double* aTop, double* aLeft, double* aBo
 
 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);
 }