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

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

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -303,7 +303,6 @@ WebRenderBridgeParent::WebRenderBridgeParent(
     : mCompositorBridge(aCompositorBridge),
       mPipelineId(aPipelineId),
       mWidget(aWidget),
-      mApis(aApis),
       mAsyncImageManager(aImageMgr),
       mCompositorScheduler(aScheduler),
       mAnimStorage(aAnimStorage),
@@ -330,6 +329,11 @@ WebRenderBridgeParent::WebRenderBridgeParent(
   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)
@@ -742,7 +746,7 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvUpdateResources(
     return IPC_OK();
   }
 
-  MOZ_RELEASE_ASSERT((size_t)aRenderRoot < mApis.Length());
+  MOZ_RELEASE_ASSERT(aRenderRoot <= wr::kHighestRenderRoot);
 
   wr::TransactionBuilder txn;
   txn.SetLowPriority(!IsRootWebRenderBridgeParent());
@@ -970,10 +974,7 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
   // 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()) {
@@ -1106,10 +1107,7 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEmptyTransaction(
 
   // 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()) {
@@ -1661,7 +1659,9 @@ wr::Epoch WebRenderBridgeParent::UpdateWebRender(
   ClearResources();
   mCompositorBridge = cBridge;
   mCompositorScheduler = aScheduler;
-  mApis = aApis;
+  for (auto& api : aApis) {
+    mApis[api->GetRenderRoot()] = api;
+  }
   mAsyncImageManager = aImageMgr;
   mAnimStorage = aAnimStorage;
 
@@ -1929,6 +1929,9 @@ void WebRenderBridgeParent::MaybeGenerateFrame(VsyncId aId,
   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
@@ -1953,6 +1956,9 @@ void WebRenderBridgeParent::MaybeGenerateFrame(VsyncId aId,
   uint8_t framesGenerated = 0;
   wr::RenderRootArray<bool> generateFrame;
   for (auto& api : mApis) {
+    if (!api) {
+      continue;
+    }
     auto renderRoot = api->GetRenderRoot();
     generateFrame[renderRoot] =
         mAsyncImageManager->GetAndResetWillGenerateFrame(renderRoot) ||
@@ -1979,6 +1985,9 @@ void WebRenderBridgeParent::MaybeGenerateFrame(VsyncId aId,
   // 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]);
@@ -1996,10 +2005,13 @@ void WebRenderBridgeParent::MaybeGenerateFrame(VsyncId aId,
 
   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();
@@ -2228,7 +2240,7 @@ bool WebRenderBridgeParent::Resume() {
 }
 
 void WebRenderBridgeParent::ClearResources() {
-  if (mApis.IsEmpty()) {
+  if (!mApis[wr::RenderRoot::Default]) {
     return;
   }
 
@@ -2255,6 +2267,9 @@ void WebRenderBridgeParent::ClearResources() {
   mAsyncImageManager->RemovePipeline(mPipelineId, wrEpoch);
 
   for (auto& api : mApis) {
+    if (!api) {
+      continue;
+    }
     wr::TransactionBuilder txn;
     txn.SetLowPriority(true);
     txn.ClearDisplayList(wrEpoch, mPipelineId);
@@ -2286,7 +2301,9 @@ void WebRenderBridgeParent::ClearResources() {
   mAnimStorage = nullptr;
   mCompositorScheduler = nullptr;
   mAsyncImageManager = nullptr;
-  mApis.Clear();
+  for (auto& api : mApis) {
+    api = nullptr;
+  }
   mCompositorBridge = nullptr;
 }
 
@@ -2348,7 +2365,7 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvReleaseCompositable(
 }
 
 TextureFactoryIdentifier WebRenderBridgeParent::GetTextureFactoryIdentifier() {
-  MOZ_ASSERT(!mApis.IsEmpty());
+  MOZ_ASSERT(!!mApis[wr::RenderRoot::Default]);
 
   return TextureFactoryIdentifier(
       LayersBackend::LAYERS_WR, XRE_GetProcessType(),

gfx/layers/wr/WebRenderBridgeParent.h

@@ -71,11 +71,13 @@ class WebRenderBridgeParent final
 
   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;
   }
@@ -83,10 +85,10 @@ class WebRenderBridgeParent final
       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() {
@@ -273,10 +275,10 @@ class WebRenderBridgeParent final
 
   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];
     }
   }
 
@@ -435,7 +437,14 @@ class WebRenderBridgeParent final
   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;