Bug 1441308 - Always send parent commands when sending mDestroyedActors r=kats,sotaro

If we try to send them separately as we were before, we can run into
cases where we try to destroy the actors and then send the OpRemoveTexture,
which crashes.

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

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

gfx/layers/wr/RenderRootTypes.h

@@ -21,12 +21,12 @@ struct RenderRootDisplayListData {
   gfx::IntRect mRect;
   nsTArray<WebRenderParentCommand> mCommands;
   wr::LayoutSize mContentSize;
-  mozilla::ipc::ByteBuf mDL;
+  Maybe<mozilla::ipc::ByteBuf> mDL;
   wr::BuiltDisplayListDescriptor mDLDesc;
   nsTArray<OpUpdateResource> mResourceUpdates;
   nsTArray<RefCountedShmem> mSmallShmems;
   nsTArray<mozilla::ipc::Shmem> mLargeShmems;
-  WebRenderScrollData mScrollData;
+  Maybe<WebRenderScrollData> mScrollData;
 };
 
 struct RenderRootUpdates {
@@ -65,5 +65,4 @@ struct IPDLParamTraits<mozilla::layers::RenderRootUpdates> {
 }  // namespace ipc
 }  // namespace mozilla
 
-
 #endif /* GFX_RENDERROOTTYPES_H */

gfx/layers/wr/WebRenderBridgeChild.h

@@ -66,6 +66,9 @@ class WebRenderBridgeChild final : public PWebRenderBridgeChild,
 
   void AddWebRenderParentCommand(const WebRenderParentCommand& aCmd,
                                  wr::RenderRoot aRenderRoot);
+  bool HasWebRenderParentCommands(wr::RenderRoot aRenderRoot) {
+    return !mParentCommands[aRenderRoot].IsEmpty();
+  }
 
   void UpdateResources(wr::IpcResourceUpdateQueue& aResources,
                        wr::RenderRoot aRenderRoot);

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -896,25 +896,13 @@ void WebRenderBridgeParent::SetAPZSampleTime() {
 
 bool WebRenderBridgeParent::SetDisplayList(
     wr::RenderRoot aRenderRoot, const gfx::IntRect& aRect,
-    const nsTArray<WebRenderParentCommand>& aCommands,
     const wr::LayoutSize& aContentSize, ipc::ByteBuf&& aDL,
     const wr::BuiltDisplayListDescriptor& aDLDesc,
     const nsTArray<OpUpdateResource>& aResourceUpdates,
     const nsTArray<RefCountedShmem>& aSmallShmems,
     const nsTArray<ipc::Shmem>& aLargeShmems, const TimeStamp& aTxnStartTime,
-    wr::TransactionBuilder& aTxn, Maybe<wr::AutoTransactionSender>& aTxnSender,
-    wr::Epoch aWrEpoch, bool aValidTransaction, bool aObserveLayersUpdate) {
-  wr::WebRenderAPI* api = Api(aRenderRoot);
-  aTxn.SetLowPriority(!IsRootWebRenderBridgeParent());
-  if (aValidTransaction) {
-    aTxnSender.emplace(api, &aTxn);
-  }
-
-  if (NS_WARN_IF(
-          !ProcessWebRenderParentCommands(aCommands, aTxn, aRenderRoot))) {
-    return false;
-  }
-
+    wr::TransactionBuilder& aTxn, wr::Epoch aWrEpoch, bool aValidTransaction,
+    bool aObserveLayersUpdate) {
   if (NS_WARN_IF(!UpdateResources(aResourceUpdates, aSmallShmems, aLargeShmems,
                                   aTxn))) {
     return false;
@@ -954,7 +942,7 @@ bool WebRenderBridgeParent::SetDisplayList(
           MakeUnique<SceneBuiltNotification>(this, aWrEpoch, aTxnStartTime));
     }
 
-    api->SendTransaction(aTxn);
+    Api(aRenderRoot)->SendTransaction(aTxn);
 
     // We will schedule generating a frame after the scene
     // build is done, so we don't need to do it here.
@@ -1006,16 +994,26 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
   mReceivedDisplayList = true;
   bool observeLayersUpdate = ShouldParentObserveEpoch();
 
-  // The IsFirstPaint() flag should be the same for all the scrolldata across
-  // all the renderroot display lists in a given transaction. We assert this
-  // below. So we can read the flag from any one of them.
-  if (aDisplayLists[0].mScrollData.IsFirstPaint()) {
-    mIsFirstPaint = true;
-  }
+  // The IsFirstPaint() flag should be the same for all the non-empty
+  // scrolldata across all the renderroot display lists in a given
+  // transaction. We assert this below. So we can read the flag from any one
+  // of them.
+  Maybe<size_t> firstScrollDataIndex;
   for (size_t i = 1; i < aDisplayLists.Length(); i++) {
-    // Ensure the flag is the same on all of them.
-    MOZ_RELEASE_ASSERT(aDisplayLists[i].mScrollData.IsFirstPaint() ==
-                       aDisplayLists[0].mScrollData.IsFirstPaint());
+    auto& scrollData = aDisplayLists[i].mScrollData;
+    if (scrollData) {
+      if (firstScrollDataIndex.isNothing()) {
+        firstScrollDataIndex = Some(i);
+        if (scrollData && scrollData->IsFirstPaint()) {
+          mIsFirstPaint = true;
+        }
+      } else {
+        auto firstNonEmpty = aDisplayLists[*firstScrollDataIndex].mScrollData;
+        // Ensure the flag is the same on all of them.
+        MOZ_RELEASE_ASSERT(scrollData->IsFirstPaint() ==
+                           firstNonEmpty->IsFirstPaint());
+      }
+    }
   }
 
   // aScrollData is moved into this function but that is not reflected by the
@@ -1026,8 +1024,10 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
   // sent to WebRender, so that the UpdateHitTestingTree call is guaranteed to
   // be in the updater queue at the time that the scene swap completes.
   for (auto& displayList : aDisplayLists) {
-    UpdateAPZScrollData(wrEpoch, std::move(displayList.mScrollData),
-                        displayList.mRenderRoot);
+    if (displayList.mScrollData) {
+      UpdateAPZScrollData(wrEpoch, std::move(displayList.mScrollData.ref()),
+                          displayList.mRenderRoot);
+    }
   }
 
   bool validTransaction = aIdNamespace == mIdNamespace;
@@ -1037,13 +1037,25 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
   for (auto& displayList : aDisplayLists) {
     MOZ_ASSERT(displayList.mRenderRoot == wr::RenderRoot::Default ||
                IsRootWebRenderBridgeParent());
-    if (!SetDisplayList(
-            displayList.mRenderRoot, displayList.mRect, displayList.mCommands,
-            displayList.mContentSize, std::move(displayList.mDL),
-            displayList.mDLDesc, displayList.mResourceUpdates,
-            displayList.mSmallShmems, displayList.mLargeShmems, aTxnStartTime,
-            txns[displayList.mRenderRoot], senders[displayList.mRenderRoot],
-            wrEpoch, validTransaction, observeLayersUpdate)) {
+    auto renderRoot = displayList.mRenderRoot;
+    auto& txn = txns[renderRoot];
+
+    txn.SetLowPriority(!IsRootWebRenderBridgeParent());
+    if (validTransaction) {
+      senders[renderRoot].emplace(Api(renderRoot), &txn);
+    }
+
+    if (NS_WARN_IF(!ProcessWebRenderParentCommands(displayList.mCommands, txn,
+                                                   renderRoot))) {
+      return IPC_FAIL(this, "Invalid parent command found");
+    }
+
+    if (displayList.mDL &&
+        !SetDisplayList(renderRoot, displayList.mRect, displayList.mContentSize,
+                        std::move(displayList.mDL.ref()), displayList.mDLDesc,
+                        displayList.mResourceUpdates, displayList.mSmallShmems,
+                        displayList.mLargeShmems, aTxnStartTime, txn, wrEpoch,
+                        validTransaction, observeLayersUpdate)) {
       return IPC_FAIL(this, "Failed call to SetDisplayList");
     }
   }

gfx/layers/wr/WebRenderBridgeParent.h

@@ -296,17 +296,14 @@ class WebRenderBridgeParent final
   }
 
   bool SetDisplayList(wr::RenderRoot aRenderRoot, const gfx::IntRect& aRect,
-                      const nsTArray<WebRenderParentCommand>& aCommands,
                       const wr::LayoutSize& aContentSize, ipc::ByteBuf&& aDL,
                       const wr::BuiltDisplayListDescriptor& aDLDesc,
                       const nsTArray<OpUpdateResource>& aResourceUpdates,
                       const nsTArray<RefCountedShmem>& aSmallShmems,
                       const nsTArray<ipc::Shmem>& aLargeShmems,
                       const TimeStamp& aTxnStartTime,
-                      wr::TransactionBuilder& aTxn,
-                      Maybe<wr::AutoTransactionSender>& aTxnSender,
-                      wr::Epoch aWrEpoch, bool aValidTransaction,
-                      bool aObserveLayersUpdate);
+                      wr::TransactionBuilder& aTxn, wr::Epoch aWrEpoch,
+                      bool aValidTransaction, bool aObserveLayersUpdate);
 
   void UpdateAPZFocusState(const FocusTarget& aFocus);
   void UpdateAPZScrollData(const wr::Epoch& aEpoch, WebRenderScrollData&& aData,

gfx/layers/wr/WebRenderLayerManager.cpp

@@ -238,7 +238,8 @@ bool WebRenderLayerManager::EndEmptyTransaction(EndTransactionFlags aFlags) {
   for (auto& stateManager : mStateManagers) {
     auto renderRoot = stateManager.GetRenderRoot();
     if (stateManager.mAsyncResourceUpdates ||
-        !mPendingScrollUpdates[renderRoot].empty()) {
+        !mPendingScrollUpdates[renderRoot].empty() ||
+        WrBridge()->HasWebRenderParentCommands(renderRoot)) {
       auto updates = renderRootUpdates.AppendElement();
       updates->mRenderRoot = renderRoot;
       if (stateManager.mAsyncResourceUpdates) {
@@ -418,12 +419,15 @@ void WebRenderLayerManager::EndTransactionWithoutLayer(
         auto renderRootDL = renderRootDLs.AppendElement();
         renderRootDL->mRenderRoot = renderRoot;
         builder.Finalize(*renderRootDL);
-        mLastDisplayListSizes[renderRoot] = renderRootDL->mDL.mCapacity;
+        mLastDisplayListSizes[renderRoot] = renderRootDL->mDL->mCapacity;
         resourceUpdates.SubQueue(renderRoot)
             .Flush(renderRootDL->mResourceUpdates, renderRootDL->mSmallShmems,
                    renderRootDL->mLargeShmems);
         renderRootDL->mRect = RoundedToInt(rects[renderRoot]).ToUnknownRect();
-        renderRootDL->mScrollData = std::move(mScrollDatas[renderRoot]);
+        renderRootDL->mScrollData.emplace(std::move(mScrollDatas[renderRoot]));
+      } else if (WrBridge()->HasWebRenderParentCommands(renderRoot)) {
+        auto renderRootDL = renderRootDLs.AppendElement();
+        renderRootDL->mRenderRoot = renderRoot;
       }
     }
 

gfx/webrender_bindings/WebRenderAPI.cpp

@@ -730,8 +730,8 @@ void DisplayListBuilder::Finalize(
   wr_api_finalize_builder(SubBuilder(aOutTransaction.mRenderRoot).mWrState,
                           &aOutTransaction.mContentSize,
                           &aOutTransaction.mDLDesc, &dl.inner);
-  aOutTransaction.mDL =
-      ipc::ByteBuf(dl.inner.data, dl.inner.length, dl.inner.capacity);
+  aOutTransaction.mDL.emplace(dl.inner.data, dl.inner.length,
+                              dl.inner.capacity);
   dl.inner.capacity = 0;
   dl.inner.data = nullptr;
 }