Bug 1441308 - Always send parent commands when sending mDestroyedActors r=kats,sotaro
☠☠ backed out by 699a6b6bee44 ☠ ☠
authorDoug Thayer <dothayer@mozilla.com>
Thu, 21 Mar 2019 23:15:42 +0000
changeset 465583 1764701d11d103a2ac02f8cfa5ff4511b8f5dd70
parent 465582 32f7793dfd1adeda4004c9de20de14010865bf25
child 465584 0692b5b93271aa26737b771c6069f4d8511af636
push id35744
push userapavel@mozilla.com
push dateFri, 22 Mar 2019 16:44:08 +0000
treeherdermozilla-central@e66a2b59914d [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(),