Bug 1641256: Don't deactivate remote canvas due to read error when th…

…e writer has already failed. r=jrmuizel

This also removes the gfxDevCrashes when an invalid enum is read, because we
suspect these are mainly due to the writer shutting down mid event.
We could only crash when the writer hasn't failed, but because these reads are
in templated code it would mean updating the different event streams with a new
function. If we are still getting high numbers in the deactivation telemetry we
will need to do this to try and track down the problem.

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

gfx/2d/Logging.h

@@ -127,8 +127,6 @@ enum class LogReason : int {
   UnscaledFontNotFound,
   ScaledFontNotFound,
   InvalidLayerType,  // 40
-  PlayEventFailed,
-  InvalidConstrainedValueRead,
   // End
   MustBeLessThanThis = 101,
 };

gfx/2d/RecordedEventImpl.h

@@ -1611,26 +1611,14 @@ void RecordedEvent::ReadPatternData(S& aStream,
       SurfacePatternStorage* sps =
           reinterpret_cast<SurfacePatternStorage*>(&aPattern.mStorage);
       ReadElement(aStream, *sps);
-      if (!aStream.good()) {
-        return;
-      }
-
       if (sps->mExtend < ExtendMode::CLAMP ||
           sps->mExtend > ExtendMode::REFLECT) {
-        gfxDevCrash(LogReason::InvalidConstrainedValueRead)
-            << "Invalid ExtendMode read: value: " << int(sps->mExtend)
-            << ", min: " << int(ExtendMode::CLAMP)
-            << ", max: " << int(ExtendMode::REFLECT);
         aStream.SetIsBad();
+        return;
       }
 
       if (sps->mSamplingFilter < SamplingFilter::GOOD ||
           sps->mSamplingFilter >= SamplingFilter::SENTINEL) {
-        gfxDevCrash(LogReason::InvalidConstrainedValueRead)
-            << "Invalid SamplingFilter read: value: "
-            << int(sps->mSamplingFilter)
-            << ", min: " << int(SamplingFilter::GOOD)
-            << ", sentinel: " << int(SamplingFilter::SENTINEL);
         aStream.SetIsBad();
       }
       return;
@@ -1758,27 +1746,14 @@ void RecordedEvent::ReadStrokeOptions(S& aStream,
 template <class S>
 static void ReadDrawOptions(S& aStream, DrawOptions& aDrawOptions) {
   ReadElement(aStream, aDrawOptions);
-  if (!aStream.good()) {
-    return;
-  }
-
   if (aDrawOptions.mAntialiasMode < AntialiasMode::NONE ||
       aDrawOptions.mAntialiasMode > AntialiasMode::DEFAULT) {
-    gfxDevCrash(LogReason::InvalidConstrainedValueRead)
-        << "Invalid AntialiasMode read: value: "
-        << int(aDrawOptions.mAntialiasMode)
-        << ", min: " << int(AntialiasMode::NONE)
-        << ", max: " << int(AntialiasMode::DEFAULT);
     aStream.SetIsBad();
+    return;
   }
 
   if (aDrawOptions.mCompositionOp < CompositionOp::OP_OVER ||
       aDrawOptions.mCompositionOp > CompositionOp::OP_COUNT) {
-    gfxDevCrash(LogReason::InvalidConstrainedValueRead)
-        << "Invalid CompositionOp read: value: "
-        << int(aDrawOptions.mCompositionOp)
-        << ", min: " << int(CompositionOp::OP_OVER)
-        << ", max: " << int(CompositionOp::OP_COUNT);
     aStream.SetIsBad();
   }
 }
@@ -1787,27 +1762,14 @@ template <class S>
 static void ReadDrawSurfaceOptions(S& aStream,
                                    DrawSurfaceOptions& aDrawSurfaceOptions) {
   ReadElement(aStream, aDrawSurfaceOptions);
-  if (!aStream.good()) {
-    return;
-  }
-
   if (aDrawSurfaceOptions.mSamplingFilter < SamplingFilter::GOOD ||
       aDrawSurfaceOptions.mSamplingFilter >= SamplingFilter::SENTINEL) {
-    gfxDevCrash(LogReason::InvalidConstrainedValueRead)
-        << "Invalid SamplingFilter read: value: "
-        << int(aDrawSurfaceOptions.mSamplingFilter)
-        << ", min: " << int(SamplingFilter::GOOD)
-        << ", sentinel: " << int(SamplingFilter::SENTINEL);
     aStream.SetIsBad();
+    return;
   }
 
   if (aDrawSurfaceOptions.mSamplingBounds < SamplingBounds::UNBOUNDED ||
       aDrawSurfaceOptions.mSamplingBounds > SamplingBounds::BOUNDED) {
-    gfxDevCrash(LogReason::InvalidConstrainedValueRead)
-        << "Invalid SamplingBounds read: value: "
-        << int(aDrawSurfaceOptions.mSamplingBounds)
-        << ", min: " << int(SamplingBounds::UNBOUNDED)
-        << ", max: " << int(SamplingBounds::BOUNDED);
     aStream.SetIsBad();
   }
 }

gfx/2d/RecordingTypes.h

@@ -49,14 +49,7 @@ template <class S, class T>
 void ReadElementConstrained(S& aStream, T& aElement, const T& aMinValue,
                             const T& aMaxValue) {
   ElementStreamFormat<S, T>::Read(aStream, aElement);
-  if (!aStream.good()) {
-    return;
-  }
-
   if (aElement < aMinValue || aElement > aMaxValue) {
-    gfxDevCrash(LogReason::InvalidConstrainedValueRead)
-        << "Invalid constrained value read: value: " << int(aElement)
-        << ", min: " << int(aMinValue) << ", max: " << int(aMaxValue);
     aStream.SetIsBad();
   }
 }

gfx/layers/CanvasDrawEventRecorder.cpp

@@ -114,7 +114,6 @@ bool CanvasEventRingBuffer::WaitForAndRecalculateAvailableSpace() {
   uint32_t maxToWrite = kStreamSize - bufPos;
   mAvailable = std::min(maxToWrite, WaitForBytesToWrite());
   if (!mAvailable) {
-    mGood = false;
     mBufPos = nullptr;
     return false;
   }
@@ -402,13 +401,16 @@ bool CanvasEventRingBuffer::WaitForReadCount(uint32_t aReadCount,
     }
   }
 
+  // Either the reader has failed or we're stopping writing for some other
+  // reason (e.g. shutdown), so mark us as failed so the reader is aware.
+  mWrite->state = State::Failed;
+  mGood = false;
   return false;
 }
 
 uint32_t CanvasEventRingBuffer::WaitForBytesToWrite() {
   uint32_t streamFullReadCount = mOurCount - kStreamSize;
   if (!WaitForReadCount(streamFullReadCount + 1, kTimeout)) {
-    mGood = false;
     return 0;
   }
 

gfx/layers/CanvasDrawEventRecorder.h

@@ -89,6 +89,8 @@ class CanvasEventRingBuffer final : public gfx::EventRingBuffer {
 
   bool good() const final { return mGood; }
 
+  bool WriterFailed() const { return mWrite->state == State::Failed; }
+
   void SetIsBad() final {
     mGood = false;
     mRead->state = State::Failed;

gfx/layers/ipc/CanvasTranslator.cpp

@@ -165,8 +165,9 @@ void CanvasTranslator::StartTranslation() {
                           &CanvasTranslator::StartTranslation)));
   }
 
-  // If the stream has been marked as bad deactivate remote canvas.
-  if (!mStream->good()) {
+  // If the stream has been marked as bad and the Writer hasn't failed,
+  // deactivate remote canvas.
+  if (!mStream->good() && !mStream->WriterFailed()) {
     Telemetry::ScalarAdd(
         Telemetry::ScalarID::GFX_CANVAS_REMOTE_DEACTIVATED_BAD_STREAM, 1);
     Deactivate();