Bug 1354624 - Fix PrintTarget::GetReferenceDrawTarget and its overrides to honor aRecorder. r=bobowen
authorJonathan Watt <jwatt@jwatt.org>
Tue, 21 Mar 2017 09:36:08 +0000
changeset 354573 e866a70f9eb6b01ba79c68afcfb9955984f953c3
parent 354489 06c7c56497ad4e34729d6e54da8408ad868ec8de
child 354574 f0d866c0600a85d6646b802a34e8eb6f9f181d36
push id31707
push userkwierso@gmail.com
push dateMon, 24 Apr 2017 22:53:41 +0000
treeherdermozilla-central@abdcc8dfc283 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1354624
milestone55.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 1354624 - Fix PrintTarget::GetReferenceDrawTarget and its overrides to honor aRecorder. r=bobowen MozReview-Commit-ID: IeXTFrTe8PL
gfx/thebes/PrintTarget.cpp
gfx/thebes/PrintTarget.h
gfx/thebes/PrintTargetCG.mm
gfx/thebes/PrintTargetSkPDF.cpp
gfx/thebes/PrintTargetThebes.cpp
--- a/gfx/thebes/PrintTarget.cpp
+++ b/gfx/thebes/PrintTarget.cpp
@@ -20,16 +20,17 @@ namespace mozilla {
 namespace gfx {
 
 PrintTarget::PrintTarget(cairo_surface_t* aCairoSurface, const IntSize& aSize)
   : mCairoSurface(aCairoSurface)
   , mSize(aSize)
   , mIsFinished(false)
 #ifdef DEBUG
   , mHasActivePage(false)
+  , mRecorder(nullptr)
 #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 "
@@ -127,26 +128,40 @@ PrintTarget::GetReferenceDrawTarget(Draw
       Factory::CreateDrawTargetForCairoSurface(similar, size);
 
     // The DT addrefs the surface, so we need drop our own reference to it:
     cairo_surface_destroy(similar);
 
     if (!dt || !dt->IsValid()) {
       return nullptr;
     }
+    mRefDT = dt.forget();
+  }
 
-    if (aRecorder) {
-      dt = CreateRecordingDrawTarget(aRecorder, dt);
+  if (aRecorder) {
+    if (!mRecordingRefDT) {
+      RefPtr<DrawTarget> dt = CreateRecordingDrawTarget(aRecorder, mRefDT);
       if (!dt || !dt->IsValid()) {
         return nullptr;
       }
+      mRecordingRefDT = dt.forget();
+#ifdef DEBUG
+      mRecorder = aRecorder;
+#endif
     }
+#ifdef DEBUG
+    else {
+      MOZ_ASSERT(aRecorder == mRecorder,
+                 "Caching mRecordingRefDT assumes the aRecorder is an invariant");
+    }
+#endif
 
-    mRefDT = dt.forget();
+    return do_AddRef(mRecordingRefDT);
   }
+
   return do_AddRef(mRefDT);
 }
 
 already_AddRefed<DrawTarget>
 PrintTarget::CreateRecordingDrawTarget(DrawEventRecorder* aRecorder,
                                        DrawTarget* aDrawTarget)
 {
   MOZ_ASSERT(aRecorder);
--- a/gfx/thebes/PrintTarget.h
+++ b/gfx/thebes/PrintTarget.h
@@ -144,19 +144,28 @@ protected:
   virtual ~PrintTarget();
 
   already_AddRefed<DrawTarget>
   CreateRecordingDrawTarget(DrawEventRecorder* aRecorder,
                             DrawTarget* aDrawTarget);
 
   cairo_surface_t* mCairoSurface;
   RefPtr<DrawTarget> mRefDT; // reference DT
+
+  // Early on during printing we expect to be called without a recorder in
+  // order to gather metrics for reflow.  However, in a content process, once
+  // we go on to paint we then expect to be called with a recorder.  Hence why
+  // we have this separate recording reference DrawTarget (which wraps mRefDT).
+  RefPtr<DrawTarget> mRecordingRefDT;
+
   IntSize mSize;
   bool mIsFinished;
 #ifdef DEBUG
   bool mHasActivePage;
+  // owned by mRecordingRefDT, so kept alive for our entire lifetime if set:
+  DrawEventRecorder* mRecorder;
 #endif
 };
 
 } // namespace gfx
 } // namespace mozilla
 
 #endif /* MOZILLA_GFX_PRINTTARGET_H */
--- a/gfx/thebes/PrintTargetCG.mm
+++ b/gfx/thebes/PrintTargetCG.mm
@@ -89,26 +89,40 @@ PrintTargetCG::GetReferenceDrawTarget(Dr
       Factory::CreateDrawTargetForCairoSurface(similar, size);
 
     // The DT addrefs the surface, so we need drop our own reference to it:
     cairo_surface_destroy(similar);
 
     if (!dt || !dt->IsValid()) {
       return nullptr;
     }
+    mRefDT = dt.forget();
+  }
 
-    if (aRecorder) {
-      dt = CreateRecordingDrawTarget(aRecorder, dt);
+  if (aRecorder) {
+    if (!mRecordingRefDT) {
+      RefPtr<DrawTarget> dt = CreateRecordingDrawTarget(aRecorder, mRefDT);
       if (!dt || !dt->IsValid()) {
         return nullptr;
       }
+      mRecordingRefDT = dt.forget();
+#ifdef DEBUG
+      mRecorder = aRecorder;
+#endif
     }
+#ifdef DEBUG
+    else {
+      MOZ_ASSERT(aRecorder == mRecorder,
+                 "Caching mRecordingRefDT assumes the aRecorder is an invariant");
+    }
+#endif
 
-    mRefDT = dt.forget();
+    return do_AddRef(mRecordingRefDT);
   }
+
   return do_AddRef(mRefDT);
 }
 
 nsresult
 PrintTargetCG::BeginPrinting(const nsAString& aTitle,
                              const nsAString& aPrintToFileName,
                              int32_t aStartPage,
                              int32_t aEndPage)
--- a/gfx/thebes/PrintTargetSkPDF.cpp
+++ b/gfx/thebes/PrintTargetSkPDF.cpp
@@ -137,23 +137,37 @@ PrintTargetSkPDF::GetReferenceDrawTarget
     if (!mRefCanvas) {
       return nullptr;
     }
     RefPtr<DrawTarget> dt =
       Factory::CreateDrawTargetWithSkCanvas(mRefCanvas.get());
     if (!dt) {
       return nullptr;
     }
+    mRefDT = dt.forget();
+  }
 
-    if (aRecorder) {
-      dt = CreateRecordingDrawTarget(aRecorder, dt);
+  if (aRecorder) {
+    if (!mRecordingRefDT) {
+      RefPtr<DrawTarget> dt = CreateRecordingDrawTarget(aRecorder, mRefDT);
       if (!dt || !dt->IsValid()) {
         return nullptr;
       }
+      mRecordingRefDT = dt.forget();
+#ifdef DEBUG
+      mRecorder = aRecorder;
+#endif
     }
+#ifdef DEBUG
+    else {
+      MOZ_ASSERT(aRecorder == mRecorder,
+                 "Caching mRecordingRefDT assumes the aRecorder is an invariant");
+    }
+#endif
 
-    mRefDT = dt.forget();
+    return do_AddRef(mRecordingRefDT);
   }
+
   return do_AddRef(mRefDT);
 }
 
 } // namespace gfx
 } // namespace mozilla
--- a/gfx/thebes/PrintTargetThebes.cpp
+++ b/gfx/thebes/PrintTargetThebes.cpp
@@ -60,24 +60,40 @@ already_AddRefed<DrawTarget>
 PrintTargetThebes::GetReferenceDrawTarget(DrawEventRecorder* aRecorder)
 {
   if (!mRefDT) {
     RefPtr<gfx::DrawTarget> dt =
       gfxPlatform::GetPlatform()->CreateDrawTargetForSurface(mGfxSurface, mSize);
     if (!dt || !dt->IsValid()) {
       return nullptr;
     }
-    if (aRecorder) {
-      dt = CreateRecordingDrawTarget(aRecorder, dt);
+    mRefDT = dt->CreateSimilarDrawTarget(IntSize(1,1), dt->GetFormat());
+  }
+
+  if (aRecorder) {
+    if (!mRecordingRefDT) {
+      RefPtr<DrawTarget> dt = CreateRecordingDrawTarget(aRecorder, mRefDT);
       if (!dt || !dt->IsValid()) {
         return nullptr;
       }
+      mRecordingRefDT = dt.forget();
+#ifdef DEBUG
+      mRecorder = aRecorder;
+#endif
     }
-    mRefDT = dt->CreateSimilarDrawTarget(IntSize(1,1), dt->GetFormat());
+#ifdef DEBUG
+    else {
+      MOZ_ASSERT(aRecorder == mRecorder,
+                 "Caching mRecordingRefDT assumes the aRecorder is an invariant");
+    }
+#endif
+
+    return do_AddRef(mRecordingRefDT);
   }
+
   return do_AddRef(mRefDT);
 }
 
 nsresult
 PrintTargetThebes::BeginPrinting(const nsAString& aTitle,
                                  const nsAString& aPrintToFileName,
                                  int32_t aStartPage,
                                  int32_t aEndPage)