Bug 1970805 - Remove validity check from Translator::GetCurrentDrawTarget to avoid aborting print-job playback, and add drawtarget-validity checks to more methods in DrawTargetCairo. a=RyanVM

Original Revision: https://phabricator.services.mozilla.com/D255099

Differential Revision: https://phabricator.services.mozilla.com/D261880

gfx/2d/DrawTargetCairo.cpp

@@ -679,6 +679,12 @@ void DrawTargetCairo::Link(const char* aDest, const char* aURI,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "Link with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   double x = aRect.x, y = aRect.y, w = aRect.width, h = aRect.height;
   cairo_user_to_device(mContext, &x, &y);
   cairo_user_to_device_distance(mContext, &w, &h);
@@ -709,6 +715,12 @@ void DrawTargetCairo::Destination(const char* aDestination,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "Destination with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   nsAutoCString dest(aDestination);
   EscapeForCairo(dest);
 
@@ -728,6 +740,7 @@ already_AddRefed<SourceSurface> DrawTargetCairo::Snapshot() {
                     << (mSurface ? cairo_surface_status(mSurface) : -1);
     return nullptr;
   }
+
   if (mSnapshot) {
     RefPtr<SourceSurface> snapshot(mSnapshot);
     return snapshot.forget();
@@ -744,6 +757,12 @@ already_AddRefed<SourceSurface> DrawTargetCairo::Snapshot() {
 bool DrawTargetCairo::LockBits(uint8_t** aData, IntSize* aSize,
                                int32_t* aStride, SurfaceFormat* aFormat,
                                IntPoint* aOrigin) {
+  if (!IsValid()) {
+    gfxCriticalNote << "LockBits with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return false;
+  }
+
   cairo_surface_t* target = cairo_get_group_target(mContext);
   cairo_surface_t* surf = target;
 #ifdef CAIRO_HAS_WIN32_SURFACE
@@ -799,6 +818,12 @@ void DrawTargetCairo::ReleaseBits(uint8_t* aData) {
 }
 
 void DrawTargetCairo::Flush() {
+  if (!IsValid()) {
+    gfxCriticalNote << "Flush with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   cairo_surface_t* surf = cairo_get_group_target(mContext);
   cairo_surface_flush(surf);
 }
@@ -918,6 +943,11 @@ void DrawTargetCairo::DrawSurface(SourceSurface* aSurface, const Rect& aDest,
 void DrawTargetCairo::DrawFilter(FilterNode* aNode, const Rect& aSourceRect,
                                  const Point& aDestPoint,
                                  const DrawOptions& aOptions) {
+  if (!IsValid() || !aNode) {
+    gfxCriticalNote << "DrawFilter with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
   FilterNodeSoftware* filter = static_cast<FilterNodeSoftware*>(aNode);
   filter->Draw(this, aSourceRect, aDestPoint, aOptions);
 }
@@ -1002,6 +1032,12 @@ void DrawTargetCairo::DrawPattern(const Pattern& aPattern,
                                   const DrawOptions& aOptions,
                                   DrawPatternType aDrawType,
                                   bool aPathBoundsClip) {
+  if (!IsValid()) {
+    gfxCriticalNote << "DrawPattern with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   if (!PatternIsCompatible(aPattern)) {
     return;
   }
@@ -1062,6 +1098,12 @@ void DrawTargetCairo::FillRect(const Rect& aRect, const Pattern& aPattern,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "FillRect with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
 
   bool restoreTransform = false;
@@ -1109,6 +1151,12 @@ void DrawTargetCairo::FillRect(const Rect& aRect, const Pattern& aPattern,
 void DrawTargetCairo::CopySurfaceInternal(cairo_surface_t* aSurface,
                                           const IntRect& aSource,
                                           const IntPoint& aDest) {
+  if (!IsValid()) {
+    gfxCriticalNote << "CopySurfaceInternal with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   if (cairo_surface_status(aSurface)) {
     gfxWarning() << "Invalid surface" << cairo_surface_status(aSurface);
     return;
@@ -1135,6 +1183,12 @@ void DrawTargetCairo::CopySurface(SourceSurface* aSurface,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "CopySurface with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
   AutoClearDeviceOffset clear(aSurface);
 
@@ -1158,6 +1212,12 @@ void DrawTargetCairo::CopyRect(const IntRect& aSource, const IntPoint& aDest) {
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "CopyRect with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
 
   IntRect source = aSource;
@@ -1189,6 +1249,12 @@ void DrawTargetCairo::ClearRect(const Rect& aRect) {
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "ClearRect with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
 
   if (!mContext || aRect.Width() < 0 || aRect.Height() < 0 ||
@@ -1215,6 +1281,12 @@ void DrawTargetCairo::StrokeRect(
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "StrokeRect with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
 
   cairo_new_path(mContext);
@@ -1232,6 +1304,12 @@ void DrawTargetCairo::StrokeLine(
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "StrokeLine with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
 
   cairo_new_path(mContext);
@@ -1249,6 +1327,12 @@ void DrawTargetCairo::Stroke(
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "Stroke with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext, aPath);
 
   if (aPath->GetBackendType() != BackendType::CAIRO) return;
@@ -1266,6 +1350,12 @@ void DrawTargetCairo::Fill(const Path* aPath, const Pattern& aPattern,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "Fill with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext, aPath);
 
   if (aPath->GetBackendType() != BackendType::CAIRO) return;
@@ -1307,6 +1397,12 @@ void DrawTargetCairo::SetFontOptions(cairo_antialias_t aAAMode) {
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "SetFontOptions with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   if (!mFontOptions) {
     mFontOptions = cairo_font_options_create();
     if (!mFontOptions) {
@@ -1431,6 +1527,12 @@ void DrawTargetCairo::Mask(const Pattern& aSource, const Pattern& aMask,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "Mask with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
   AutoClearDeviceOffset clearSource(aSource);
   AutoClearDeviceOffset clearMask(aMask);
@@ -1472,6 +1574,12 @@ void DrawTargetCairo::MaskSurface(const Pattern& aSource, SourceSurface* aMask,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "MaskSurface with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
   AutoClearDeviceOffset clearSource(aSource);
   AutoClearDeviceOffset clearMask(aMask);
@@ -1535,6 +1643,12 @@ void DrawTargetCairo::PushClip(const Path* aPath) {
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "PushClip with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   WillChange(aPath);
   cairo_save(mContext);
 
@@ -1553,6 +1667,12 @@ void DrawTargetCairo::PushClip(const Path* aPath) {
 }
 
 void DrawTargetCairo::PushClipRect(const Rect& aRect) {
+  if (!IsValid()) {
+    gfxCriticalNote << "PushClipRect with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   WillChange();
   cairo_save(mContext);
 
@@ -1573,6 +1693,12 @@ void DrawTargetCairo::PopClip() {
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "PopClip with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   // save/restore does not affect the path, so no need to call WillChange()
 
   // cairo_restore will restore the transform too and we don't want to do that
@@ -1608,6 +1734,12 @@ void DrawTargetCairo::PushLayerWithBlend(bool aOpaque, Float aOpacity,
                                          const IntRect& aBounds,
                                          bool aCopyBackground,
                                          CompositionOp aCompositionOp) {
+  if (!IsValid()) {
+    gfxCriticalNote << "PushLayerWithBlend with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   cairo_content_t content = CAIRO_CONTENT_COLOR_ALPHA;
 
   if (mFormat == SurfaceFormat::A8) {
@@ -1653,6 +1785,12 @@ void DrawTargetCairo::PushLayerWithBlend(bool aOpaque, Float aOpacity,
 }
 
 void DrawTargetCairo::PopLayer() {
+  if (!IsValid()) {
+    gfxCriticalNote << "PopLayer with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   MOZ_RELEASE_ASSERT(!mPushedLayers.empty());
 
   cairo_set_operator(mContext, CAIRO_OPERATOR_OVER);
@@ -1690,7 +1828,16 @@ void DrawTargetCairo::PopLayer() {
 
 void DrawTargetCairo::ClearSurfaceForUnboundedSource(
     const CompositionOp& aOperator) {
-  if (aOperator != CompositionOp::OP_SOURCE) return;
+  if (aOperator != CompositionOp::OP_SOURCE) {
+    return;
+  }
+
+  if (!IsValid()) {
+    gfxCriticalNote << "ClearSurfaceForUnboundedSource with bad surface "
+                    << cairo_surface_status(cairo_get_group_target(mContext));
+    return;
+  }
+
   cairo_set_operator(mContext, CAIRO_OPERATOR_CLEAR);
   // It doesn't really matter what the source is here, since Paint
   // isn't bounded by the source and the mask covers the entire clip
@@ -1824,6 +1971,7 @@ RefPtr<DrawTarget> DrawTargetCairo::CreateClippedDrawTarget(
   cairo_restore(mContext);
   return result;
 }
+
 bool DrawTargetCairo::InitAlreadyReferenced(cairo_surface_t* aSurface,
                                             const IntSize& aSize,
                                             SurfaceFormat* aFormat) {

gfx/2d/RecordedEvent.h

@@ -159,9 +159,8 @@ class Translator {
     mDependentSurfaces = aDependentSurfaces;
   }
 
-  DrawTarget* GetCurrentDrawTarget() const {
+  // NOTE that the returned DrawTarget may be in an error state!
-    return mCurrentDT && mCurrentDT->IsValid() ? mCurrentDT : nullptr;
+  DrawTarget* GetCurrentDrawTarget() const { return mCurrentDT; }
-  }
 
   nsRefPtrHashtable<nsUint64HashKey, RecordedDependentSurface>*
       mDependentSurfaces = nullptr;

testing/web-platform/meta/css/printing/huge-font-crash-print.html.ini

@@ -1,6 +1,10 @@
 [huge-font-crash-print.html]
+  # Linux(debug) crashes with a fatal (parent-process) assertion because freetype
+  # failure puts then cairo surface into an error state, see bug 1725070.
   disabled:
     if asan and (os == "linux"): https://bugzilla.mozilla.org/show_bug.cgi?id=1929461
+    if debug and (os == "linux"): https://bugzilla.mozilla.org/show_bug.cgi?id=1725070
   # (jfkthame) This passes for me locally, but currently times out in CI.
   # The important thing is that it should not crash.
-  expected: [PASS, TIMEOUT]
+  expected:
+    [PASS, TIMEOUT]