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
authorJonathan Watt <jwatt@jwatt.org>
Mon, 28 Nov 2016 22:40:43 +0000
changeset 324468 c5c8bd440978ec31deb66443bc2b17095e4a9fa4
parent 324467 44360d87a266785851b0907164531c8019d41545
child 324469 af5401b40b582b8effd44505bdf2dd4c71ed4f3f
push id31006
push usercbook@mozilla.com
push dateTue, 29 Nov 2016 10:40:01 +0000
treeherdermozilla-central@f8107cf96144 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1309272
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
CLOBBER
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/CLOBBER
+++ b/CLOBBER
@@ -17,9 +17,9 @@
 #
 # Modifying this file will now automatically clobber the buildbot machines \o/
 #
 
 # Are you updating CLOBBER because you think it's needed for your WebIDL
 # 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
\ No newline at end of file
+Bug 1309272, part 5 renames gfx/thebes/PrintTargetCG.cpp (to .mm) which results in an object file of the same name, requiring a clobber.
--- 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);
 }