Bug 1280324, part 4 - Assert that PrintTarget::MakeDrawTarget is only called while a print page is being processed. r=edwin
authorJonathan Watt <jwatt@jwatt.org>
Thu, 27 Oct 2016 19:25:02 +0100
changeset 320199 afb9ba10005efaddaa9ec736f9c6bf058d4c6362
parent 320198 20f8aeb59ada4e5165ea585c490c62c0e5d4f2f3
child 320200 45b462d40300df32424e2b582eb3b491de7331de
push id20751
push userphilringnalda@gmail.com
push dateSun, 30 Oct 2016 18:06:35 +0000
treeherderfx-team@e3279760cd97 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1280324
milestone52.0a1
Bug 1280324, part 4 - Assert that PrintTarget::MakeDrawTarget is only called while a print page is being processed. r=edwin
gfx/thebes/PrintTarget.cpp
gfx/thebes/PrintTarget.h
gfx/thebes/PrintTargetThebes.cpp
--- a/gfx/thebes/PrintTarget.cpp
+++ b/gfx/thebes/PrintTarget.cpp
@@ -11,16 +11,20 @@
 
 namespace mozilla {
 namespace gfx {
 
 PrintTarget::PrintTarget(cairo_surface_t* aCairoSurface, const IntSize& aSize)
   : mCairoSurface(aCairoSurface)
   , mSize(aSize)
   , mIsFinished(false)
+#ifdef DEBUG
+  , mHasActivePage(false)
+#endif
+
 {
 #if 0
   // aCairoSurface is null when our PrintTargetThebes subclass's ctor calls us.
   // Once PrintTargetThebes is removed, enable this assertion.
   MOZ_ASSERT(aCairoSurface && !cairo_surface_status(aCairoSurface),
              "CreateOrNull factory methods should not call us without a "
              "valid cairo_surface_t*");
 #endif
@@ -47,16 +51,20 @@ PrintTarget::~PrintTarget()
 
 already_AddRefed<DrawTarget>
 PrintTarget::MakeDrawTarget(const IntSize& aSize,
                             DrawEventRecorder* aRecorder)
 {
   MOZ_ASSERT(mCairoSurface,
              "We shouldn't have been constructed without a cairo surface");
 
+  // This should not be called outside of BeginPage()/EndPage() calls since
+  // some backends can only provide a valid DrawTarget at that time.
+  MOZ_ASSERT(mHasActivePage, "We can't guarantee a valid DrawTarget");
+
   if (cairo_surface_status(mCairoSurface)) {
     return nullptr;
   }
 
   // Note than aSize may not be the same as mSize (the size of mCairoSurface).
   // See the comments in our header.  If the sizes are different a clip will
   // be applied to mCairoSurface.
   RefPtr<DrawTarget> dt =
--- a/gfx/thebes/PrintTarget.h
+++ b/gfx/thebes/PrintTarget.h
@@ -32,22 +32,32 @@ public:
   virtual nsresult BeginPrinting(const nsAString& aTitle,
                                  const nsAString& aPrintToFileName) {
     return NS_OK;
   }
   virtual nsresult EndPrinting() {
     return NS_OK;
   }
   virtual nsresult AbortPrinting() {
+#ifdef DEBUG
+    mHasActivePage = false;
+#endif
     return NS_OK;
   }
   virtual nsresult BeginPage() {
+#ifdef DEBUG
+    MOZ_ASSERT(!mHasActivePage, "Missing EndPage() call");
+    mHasActivePage = true;
+#endif
     return NS_OK;
   }
   virtual nsresult EndPage() {
+#ifdef DEBUG
+    mHasActivePage = false;
+#endif
     return NS_OK;
   }
 
   /**
    * Releases the resources used by this PrintTarget.  Typically this should be
    * called after calling EndPrinting().  Calling this more than once is
    * allowed, but subsequent calls are a no-op.
    *
@@ -73,16 +83,20 @@ public:
    * failure.
    *
    * If aRecorder is passed a recording DrawTarget will be created instead of
    * the type of DrawTarget that would normally be returned for a particular
    * subclass of this class.  This argument is only intended to be used in
    * the e10s content process if printing output can't otherwise be transfered
    * over to the parent process using the normal DrawTarget type.
    *
+   * NOTE: this should only be called between BeginPage()/EndPage() calls, and
+   * the returned DrawTarget should not be drawn to after EndPage() has been
+   * called.
+   *
    * XXX For consistency with the old code this takes a size parameter even
    * though we already have the size passed to our subclass's CreateOrNull
    * factory methods.  The size passed to the factory method comes from
    * nsIDeviceContextSpec::MakePrintTarget overrides, whereas the size
    * passed to us comes from nsDeviceContext::CreateRenderingContext.  In at
    * least one case (nsDeviceContextSpecAndroid::MakePrintTarget) these are
    * different.  At some point we should align the two sources and get rid of
    * this method's size parameter.
@@ -130,14 +144,17 @@ protected:
   already_AddRefed<DrawTarget>
   CreateRecordingDrawTarget(DrawEventRecorder* aRecorder,
                             DrawTarget* aDrawTarget);
 
   cairo_surface_t* mCairoSurface;
   RefPtr<DrawTarget> mRefDT; // reference DT
   IntSize mSize;
   bool mIsFinished;
+#ifdef DEBUG
+  bool mHasActivePage;
+#endif
 };
 
 } // namespace gfx
 } // namespace mozilla
 
 #endif /* MOZILLA_GFX_PRINTTARGET_H */
--- a/gfx/thebes/PrintTargetThebes.cpp
+++ b/gfx/thebes/PrintTargetThebes.cpp
@@ -31,16 +31,20 @@ PrintTargetThebes::PrintTargetThebes(gfx
   , mGfxSurface(aSurface)
 {
 }
 
 already_AddRefed<DrawTarget>
 PrintTargetThebes::MakeDrawTarget(const IntSize& aSize,
                                   DrawEventRecorder* aRecorder)
 {
+  // This should not be called outside of BeginPage()/EndPage() calls since
+  // some backends can only provide a valid DrawTarget at that time.
+  MOZ_ASSERT(mHasActivePage, "We can't guarantee a valid DrawTarget");
+
   RefPtr<gfx::DrawTarget> dt =
     gfxPlatform::GetPlatform()->CreateDrawTargetForSurface(mGfxSurface, aSize);
   if (!dt || !dt->IsValid()) {
     return nullptr;
   }
 
   if (aRecorder) {
     dt = CreateRecordingDrawTarget(aRecorder, dt);