Bug 1551735 - Clearly document the case of the RendererOGL receiving a new WebRenderCompositionRecorder while it has one r=kats

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

--HG--
extra : moz-landing-system : lando

gfx/layers/CompositionRecorder.cpp

@@ -58,5 +58,7 @@ void CompositionRecorder::WriteCollectedFrames() {
   mCollectedFrames.Clear();
 }
 
+void CompositionRecorder::ClearCollectedFrames() { mCollectedFrames.Clear(); }
+
 }  // namespace layers
 }  // namespace mozilla

gfx/layers/CompositionRecorder.h

@@ -68,6 +68,8 @@ class CompositionRecorder {
  protected:
   virtual ~CompositionRecorder() = default;
 
+  void ClearCollectedFrames();
+
  private:
   nsTArray<RefPtr<RecordedFrame>> mCollectedFrames;
   TimeStamp mRecordingStart;

gfx/layers/wr/WebRenderCompositionRecorder.cpp

@@ -60,8 +60,9 @@ bool WebRenderCompositionRecorder::MaybeRecordFrame(
   }
 
   if (!mMutex.TryLock()) {
-    // If we cannot lock the mutex, then the |CompositorBridgeParent|
-    // is holding the mutex in |WriteCollectedFrames|.
+    // If we cannot lock the mutex, then either (a) the |CompositorBridgeParent|
+    // is holding the mutex in |WriteCollectedFrames| or (b) the |RenderThread|
+    // is holding the mutex in |ForceFinishRecording|.
     //
     // In either case we do not want to wait to acquire the mutex to record a
     // frame since frames recorded now will not be written to disk.
@@ -106,6 +107,17 @@ void WebRenderCompositionRecorder::WriteCollectedFrames() {
   mFinishedRecording = true;
 }
 
+bool WebRenderCompositionRecorder::ForceFinishRecording() {
+  MutexAutoLock guard(mMutex);
+
+  bool wasRecording = !mFinishedRecording;
+  mFinishedRecording = true;
+
+  ClearCollectedFrames();
+
+  return wasRecording;
+}
+
 bool WebRenderCompositionRecorder::DidPaintContent(
     wr::WebRenderPipelineInfo* aFrameEpochs) {
   const wr::WrPipelineInfo& info = aFrameEpochs->Raw();

gfx/layers/wr/WebRenderCompositionRecorder.h

@@ -82,6 +82,19 @@ class WebRenderCompositionRecorder final : public CompositionRecorder {
   bool MaybeRecordFrame(wr::Renderer* aRenderer,
                         wr::WebRenderPipelineInfo* aFrameEpochs);
 
+  /**
+   * Force the composition recorder to finish recording.
+   *
+   * This should only be called if |WriteCollectedFrames| is not to be called,
+   * since the recorder will be in an invalid state to do so.
+   *
+   * This returns whether or not the recorder was recording before this method
+   * was called.
+   *
+   * Note: This method will block acquiring a lock.
+   */
+  bool ForceFinishRecording();
+
  protected:
   ~WebRenderCompositionRecorder() = default;
 

gfx/webrender_bindings/RenderThread.cpp

@@ -236,6 +236,28 @@ void RenderThread::SetCompositionRecorderForWindow(
   MOZ_ASSERT(IsInRenderThread());
   MOZ_ASSERT(GetRenderer(aWindowId));
 
+  auto it = mCompositionRecorders.find(aWindowId);
+  if (it != mCompositionRecorders.end() && it->second->ForceFinishRecording()) {
+    // This case should never occur since the |CompositorBridgeParent| will
+    // receive its "EndRecording" IPC message before another "BeginRecording"
+    // IPC message.
+    //
+    // However, if we do hit this case, then we should handle it gracefully.
+    // We free the structures here because any captured frames are not going
+    // to be read back.
+    if (RendererOGL* renderer = GetRenderer(aWindowId)) {
+      wr_renderer_release_composition_recorder_structures(
+          renderer->GetRenderer());
+    }
+  }
+
+  // If we have finished recording, then we have received
+  // |SetCompositionRecorderEvent| after the compositor brige parent finished
+  // writing but before we handled another frame to delete the data structure.
+  //
+  // In this case we do not need to free the |wr::Renderer|'s composition
+  // recorder structures since we can re-use them.
+
   mCompositionRecorders[aWindowId] = std::move(aCompositionRecorder);
 }