Bug 1538572 - Replace mApis nsTArray with RenderRootArray r=sotaro
authorDoug Thayer <dothayer@mozilla.com>
Fri, 05 Apr 2019 15:42:50 +0000
changeset 468186 1d3fca2f0721c26585e70fa9aabb9069c99052ec
parent 468185 dd68a6245f61cc68835c87f71b1116a75d41266f
child 468187 9c03d5b451e0a6c44b6e446ede5a8309701f3067
push id82437
push userdothayer@mozilla.com
push dateFri, 05 Apr 2019 15:47:11 +0000
treeherderautoland@1d3fca2f0721 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1538572
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 1538572 - Replace mApis nsTArray with RenderRootArray r=sotaro This will allow us to simply do null checks when trying to get an API, which will prevent problems with concurrently accessing an nsTArray. Differential Revision: https://phabricator.services.mozilla.com/D26190
gfx/layers/wr/WebRenderBridgeParent.cpp
gfx/layers/wr/WebRenderBridgeParent.h
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -298,17 +298,16 @@ WebRenderBridgeParent::WebRenderBridgePa
     const wr::PipelineId& aPipelineId, widget::CompositorWidget* aWidget,
     CompositorVsyncScheduler* aScheduler,
     nsTArray<RefPtr<wr::WebRenderAPI>>&& aApis,
     RefPtr<AsyncImagePipelineManager>&& aImageMgr,
     RefPtr<CompositorAnimationStorage>&& aAnimStorage, TimeDuration aVsyncRate)
     : mCompositorBridge(aCompositorBridge),
       mPipelineId(aPipelineId),
       mWidget(aWidget),
-      mApis(aApis),
       mAsyncImageManager(aImageMgr),
       mCompositorScheduler(aScheduler),
       mAnimStorage(aAnimStorage),
       mVsyncRate(aVsyncRate),
       mChildLayersObserverEpoch{0},
       mParentLayersObserverEpoch{0},
       mWrEpoch{0},
       mIdNamespace(aApis[0]->GetNamespace()),
@@ -325,16 +324,21 @@ WebRenderBridgeParent::WebRenderBridgePa
   if (IsRootWebRenderBridgeParent()) {
     MOZ_ASSERT(!mCompositorScheduler);
     mCompositorScheduler = new CompositorVsyncScheduler(this, mWidget);
   }
 
   if (!IsRootWebRenderBridgeParent() && gfxPrefs::WebRenderSplitRenderRoots()) {
     mRenderRoot = wr::RenderRoot::Content;
   }
+
+  for (auto& api : aApis) {
+    MOZ_ASSERT(api);
+    mApis[api->GetRenderRoot()] = api;
+  }
 }
 
 WebRenderBridgeParent::WebRenderBridgeParent(const wr::PipelineId& aPipelineId)
     : mCompositorBridge(nullptr),
       mPipelineId(aPipelineId),
       mChildLayersObserverEpoch{0},
       mParentLayersObserverEpoch{0},
       mWrEpoch{0},
@@ -737,17 +741,17 @@ mozilla::ipc::IPCResult WebRenderBridgeP
     nsTArray<RefCountedShmem>&& aSmallShmems,
     nsTArray<ipc::Shmem>&& aLargeShmems, const wr::RenderRoot& aRenderRoot) {
   if (mDestroyed) {
     wr::IpcResourceUpdateQueue::ReleaseShmems(this, aSmallShmems);
     wr::IpcResourceUpdateQueue::ReleaseShmems(this, aLargeShmems);
     return IPC_OK();
   }
 
-  MOZ_RELEASE_ASSERT((size_t)aRenderRoot < mApis.Length());
+  MOZ_RELEASE_ASSERT(aRenderRoot <= wr::kHighestRenderRoot);
 
   wr::TransactionBuilder txn;
   txn.SetLowPriority(!IsRootWebRenderBridgeParent());
 
   bool success =
       UpdateResources(aResourceUpdates, aSmallShmems, aLargeShmems, txn);
   wr::IpcResourceUpdateQueue::ReleaseShmems(this, aSmallShmems);
   wr::IpcResourceUpdateQueue::ReleaseShmems(this, aLargeShmems);
@@ -965,20 +969,17 @@ mozilla::ipc::IPCResult WebRenderBridgeP
       DestroyActor(op);
     }
     return IPC_OK();
   }
 
   // Guard against malicious content processes
   MOZ_RELEASE_ASSERT(aDisplayLists.Length() > 0);
   for (auto& displayList : aDisplayLists) {
-    // mApis.Length() should be the lowest possible length of things that we
-    // will be indexing via a RenderRoot, so it should be sufficient to check
-    // just that.
-    MOZ_RELEASE_ASSERT((size_t)displayList.mRenderRoot < mApis.Length());
+    MOZ_RELEASE_ASSERT(displayList.mRenderRoot <= wr::kHighestRenderRoot);
   }
 
   if (!IsRootWebRenderBridgeParent()) {
     CrashReporter::AnnotateCrashReport(CrashReporter::Annotation::URL, aTxnURL);
   }
 
   AUTO_PROFILER_TRACING("Paint", "SetDisplayList", GRAPHICS);
   UpdateFwdTransactionId(aFwdTransactionId);
@@ -1101,20 +1102,17 @@ mozilla::ipc::IPCResult WebRenderBridgeP
     for (const auto& op : aToDestroy) {
       DestroyActor(op);
     }
     return IPC_OK();
   }
 
   // Guard against malicious content processes
   for (auto& update : aRenderRootUpdates) {
-    // mApis.Length() should be the lowest possible length of things that we
-    // will be indexing via a RenderRoot, so it should be sufficient to check
-    // just that.
-    MOZ_RELEASE_ASSERT((size_t)update.mRenderRoot < mApis.Length());
+    MOZ_RELEASE_ASSERT(update.mRenderRoot <= wr::kHighestRenderRoot);
   }
 
   if (!IsRootWebRenderBridgeParent()) {
     CrashReporter::AnnotateCrashReport(CrashReporter::Annotation::URL, aTxnURL);
   }
 
   AUTO_PROFILER_TRACING("Paint", "EmptyTransaction", GRAPHICS);
   UpdateFwdTransactionId(aFwdTransactionId);
@@ -1656,17 +1654,19 @@ wr::Epoch WebRenderBridgeParent::UpdateW
   // allocation. In future, we could address the problem.
   Unused << SendWrUpdated(mIdNamespace, aTextureFactoryIdentifier);
   CompositorBridgeParentBase* cBridge = mCompositorBridge;
   // XXX Stop to clear resources if webreder supports resources sharing between
   // different webrender instances.
   ClearResources();
   mCompositorBridge = cBridge;
   mCompositorScheduler = aScheduler;
-  mApis = aApis;
+  for (auto& api : aApis) {
+    mApis[api->GetRenderRoot()] = api;
+  }
   mAsyncImageManager = aImageMgr;
   mAnimStorage = aAnimStorage;
 
   // Register pipeline to updated AsyncImageManager.
   mAsyncImageManager->AddPipeline(mPipelineId, this);
 
   return GetNextWrEpoch();  // Update webrender epoch
 }
@@ -1924,16 +1924,19 @@ void WebRenderBridgeParent::MaybeGenerat
   TimeStamp start = TimeStamp::Now();
   mAsyncImageManager->SetCompositionTime(start);
 
   wr::RenderRootArray<Maybe<wr::TransactionBuilder>> fastTxns;
   // Handle transaction that is related to DisplayList.
   wr::RenderRootArray<Maybe<wr::TransactionBuilder>> sceneBuilderTxns;
   wr::RenderRootArray<Maybe<wr::AutoTransactionSender>> senders;
   for (auto& api : mApis) {
+    if (!api) {
+      continue;
+    }
     auto renderRoot = api->GetRenderRoot();
     // Ensure GenerateFrame is handled on the render backend thread rather
     // than going through the scene builder thread. That way we continue
     // generating frames with the old scene even during slow scene builds.
     fastTxns[renderRoot].emplace(false /* useSceneBuilderThread */);
     sceneBuilderTxns[renderRoot].emplace();
     senders[renderRoot].emplace(api, sceneBuilderTxns[renderRoot].ptr());
   }
@@ -1948,16 +1951,19 @@ void WebRenderBridgeParent::MaybeGenerat
     // frame that we want to generate after this one.
     // It will check if we actually want to generate the frame or not.
     mCompositorScheduler->ScheduleComposition();
   }
 
   uint8_t framesGenerated = 0;
   wr::RenderRootArray<bool> generateFrame;
   for (auto& api : mApis) {
+    if (!api) {
+      continue;
+    }
     auto renderRoot = api->GetRenderRoot();
     generateFrame[renderRoot] =
         mAsyncImageManager->GetAndResetWillGenerateFrame(renderRoot) ||
         !fastTxns[renderRoot]->IsEmpty() || aForceGenerateFrame;
     if (generateFrame[renderRoot]) {
       framesGenerated++;
     }
   }
@@ -1974,16 +1980,19 @@ void WebRenderBridgeParent::MaybeGenerat
   if (SampleAnimations(opacityArrays, transformArrays)) {
     // TODO we should have a better way of assessing whether we need a content
     // or a chrome frame generation.
     ScheduleGenerateFrameAllRenderRoots();
   }
   // We do this even if the arrays are empty, because it will clear out any
   // previous properties store on the WR side, which is desirable.
   for (auto& api : mApis) {
+    if (!api) {
+      continue;
+    }
     auto renderRoot = api->GetRenderRoot();
     fastTxns[renderRoot]->UpdateDynamicProperties(opacityArrays[renderRoot],
                                                   transformArrays[renderRoot]);
   }
 
   SetAPZSampleTime();
 
   wr::RenderThread::Get()->IncPendingFrameCount(
@@ -1991,20 +2000,23 @@ void WebRenderBridgeParent::MaybeGenerat
 
 #if defined(ENABLE_FRAME_LATENCY_LOG)
   auto startTime = TimeStamp::Now();
   Api(wr::RenderRoot::Default)->SetFrameStartTime(startTime);
 #endif
 
   MOZ_ASSERT(framesGenerated > 0);
   for (auto& api : mApis) {
+    if (!api) {
+      continue;
+    }
     auto renderRoot = api->GetRenderRoot();
     if (generateFrame[renderRoot]) {
       fastTxns[renderRoot]->GenerateFrame();
-      mApis[(size_t)renderRoot]->SendTransaction(*fastTxns[renderRoot]);
+      api->SendTransaction(*fastTxns[renderRoot]);
     }
   }
   mMostRecentComposite = TimeStamp::Now();
 }
 
 void WebRenderBridgeParent::HoldPendingTransactionId(
     const wr::Epoch& aWrEpoch, TransactionId aTransactionId,
     bool aContainsSVGGroup, const VsyncId& aVsyncId,
@@ -2223,17 +2235,17 @@ bool WebRenderBridgeParent::Resume() {
     return false;
   }
 #endif
   mPaused = false;
   return true;
 }
 
 void WebRenderBridgeParent::ClearResources() {
-  if (mApis.IsEmpty()) {
+  if (!mApis[wr::RenderRoot::Default]) {
     return;
   }
 
   wr::Epoch wrEpoch = GetNextWrEpoch();
   mReceivedDisplayList = false;
   // Schedule generate frame to clean up Pipeline
   ScheduleGenerateFrameAllRenderRoots();
 
@@ -2250,16 +2262,19 @@ void WebRenderBridgeParent::ClearResourc
   for (const auto& entry : mSharedSurfaceIds) {
     mAsyncImageManager->HoldExternalImage(mPipelineId, mWrEpoch, entry.second);
   }
   mSharedSurfaceIds.clear();
 
   mAsyncImageManager->RemovePipeline(mPipelineId, wrEpoch);
 
   for (auto& api : mApis) {
+    if (!api) {
+      continue;
+    }
     wr::TransactionBuilder txn;
     txn.SetLowPriority(true);
     txn.ClearDisplayList(wrEpoch, mPipelineId);
 
     auto renderRoot = api->GetRenderRoot();
     for (const auto& entry : mAsyncCompositables[renderRoot]) {
       wr::PipelineId pipelineId = wr::AsPipelineId(entry.first);
       RefPtr<WebRenderImageHost> host = entry.second;
@@ -2281,17 +2296,19 @@ void WebRenderBridgeParent::ClearResourc
 
   if (IsRootWebRenderBridgeParent()) {
     mCompositorScheduler->Destroy();
   }
 
   mAnimStorage = nullptr;
   mCompositorScheduler = nullptr;
   mAsyncImageManager = nullptr;
-  mApis.Clear();
+  for (auto& api : mApis) {
+    api = nullptr;
+  }
   mCompositorBridge = nullptr;
 }
 
 bool WebRenderBridgeParent::ShouldParentObserveEpoch() {
   if (mParentLayersObserverEpoch == mChildLayersObserverEpoch) {
     return false;
   }
 
@@ -2343,17 +2360,17 @@ mozilla::ipc::IPCResult WebRenderBridgeP
   if (mDestroyed) {
     return IPC_OK();
   }
   ReleaseCompositable(aHandle);
   return IPC_OK();
 }
 
 TextureFactoryIdentifier WebRenderBridgeParent::GetTextureFactoryIdentifier() {
-  MOZ_ASSERT(!mApis.IsEmpty());
+  MOZ_ASSERT(!!mApis[wr::RenderRoot::Default]);
 
   return TextureFactoryIdentifier(
       LayersBackend::LAYERS_WR, XRE_GetProcessType(),
       Api(wr::RenderRoot::Default)->GetMaxTextureSize(), false,
       Api(wr::RenderRoot::Default)->GetUseANGLE(),
       Api(wr::RenderRoot::Default)->GetUseDComp(), false, false, false,
       Api(wr::RenderRoot::Default)->GetSyncHandle());
 }
--- a/gfx/layers/wr/WebRenderBridgeParent.h
+++ b/gfx/layers/wr/WebRenderBridgeParent.h
@@ -66,32 +66,34 @@ class WebRenderBridgeParent final
 
   static WebRenderBridgeParent* CreateDestroyed(
       const wr::PipelineId& aPipelineId);
 
   wr::PipelineId PipelineId() { return mPipelineId; }
 
   bool CloneWebRenderAPIs(nsTArray<RefPtr<wr::WebRenderAPI>>& aOutAPIs) {
     for (auto& api : mApis) {
-      RefPtr<wr::WebRenderAPI> clone = api->Clone();
-      if (!clone) {
-        return false;
+      if (api) {
+        RefPtr<wr::WebRenderAPI> clone = api->Clone();
+        if (!clone) {
+          return false;
+        }
+        aOutAPIs.AppendElement(clone);
       }
-      aOutAPIs.AppendElement(clone);
     }
     return true;
   }
   already_AddRefed<wr::WebRenderAPI> GetWebRenderAPIAtPoint(
       const ScreenPoint& aPoint);
   already_AddRefed<wr::WebRenderAPI> GetWebRenderAPI(
       wr::RenderRoot aRenderRoot) {
-    if ((size_t)aRenderRoot >= mApis.Length()) {
+    if (aRenderRoot > wr::kHighestRenderRoot) {
       return nullptr;
     }
-    return do_AddRef(mApis[(int)aRenderRoot]);
+    return do_AddRef(mApis[aRenderRoot]);
   }
   AsyncImagePipelineManager* AsyncImageManager() { return mAsyncImageManager; }
   CompositorVsyncScheduler* CompositorScheduler() {
     return mCompositorScheduler.get();
   }
   CompositorBridgeParentBase* GetCompositorBridge() {
     return mCompositorBridge;
   }
@@ -268,20 +270,20 @@ class WebRenderBridgeParent final
  private:
   class ScheduleSharedSurfaceRelease;
 
   explicit WebRenderBridgeParent(const wr::PipelineId& aPipelineId);
   virtual ~WebRenderBridgeParent();
 
   wr::WebRenderAPI* Api(wr::RenderRoot aRenderRoot) {
     if (IsRootWebRenderBridgeParent()) {
-      return mApis[(size_t)aRenderRoot];
+      return mApis[aRenderRoot];
     } else {
       MOZ_ASSERT(aRenderRoot == wr::RenderRoot::Default);
-      return mApis[(size_t)mRenderRoot];
+      return mApis[mRenderRoot];
     }
   }
 
   // Within WebRenderBridgeParent, we can use wr::RenderRoot::Default to
   // refer to DefaultApi(), which can be the content API if this
   // WebRenderBridgeParent is for a content WebRenderBridgeChild. However,
   // different WebRenderBridgeParents use the same AsyncImagePipelineManager,
   // for example, which doesn't have this distinction, so we need to
@@ -430,17 +432,24 @@ class WebRenderBridgeParent final
 
     wr::Epoch mEpoch;
     InfallibleTArray<uint64_t> mIds;
   };
 
   CompositorBridgeParentBase* MOZ_NON_OWNING_REF mCompositorBridge;
   wr::PipelineId mPipelineId;
   RefPtr<widget::CompositorWidget> mWidget;
-  nsTArray<RefPtr<wr::WebRenderAPI>> mApis;
+  // The RenderRootArray means there will always be a fixed number of apis,
+  // one for each RenderRoot, even if renderroot splitting isn't enabled.
+  // In this case, the unused apis will be nullptrs. Also, if this is not
+  // the root WebRenderBridgeParent, there should only be one api in this
+  // list. We avoid using a dynamically sized array for this because we
+  // need to be able to null these out in a thread-safe way from
+  // ClearResources, and there's no way to do that with an nsTArray.
+  wr::RenderRootArray<RefPtr<wr::WebRenderAPI>> mApis;
   RefPtr<AsyncImagePipelineManager> mAsyncImageManager;
   RefPtr<CompositorVsyncScheduler> mCompositorScheduler;
   RefPtr<CompositorAnimationStorage> mAnimStorage;
   // mActiveAnimations is used to avoid leaking animations when
   // WebRenderBridgeParent is destroyed abnormally and Tab move between
   // different windows.
   std::unordered_map<uint64_t, wr::Epoch> mActiveAnimations;
   wr::RenderRootArray<std::unordered_map<uint64_t, RefPtr<WebRenderImageHost>>>