Bug 1644208: Change RecordSourceSurfaceDestruction to take a void* not SourceSurface* to avoid AddRef during destructor. r=jrmuizel

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

gfx/2d/DrawEventRecorder.cpp

@@ -48,9 +48,8 @@ void DrawEventRecorderPrivate::StoreSourceSurfaceRecording(
       dataSurf->GetFormat()));
 }
 
-void DrawEventRecorderPrivate::RecordSourceSurfaceDestruction(
-    SourceSurface* aSurface) {
-  RemoveSourceSurface(aSurface);
+void DrawEventRecorderPrivate::RecordSourceSurfaceDestruction(void* aSurface) {
+  RemoveSourceSurface(static_cast<SourceSurface*>(aSurface));
   RemoveStoredObject(aSurface);
   RecordEvent(RecordedSourceSurfaceDestruction(ReferencePtr(aSurface)));
 }

gfx/2d/DrawEventRecorder.h

@@ -128,7 +128,18 @@ class DrawEventRecorderPrivate : public DrawEventRecorder {
   virtual void StoreSourceSurfaceRecording(SourceSurface* aSurface,
                                            const char* aReason);
 
-  virtual void RecordSourceSurfaceDestruction(SourceSurface* aSurface);
+  /**
+   * This is virtual to allow subclasses to control the recording, if for
+   * example it needs to happen on a specific thread. aSurface is a void*
+   * instead of a SourceSurface* because this is called during the SourceSurface
+   * destructor, so it is partially destructed and should not be accessed. If we
+   * use a SourceSurface* we might, for example, accidentally AddRef/Release the
+   * object by passing it to NewRunnableMethod to submit to a different thread.
+   * We are only using the pointer as a lookup ID to our internal maps and
+   * ReferencePtr to be used on the translation side.
+   * @param aSurface the surface whose destruction is being recorded
+   */
+  virtual void RecordSourceSurfaceDestruction(void* aSurface);
 
   virtual void AddDependentSurface(uint64_t aDependencyId) {
     MOZ_CRASH("GFX: AddDependentSurface");

gfx/layers/CanvasDrawEventRecorder.cpp

@@ -491,8 +491,7 @@ void CanvasEventRingBuffer::ReturnRead(char* aOut, size_t aSize) {
   mWrite->returnCount = readCount;
 }
 
-void CanvasDrawEventRecorder::RecordSourceSurfaceDestruction(
-    gfx::SourceSurface* aSurface) {
+void CanvasDrawEventRecorder::RecordSourceSurfaceDestruction(void* aSurface) {
   // We must only record things on the main thread and surfaces that have been
   // recorded can sometimes be destroyed off the main thread.
   if (NS_IsMainThread()) {
@@ -500,7 +499,7 @@ void CanvasDrawEventRecorder::RecordSourceSurfaceDestruction(
     return;
   }
 
-  NS_DispatchToMainThread(NewRunnableMethod<gfx::SourceSurface*>(
+  NS_DispatchToMainThread(NewRunnableMethod<void*>(
       "DrawEventRecorderPrivate::RecordSourceSurfaceDestruction", this,
       &DrawEventRecorderPrivate::RecordSourceSurfaceDestruction, aSurface));
 }

gfx/layers/CanvasDrawEventRecorder.h

@@ -247,7 +247,7 @@ class CanvasDrawEventRecorder final : public gfx::DrawEventRecorderPrivate {
     aEvent.RecordToStream(mOutputStream);
   }
 
-  void RecordSourceSurfaceDestruction(gfx::SourceSurface* aSurface) final;
+  void RecordSourceSurfaceDestruction(void* aSurface) final;
 
   void Flush() final {}