Bug 1441308 - Always send parent commands when sending mDestroyedActors r=kats,sotaro
authorDoug Thayer <dothayer@mozilla.com>
Fri, 22 Mar 2019 18:29:04 +0000
changeset 465760 9bfd4e60ec4e
parent 465759 8761ce294d2c
child 465761 99140a4b0be4
push id35746
push usershindli@mozilla.com
push dateSat, 23 Mar 2019 09:46:24 +0000
treeherdermozilla-central@02b7484f316b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, sotaro
bugs1441308
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
gfx/layers/wr/RenderRootTypes.h
gfx/layers/wr/WebRenderBridgeChild.h
gfx/layers/wr/WebRenderBridgeParent.cpp
gfx/layers/wr/WebRenderBridgeParent.h
gfx/layers/wr/WebRenderLayerManager.cpp
gfx/webrender_bindings/WebRenderAPI.cpp
--- a/gfx/layers/wr/RenderRootTypes.h
+++ b/gfx/layers/wr/RenderRootTypes.h
@@ -16,22 +16,22 @@ namespace mozilla {
 
 namespace layers {
 
 struct RenderRootDisplayListData {
   wr::RenderRoot mRenderRoot;
   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 {
   wr::RenderRoot mRenderRoot;
   nsTArray<WebRenderParentCommand> mCommands;
   nsTArray<OpUpdateResource> mResourceUpdates;
   nsTArray<RefCountedShmem> mSmallShmems;
   nsTArray<mozilla::ipc::Shmem> mLargeShmems;
@@ -60,10 +60,9 @@ struct IPDLParamTraits<mozilla::layers::
 
   static bool Read(const IPC::Message* aMsg, PickleIterator* aIter,
                    IProtocol* aActor, paramType* aResult);
 };
 
 }  // namespace ipc
 }  // namespace mozilla
 
-
 #endif /* GFX_RENDERROOTTYPES_H */
--- a/gfx/layers/wr/WebRenderBridgeChild.h
+++ b/gfx/layers/wr/WebRenderBridgeChild.h
@@ -61,16 +61,19 @@ class WebRenderBridgeChild final : publi
 
   friend class PWebRenderBridgeChild;
 
  public:
   explicit WebRenderBridgeChild(const wr::PipelineId& aPipelineId);
 
   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);
   void BeginTransaction();
   void EndTransaction(nsTArray<RenderRootDisplayListData>& aRenderRoots,
                       TransactionId aTransactionId, bool aContainsSVGroup,
                       const mozilla::VsyncId& aVsyncId,
                       const mozilla::TimeStamp& aVsyncStartTime,
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -891,35 +891,23 @@ void WebRenderBridgeParent::SetAPZSample
       animationTime += frameInterval;
     }
     apz->SetSampleTime(animationTime);
   }
 }
 
 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;
   }
 
   wr::Vec<uint8_t> dlData(std::move(aDL));
 
   if (aValidTransaction) {
@@ -949,17 +937,17 @@ bool WebRenderBridgeParent::SetDisplayLi
     }
 
     if (!IsRootWebRenderBridgeParent()) {
       aTxn.Notify(
           wr::Checkpoint::SceneBuilt,
           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.
   }
 
   return true;
 }
 
@@ -1001,54 +989,78 @@ mozilla::ipc::IPCResult WebRenderBridgeP
 
   wr::Epoch wrEpoch = GetNextWrEpoch();
 
   mAsyncImageManager->SetCompositionTime(TimeStamp::Now());
 
   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
   // function signature due to the way the IPDL generator works. We remove the
   // const so that we can move this structure all the way to the desired
   // destination.
   // Also note that this needs to happen before the display list transaction is
   // 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;
 
   wr::RenderRootArray<wr::TransactionBuilder> txns;
   wr::RenderRootArray<Maybe<wr::AutoTransactionSender>> senders;
   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");
     }
   }
 
   if (!validTransaction && observeLayersUpdate) {
     mCompositorBridge->ObserveLayersUpdate(GetLayersId(),
                                            mChildLayersObserverEpoch, true);
   }
--- a/gfx/layers/wr/WebRenderBridgeParent.h
+++ b/gfx/layers/wr/WebRenderBridgeParent.h
@@ -291,27 +291,24 @@ class WebRenderBridgeParent final
       return aRenderRoot;
     } else {
       MOZ_ASSERT(aRenderRoot == wr::RenderRoot::Default);
       return mRenderRoot;
     }
   }
 
   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,
                            wr::RenderRoot aRenderRoot);
   void UpdateAPZScrollOffsets(ScrollUpdatesMap&& aUpdates,
                               uint32_t aPaintSequenceNumber,
                               wr::RenderRoot aRenderRoot);
 
--- a/gfx/layers/wr/WebRenderLayerManager.cpp
+++ b/gfx/layers/wr/WebRenderLayerManager.cpp
@@ -233,17 +233,18 @@ bool WebRenderLayerManager::EndEmptyTran
       WrBridge()->GetSyncObject()->Synchronize();
     }
   }
 
   AutoTArray<RenderRootUpdates, wr::kRenderRootCount> renderRootUpdates;
   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) {
         stateManager.mAsyncResourceUpdates->Flush(updates->mResourceUpdates,
                                                   updates->mSmallShmems,
                                                   updates->mLargeShmems);
       }
       updates->mScrollUpdates = std::move(mPendingScrollUpdates[renderRoot]);
@@ -413,22 +414,25 @@ void WebRenderLayerManager::EndTransacti
   {
     AUTO_PROFILER_TRACING("Paint", "ForwardDPTransaction", GRAPHICS);
     InfallibleTArray<RenderRootDisplayListData> renderRootDLs;
     for (auto renderRoot : wr::kRenderRoots) {
       if (builder.GetSendSubBuilderDisplayList(renderRoot)) {
         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;
       }
     }
 
     WrBridge()->EndTransaction(renderRootDLs, mLatestTransactionId,
                                containsSVGGroup,
                                mTransactionIdAllocator->GetVsyncId(),
                                mTransactionIdAllocator->GetVsyncStart(),
                                refreshStart, mTransactionStart, mURL);
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -725,18 +725,18 @@ void DisplayListBuilder::Finalize(wr::La
 
 void DisplayListBuilder::Finalize(
     layers::RenderRootDisplayListData& aOutTransaction) {
   MOZ_ASSERT(mRenderRoot == wr::RenderRoot::Default);
   wr::VecU8 dl;
   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;
 }
 
 Maybe<wr::WrSpatialId> DisplayListBuilder::PushStackingContext(
     const wr::StackingContextParams& aParams, const wr::LayoutRect& aBounds,
     const wr::RasterSpace& aRasterSpace) {
   MOZ_ASSERT(mClipChainLeaf.isNothing(),