Bug 1566915 - Write collected frames from the composition recorder on the render thread r=kvark

On macOS, if we try to write the collected frames from the
`CompositorBridgeParent` we will not have an active GL context, resulting in a
crash. Writing the frames from the `RenderThread` solves this problem.

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

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

gfx/layers/ipc/CompositorBridgeParent.cpp

@@ -2642,13 +2642,14 @@ mozilla::ipc::IPCResult CompositorBridgeParent::RecvEndRecording(
 
   if (mLayerManager) {
     mLayerManager->SetCompositionRecorder(nullptr);
+    mCompositionRecorder->WriteCollectedFrames();
+  } else if (mWrBridge) {
+    // If we are using WebRender, the |RenderThread| will have a handle to this
+    // |WebRenderCompositionRecorder|, which it will release once the frames
+    // have been written.
+    mWrBridge->WriteCollectedFrames();
   }
 
-  // If we are using WebRender, the |RenderThread| will have a handle to this
-  // |WebRenderCompositionRecorder|, which it will release once the frames have
-  // been written.
-
-  mCompositionRecorder->WriteCollectedFrames();
   mCompositionRecorder = nullptr;
   *aOutSuccess = true;
   return IPC_OK();

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -384,7 +384,6 @@ void WebRenderBridgeParent::RemoveDeferredPipeline(wr::PipelineId aPipelineId) {
   }
 }
 
-
 /* static */
 WebRenderBridgeParent* WebRenderBridgeParent::CreateDestroyed(
     const wr::PipelineId& aPipelineId) {
@@ -415,11 +414,8 @@ bool WebRenderBridgeParent::HandleDeferredPipelineData(
           wr::Epoch wrEpoch = GetNextWrEpoch();
           bool validTransaction = data.mIdNamespace == mIdNamespace;
 
-          if (!ProcessRenderRootDisplayListData(data,
-                                                wrEpoch,
-                                                aTxnStartTime,
-                                                validTransaction,
-                                                false)){
+          if (!ProcessRenderRootDisplayListData(data, wrEpoch, aTxnStartTime,
+                                                validTransaction, false)) {
             return false;
           }
 
@@ -521,13 +517,11 @@ bool WebRenderBridgeParent::MaybeHandleDeferredPipelineData(
                                                     aTxnStartTime)) {
       return false;
     }
-
   }
 
   return true;
 }
 
-
 mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEnsureConnected(
     TextureFactoryIdentifier* aTextureFactoryIdentifier,
     MaybeIdNamespace* aMaybeIdNamespace) {
@@ -995,6 +989,10 @@ void WebRenderBridgeParent::SetCompositionRecorder(
   Api(wr::RenderRoot::Default)->SetCompositionRecorder(std::move(aRecorder));
 }
 
+void WebRenderBridgeParent::WriteCollectedFrames() {
+  Api(wr::RenderRoot::Default)->WriteCollectedFrames();
+}
+
 CompositorBridgeParent* WebRenderBridgeParent::GetRootCompositorBridgeParent()
     const {
   if (!mCompositorBridge) {
@@ -1165,10 +1163,8 @@ bool WebRenderBridgeParent::SetDisplayList(
 }
 
 bool WebRenderBridgeParent::ProcessRenderRootDisplayListData(
-    RenderRootDisplayListData& aDisplayList,
-    wr::Epoch aWrEpoch,
-    const TimeStamp& aTxnStartTime,
-    bool aValidTransaction,
+    RenderRootDisplayListData& aDisplayList, wr::Epoch aWrEpoch,
+    const TimeStamp& aTxnStartTime, bool aValidTransaction,
     bool aObserveLayersUpdate) {
   wr::TransactionBuilder txn;
   Maybe<wr::AutoTransactionSender> sender;
@@ -1276,7 +1272,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
     // Only non-root WRBPs will ever have an unresolved mRenderRoot
     MOZ_ASSERT(!IsRootWebRenderBridgeParent());
     if (aDisplayLists.Length() != 1) {
-      return IPC_FAIL(this, "Well-behaved content processes must only send a DL for a single renderRoot");
+      return IPC_FAIL(this,
+                      "Well-behaved content processes must only send a DL for "
+                      "a single renderRoot");
     }
     PushDeferredPipelineData(AsVariant(std::move(aDisplayLists[0])));
     aDisplayLists.Clear();
@@ -1284,10 +1282,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
 
   for (auto& displayList : aDisplayLists) {
     if (IsRootWebRenderBridgeParent()) {
-      if (!MaybeHandleDeferredPipelineData(
-              displayList.mRenderRoot,
-              displayList.mRemotePipelineIds,
-              aTxnStartTime)) {
+      if (!MaybeHandleDeferredPipelineData(displayList.mRenderRoot,
+                                           displayList.mRemotePipelineIds,
+                                           aTxnStartTime)) {
         return IPC_FAIL(this, "Failed processing deferred pipeline data");
       }
     } else {
@@ -1309,12 +1306,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
   bool observeLayersUpdate = ShouldParentObserveEpoch();
 
   for (auto& displayList : aDisplayLists) {
-    if (!ProcessRenderRootDisplayListData(
-            displayList,
-            wrEpoch,
-            aTxnStartTime,
-            validTransaction,
-            observeLayersUpdate)) {
+    if (!ProcessRenderRootDisplayListData(displayList, wrEpoch, aTxnStartTime,
+                                          validTransaction,
+                                          observeLayersUpdate)) {
       return IPC_FAIL(this, "Failed to process RenderRootDisplayListData.");
     }
   }
@@ -1456,7 +1450,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEmptyTransaction(
     // Only non-root WRBPs will ever have an unresolved mRenderRoot
     MOZ_ASSERT(!IsRootWebRenderBridgeParent());
     if (aRenderRootUpdates.Length() != 1) {
-      return IPC_FAIL(this, "Well-behaved content processes must only send a DL for a single renderRoot");
+      return IPC_FAIL(this,
+                      "Well-behaved content processes must only send a DL for "
+                      "a single renderRoot");
     }
     PushDeferredPipelineData(AsVariant(std::move(aRenderRootUpdates[0])));
     aRenderRootUpdates.Clear();

gfx/layers/wr/WebRenderBridgeParent.h

@@ -282,8 +282,8 @@ class WebRenderBridgeParent final
       const TimeStamp& aTxnStartTime);
 
   /**
-   * See MaybeHandleDeferredPipelineData - this is the implementation of that for
-   * a single pipeline.
+   * See MaybeHandleDeferredPipelineData - this is the implementation of that
+   * for a single pipeline.
    */
   bool MaybeHandleDeferredPipelineDataForPipeline(
       wr::RenderRoot aRenderRoot, wr::PipelineId aPipelineId,
@@ -300,6 +300,13 @@ class WebRenderBridgeParent final
   void SetCompositionRecorder(
       RefPtr<layers::WebRenderCompositionRecorder>&& aRecorder);
 
+  /**
+   * Write the frames collected by the |WebRenderCompositionRecorder| to disk.
+   *
+   * If there is not currently a recorder, this is a no-op.
+   */
+  void WriteCollectedFrames();
+
  private:
   class ScheduleSharedSurfaceRelease;
 

gfx/webrender_bindings/RenderThread.cpp

@@ -267,6 +267,21 @@ void RenderThread::SetCompositionRecorderForWindow(
   mCompositionRecorders[aWindowId] = std::move(aCompositionRecorder);
 }
 
+void RenderThread::WriteCollectedFramesForWindow(wr::WindowId aWindowId) {
+  MOZ_ASSERT(IsInRenderThread());
+  MOZ_ASSERT(GetRenderer(aWindowId));
+
+  auto it = mCompositionRecorders.find(aWindowId);
+  MOZ_DIAGNOSTIC_ASSERT(
+      it != mCompositionRecorders.end(),
+      "Attempted to write frames from a window that was not recording.");
+  if (it != mCompositionRecorders.end()) {
+    it->second->WriteCollectedFrames();
+
+    mCompositionRecorders.erase(it);
+  }
+}
+
 void RenderThread::HandleFrame(wr::WindowId aWindowId, bool aRender) {
   if (mHasShutdown) {
     return;

gfx/webrender_bindings/RenderThread.h

@@ -271,6 +271,8 @@ class RenderThread final {
       wr::WindowId aWindowId,
       RefPtr<layers::WebRenderCompositionRecorder>&& aCompositionRecorder);
 
+  void WriteCollectedFramesForWindow(wr::WindowId aWindowId);
+
  private:
   explicit RenderThread(base::Thread* aThread);
 

gfx/webrender_bindings/WebRenderAPI.cpp

@@ -606,6 +606,25 @@ void WebRenderAPI::SetCompositionRecorder(
   auto event = MakeUnique<SetCompositionRecorderEvent>(std::move(aRecorder));
   RunOnRenderThread(std::move(event));
 }
+
+void WebRenderAPI::WriteCollectedFrames() {
+  class WriteCollectedFramesEvent final : public RendererEvent {
+   public:
+    explicit WriteCollectedFramesEvent() {
+      MOZ_COUNT_CTOR(WriteCollectedFramesEvent);
+    }
+
+    ~WriteCollectedFramesEvent() { MOZ_COUNT_DTOR(WriteCollectedFramesEvent); }
+
+    void Run(RenderThread& aRenderThread, WindowId aWindowId) override {
+      aRenderThread.WriteCollectedFramesForWindow(aWindowId);
+    }
+  };
+
+  auto event = MakeUnique<WriteCollectedFramesEvent>();
+  RunOnRenderThread(std::move(event));
+}
+
 void TransactionBuilder::Clear() { wr_resource_updates_clear(mTxn); }
 
 void TransactionBuilder::Notify(wr::Checkpoint aWhen,

gfx/webrender_bindings/WebRenderAPI.h

@@ -268,6 +268,13 @@ class WebRenderAPI final {
   void SetCompositionRecorder(
       RefPtr<layers::WebRenderCompositionRecorder>&& aRecorder);
 
+  /**
+   * Write the frames collected by the |WebRenderCompositionRecorder| to disk.
+   *
+   * If there is not currently a recorder, this is a no-op.
+   */
+  void WriteCollectedFrames();
+
  protected:
   WebRenderAPI(wr::DocumentHandle* aHandle, wr::WindowId aId,
                uint32_t aMaxTextureSize, bool aUseANGLE, bool aUseDComp,