Bug 1325227 - Part 1: Remove synchronous transaction replies since they don't return any data we didn't already have. r=nical

MozReview-Commit-ID: 2HIbk65L7zR

gfx/layers/client/ClientPaintedLayer.cpp

@@ -30,7 +30,7 @@ namespace layers {
 using namespace mozilla::gfx;
 
 void
-ClientPaintedLayer::PaintThebes()
+ClientPaintedLayer::PaintThebes(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates)
 {
   PROFILER_LABEL("ClientPaintedLayer", "PaintThebes",
     js::ProfileEntry::Category::GRAPHICS);
@@ -38,6 +38,8 @@ ClientPaintedLayer::PaintThebes()
   NS_ASSERTION(ClientManager()->InDrawing(),
                "Can only draw in drawing phase");
   
+  mContentClient->BeginPaint();
+
   uint32_t flags = RotatedContentBuffer::PAINT_CAN_DRAW_ROTATED;
 #ifndef MOZ_IGNORE_PAINT_WILL_RESAMPLE
   if (ClientManager()->CompositorMightResample()) {
@@ -93,6 +95,8 @@ ClientPaintedLayer::PaintThebes()
     didUpdate = true;
   }
 
+  mContentClient->EndPaint(aReadbackUpdates);
+
   if (didUpdate) {
     Mutated();
 
@@ -132,10 +136,7 @@ ClientPaintedLayer::RenderLayerWithReadback(ReadbackProcessor *aReadback)
     aReadback->GetPaintedLayerUpdates(this, &readbackUpdates);
   }
 
-  IntPoint origin(mVisibleRegion.GetBounds().x, mVisibleRegion.GetBounds().y);
+  PaintThebes(&readbackUpdates);
-  mContentClient->BeginPaint();
-  PaintThebes();
-  mContentClient->EndPaint(&readbackUpdates);
 }
 
 already_AddRefed<PaintedLayer>

gfx/layers/client/ClientPaintedLayer.h

@@ -108,7 +108,7 @@ public:
   }
 
 protected:
-  void PaintThebes();
+  void PaintThebes(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates);
 
   virtual void PrintInfo(std::stringstream& aStream, const char* aPrefix) override;
 

gfx/layers/client/ContentClient.cpp

@@ -410,10 +410,14 @@ ContentClientRemoteBuffer::Updated(const nsIntRegion& aRegionToDraw,
     t->mPictureRect = nsIntRect(0, 0, size.width, size.height);
     GetForwarder()->UseTextures(this, textures);
   }
+  // This forces a synchronous transaction, so we can swap buffers now
+  // and know that we'll have sole ownership of the old front buffer
+  // by the time we paint next.
   mForwarder->UpdateTextureRegion(this,
                                   ThebesBufferData(BufferRect(),
                                                    BufferRotation()),
                                   updatedRegion);
+  SwapBuffers(updatedRegion);
 }
 
 void

gfx/layers/composite/CompositableHost.h

@@ -105,13 +105,11 @@ public:
 
   /**
    * Update the content host.
-   * aUpdated is the region which should be updated
+   * aUpdated is the region which should be updated.
-   * aUpdatedRegionBack is the region in aNewBackResult which has been updated
    */
   virtual bool UpdateThebes(const ThebesBufferData& aData,
                             const nsIntRegion& aUpdated,
-                            const nsIntRegion& aOldValidRegionBack,
+                            const nsIntRegion& aOldValidRegionBack)
-                            nsIntRegion* aUpdatedRegionBack)
   {
     NS_ERROR("should be implemented or not used");
     return false;

gfx/layers/composite/ContentHost.cpp

@@ -322,11 +322,8 @@ AddWrappedRegion(const nsIntRegion& aInput, nsIntRegion& aOutput,
 bool
 ContentHostSingleBuffered::UpdateThebes(const ThebesBufferData& aData,
                                         const nsIntRegion& aUpdated,
-                                        const nsIntRegion& aOldValidRegionBack,
+                                        const nsIntRegion& aOldValidRegionBack)
-                                        nsIntRegion* aUpdatedRegionBack)
 {
-  aUpdatedRegionBack->SetEmpty();
-
   if (!mTextureHost) {
     mInitialised = false;
     return true; // FIXME should we return false? Returning true for now
@@ -385,13 +382,10 @@ ContentHostSingleBuffered::UpdateThebes(const ThebesBufferData& aData,
 bool
 ContentHostDoubleBuffered::UpdateThebes(const ThebesBufferData& aData,
                                         const nsIntRegion& aUpdated,
-                                        const nsIntRegion& aOldValidRegionBack,
+                                        const nsIntRegion& aOldValidRegionBack)
-                                        nsIntRegion* aUpdatedRegionBack)
 {
   if (!mTextureHost) {
     mInitialised = false;
-
-    *aUpdatedRegionBack = aUpdated;
     return true;
   }
 
@@ -407,8 +401,6 @@ ContentHostDoubleBuffered::UpdateThebes(const ThebesBufferData& aData,
   mBufferRect = aData.rect();
   mBufferRotation = aData.rotation();
 
-  *aUpdatedRegionBack = aUpdated;
-
   // Save the current valid region of our front buffer, because if
   // we're double buffering, it's going to be the valid region for the
   // next back buffer sent back to the renderer.

gfx/layers/composite/ContentHost.h

@@ -54,8 +54,7 @@ class ContentHost : public CompositableHost
 public:
   virtual bool UpdateThebes(const ThebesBufferData& aData,
                             const nsIntRegion& aUpdated,
-                            const nsIntRegion& aOldValidRegionBack,
+                            const nsIntRegion& aOldValidRegionBack) = 0;
-                            nsIntRegion* aUpdatedRegionBack) = 0;
 
   virtual void SetPaintWillResample(bool aResample) { mPaintWillResample = aResample; }
   bool PaintWillResample() { return mPaintWillResample; }
@@ -197,8 +196,7 @@ public:
 
   virtual bool UpdateThebes(const ThebesBufferData& aData,
                             const nsIntRegion& aUpdated,
-                            const nsIntRegion& aOldValidRegionBack,
+                            const nsIntRegion& aOldValidRegionBack);
-                            nsIntRegion* aUpdatedRegionBack);
 
 protected:
   nsIntRegion mValidRegionForNextBackBuffer;
@@ -220,8 +218,7 @@ public:
 
   virtual bool UpdateThebes(const ThebesBufferData& aData,
                             const nsIntRegion& aUpdated,
-                            const nsIntRegion& aOldValidRegionBack,
+                            const nsIntRegion& aOldValidRegionBack);
-                            nsIntRegion* aUpdatedRegionBack);
 };
 
 } // namespace layers

gfx/layers/composite/TiledContentHost.h

@@ -202,8 +202,7 @@ public:
 
   virtual bool UpdateThebes(const ThebesBufferData& aData,
                             const nsIntRegion& aUpdated,
-                            const nsIntRegion& aOldValidRegionBack,
+                            const nsIntRegion& aOldValidRegionBack) override
-                            nsIntRegion* aUpdatedRegionBack) override
   {
     NS_ERROR("N/A for tiled layers");
     return false;

gfx/layers/ipc/CompositableTransactionParent.cpp

@@ -61,8 +61,7 @@ ScheduleComposition(CompositableHost* aCompositable)
 }
 
 bool
-CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation& aEdit,
+CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation& aEdit)
-                                                     EditReplyVector& replyv)
 {
   // Ignore all operations on compositables created on stale compositors. We
   // return true because the child is unable to handle errors.
@@ -89,16 +88,12 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation
 
       RenderTraceInvalidateStart(thebes, "FF00FF", op.updatedRegion().GetBounds());
 
-      nsIntRegion frontUpdatedRegion;
       if (!compositable->UpdateThebes(bufferData,
                                       op.updatedRegion(),
-                                      thebes->GetValidRegion(),
+                                      thebes->GetValidRegion()))
-                                      &frontUpdatedRegion))
       {
         return false;
       }
-      replyv.push_back(
-        OpContentBufferSwap(aEdit.compositable(), frontUpdatedRegion));
 
       RenderTraceInvalidateEnd(thebes, "FF00FF");
       break;

gfx/layers/ipc/CompositableTransactionParent.h

@@ -17,8 +17,6 @@
 namespace mozilla {
 namespace layers {
 
-typedef std::vector<mozilla::layers::EditReply> EditReplyVector;
-
 // Since PCompositble has two potential manager protocols, we can't just call
 // the Manager() method usually generated when there's one manager protocol,
 // so both manager protocols implement this and we keep a reference to them
@@ -47,8 +45,7 @@ protected:
   /**
    * Handle the IPDL messages that affect PCompositable actors.
    */
-  bool ReceiveCompositableUpdate(const CompositableOperation& aEdit,
+  bool ReceiveCompositableUpdate(const CompositableOperation& aEdit);
-                                 EditReplyVector& replyv);
 
   void ReleaseCompositable(const CompositableHandle& aHandle);
 

gfx/layers/ipc/ImageBridgeChild.cpp

@@ -555,10 +555,8 @@ ImageBridgeChild::EndTransaction()
     ShadowLayerForwarder::PlatformSyncBeforeUpdate();
   }
 
-  AutoTArray<EditReply, 10> replies;
-
   if (mTxn->mSwapRequired) {
-    if (!SendUpdate(cset, mTxn->mDestroyedActors, GetFwdTransactionId(), &replies)) {
+    if (!SendUpdate(cset, mTxn->mDestroyedActors, GetFwdTransactionId())) {
       NS_WARNING("could not send async texture transaction");
       return;
     }
@@ -570,9 +568,6 @@ ImageBridgeChild::EndTransaction()
       return;
     }
   }
-  for (nsTArray<EditReply>::size_type i = 0; i < replies.Length(); ++i) {
-    MOZ_CRASH("not reached");
-  }
 }
 
 void

gfx/layers/ipc/ImageBridgeParent.cpp

@@ -154,26 +154,19 @@ private:
 
 mozilla::ipc::IPCResult
 ImageBridgeParent::RecvUpdate(EditArray&& aEdits, OpDestroyArray&& aToDestroy,
-                              const uint64_t& aFwdTransactionId,
+                              const uint64_t& aFwdTransactionId)
-                              EditReplyArray* aReply)
 {
   // This ensures that destroy operations are always processed. It is not safe
   // to early-return from RecvUpdate without doing so.
   AutoImageBridgeParentAsyncMessageSender autoAsyncMessageSender(this, &aToDestroy);
   UpdateFwdTransactionId(aFwdTransactionId);
 
-  EditReplyVector replyv;
   for (EditArray::index_type i = 0; i < aEdits.Length(); ++i) {
-    if (!ReceiveCompositableUpdate(aEdits[i], replyv)) {
+    if (!ReceiveCompositableUpdate(aEdits[i])) {
       return IPC_FAIL_NO_REASON(this);
     }
   }
 
-  aReply->SetCapacity(replyv.size());
-  if (replyv.size() > 0) {
-    aReply->AppendElements(&replyv.front(), replyv.size());
-  }
-
   if (!IsSameProcess()) {
     // Ensure that any pending operations involving back and front
     // buffers have completed, so that neither process stomps on the
@@ -188,9 +181,7 @@ mozilla::ipc::IPCResult
 ImageBridgeParent::RecvUpdateNoSwap(EditArray&& aEdits, OpDestroyArray&& aToDestroy,
                                     const uint64_t& aFwdTransactionId)
 {
-  InfallibleTArray<EditReply> noReplies;
+  bool success = RecvUpdate(Move(aEdits), Move(aToDestroy), aFwdTransactionId);
-  bool success = RecvUpdate(Move(aEdits), Move(aToDestroy), aFwdTransactionId, &noReplies);
-  MOZ_ASSERT(noReplies.Length() == 0, "RecvUpdateNoSwap requires a sync Update to carry Edits");
   if (!success) {
     return IPC_FAIL_NO_REASON(this);
   }

gfx/layers/ipc/ImageBridgeParent.h

@@ -43,7 +43,6 @@ class ImageBridgeParent final : public PImageBridgeParent,
 public:
   typedef InfallibleTArray<CompositableOperation> EditArray;
   typedef InfallibleTArray<OpDestroy> OpDestroyArray;
-  typedef InfallibleTArray<EditReply> EditReplyArray;
 
 protected:
   ImageBridgeParent(MessageLoop* aLoop, ProcessId aChildProcessId);
@@ -72,8 +71,7 @@ public:
   // PImageBridge
   virtual mozilla::ipc::IPCResult RecvImageBridgeThreadId(const PlatformThreadId& aThreadId) override;
   virtual mozilla::ipc::IPCResult RecvUpdate(EditArray&& aEdits, OpDestroyArray&& aToDestroy,
-                                          const uint64_t& aFwdTransactionId,
+                                          const uint64_t& aFwdTransactionId) override;
-                                          EditReplyArray* aReply) override;
   virtual mozilla::ipc::IPCResult RecvUpdateNoSwap(EditArray&& aEdits, OpDestroyArray&& aToDestroy,
                                                 const uint64_t& aFwdTransactionId) override;
 

gfx/layers/ipc/LayerTransactionParent.cpp

@@ -41,8 +41,6 @@
 #include "mozilla/layers/TextureHost.h"
 #include "mozilla/layers/AsyncCompositionManager.h"
 
-typedef std::vector<mozilla::layers::EditReply> EditReplyVector;
-
 using mozilla::layout::RenderFrameParent;
 
 namespace mozilla {
@@ -99,7 +97,7 @@ LayerTransactionParent::Destroy()
 mozilla::ipc::IPCResult
 LayerTransactionParent::RecvUpdateNoSwap(const TransactionInfo& txn)
 {
-  return RecvUpdate(txn, nullptr);
+  return RecvUpdate(txn);
 }
 
 class MOZ_STACK_CLASS AutoLayerTransactionParentAsyncMessageSender
@@ -140,8 +138,7 @@ LayerTransactionParent::RecvPaintTime(const uint64_t& aTransactionId,
 }
 
 mozilla::ipc::IPCResult
-LayerTransactionParent::RecvUpdate(const TransactionInfo& aInfo,
+LayerTransactionParent::RecvUpdate(const TransactionInfo& aInfo)
-                                   InfallibleTArray<EditReply>* reply)
 {
   GeckoProfilerTracingRAII tracer("Paint", "LayerTransaction");
   PROFILER_LABEL("LayerTransactionParent", "RecvUpdate",
@@ -165,7 +162,6 @@ LayerTransactionParent::RecvUpdate(const TransactionInfo& aInfo,
   // This ensures that destroy operations are always processed. It is not safe
   // to early-return from RecvUpdate without doing so.
   AutoLayerTransactionParentAsyncMessageSender autoAsyncMessageSender(this, &aInfo.toDestroy());
-  EditReplyVector replyv;
 
   {
     AutoResolveRefLayers resolve(mCompositorBridge->GetCompositionManager(this));
@@ -385,8 +381,7 @@ LayerTransactionParent::RecvUpdate(const TransactionInfo& aInfo,
       break;
     }
     case Edit::TCompositableOperation: {
-      if (!ReceiveCompositableUpdate(edit.get_CompositableOperation(),
+      if (!ReceiveCompositableUpdate(edit.get_CompositableOperation())) {
-                                replyv)) {
         return IPC_FAIL_NO_REASON(this);
       }
       break;
@@ -462,7 +457,7 @@ LayerTransactionParent::RecvUpdate(const TransactionInfo& aInfo,
 
   // Process paints separately, after all normal edits.
   for (const auto& op : aInfo.paints()) {
-    if (!ReceiveCompositableUpdate(op, replyv)) {
+    if (!ReceiveCompositableUpdate(op)) {
       return IPC_FAIL_NO_REASON(this);
     }
   }
@@ -474,13 +469,6 @@ LayerTransactionParent::RecvUpdate(const TransactionInfo& aInfo,
     layer_manager()->EndTransaction(TimeStamp(), LayerManager::END_NO_IMMEDIATE_REDRAW);
   }
 
-  if (reply) {
-    reply->SetCapacity(replyv.size());
-    if (replyv.size() > 0) {
-      reply->AppendElements(&replyv.front(), replyv.size());
-    }
-  }
-
   if (!IsSameProcess()) {
     // Ensure that any pending operations involving back and front
     // buffers have completed, so that neither process stomps on the

gfx/layers/ipc/LayerTransactionParent.h

@@ -42,7 +42,6 @@ class LayerTransactionParent final : public PLayerTransactionParent,
   typedef mozilla::layout::RenderFrameParent RenderFrameParent;
   typedef InfallibleTArray<Edit> EditArray;
   typedef InfallibleTArray<OpDestroy> OpDestroyArray;
-  typedef InfallibleTArray<EditReply> EditReplyArray;
   typedef InfallibleTArray<PluginWindowData> PluginsArray;
 
 public:
@@ -112,8 +111,7 @@ protected:
   virtual mozilla::ipc::IPCResult RecvPaintTime(const uint64_t& aTransactionId,
                                                 const TimeDuration& aPaintTime) override;
 
-  virtual mozilla::ipc::IPCResult RecvUpdate(const TransactionInfo& aInfo,
+  virtual mozilla::ipc::IPCResult RecvUpdate(const TransactionInfo& aInfo) override;
-                                             EditReplyArray* reply) override;
 
   virtual mozilla::ipc::IPCResult RecvUpdateNoSwap(const TransactionInfo& aInfo) override;
 

gfx/layers/ipc/LayersMessages.ipdlh

@@ -506,12 +506,6 @@ struct ImageCompositeNotification {
   uint32_t producerID;
 };
 
-// Unit of a "changeset reply".  This is a weird abstraction, probably
-// only to be used for buffer swapping.
-union EditReply {
-  OpContentBufferSwap;
-};
-
 union AsyncParentMessageData {
   OpNotifyNotUsed;
 };

gfx/layers/ipc/PImageBridge.ipdl

@@ -38,8 +38,7 @@ child:
 parent:
   async ImageBridgeThreadId(PlatformThreadId aTreahdId);
 
-  sync Update(CompositableOperation[] ops, OpDestroy[] toDestroy, uint64_t fwdTransactionId)
+  sync Update(CompositableOperation[] ops, OpDestroy[] toDestroy, uint64_t fwdTransactionId);
-    returns (EditReply[] reply);
 
   async UpdateNoSwap(CompositableOperation[] ops, OpDestroy[] toDestroy, uint64_t fwdTransactionId);
 

gfx/layers/ipc/PLayerTransaction.ipdl

@@ -52,7 +52,7 @@ sync protocol PLayerTransaction {
 parent:
   // The isFirstPaint flag can be used to indicate that this is the first update
   // for a particular document.
-  sync Update(TransactionInfo txn) returns (EditReply[] reply);
+  sync Update(TransactionInfo txn);
 
   async PaintTime(uint64_t id, TimeDuration paintTime);
 

gfx/layers/ipc/ShadowLayers.cpp

@@ -727,15 +727,13 @@ ShadowLayerForwarder::EndTransaction(const nsIntRegion& aRegionToClear,
 
   profiler_tracing("Paint", "Rasterize", TRACING_INTERVAL_END);
 
-  AutoTArray<EditReply, 10> replies;
   if (mTxn->mSwapRequired) {
     MOZ_LAYERS_LOG(("[LayersForwarder] sending transaction..."));
     RenderTraceScope rendertrace3("Forward Transaction", "000093");
-    if (!mShadowManager->SendUpdate(info, &replies)) {
+    if (!mShadowManager->SendUpdate(info)) {
       MOZ_LAYERS_LOG(("[LayersForwarder] WARNING: sending transaction failed!"));
       return false;
     }
-    ProcessReplies(replies);
   } else {
     // If we don't require a swap we can call SendUpdateNoSwap which
     // assumes that aReplies is empty (DEBUG assertion)
@@ -754,29 +752,6 @@ ShadowLayerForwarder::EndTransaction(const nsIntRegion& aRegionToClear,
   return true;
 }
 
-void
-ShadowLayerForwarder::ProcessReplies(const nsTArray<EditReply>& aReplies)
-{
-  for (const auto& reply : aReplies) {
-    switch (reply.type()) {
-    case EditReply::TOpContentBufferSwap: {
-      MOZ_LAYERS_LOG(("[LayersForwarder] DoubleBufferSwap"));
-
-      const OpContentBufferSwap& obs = reply.get_OpContentBufferSwap();
-
-      RefPtr<CompositableClient> compositable = FindCompositable(obs.compositable());
-      ContentClientRemote* contentClient = compositable->AsContentClientRemote();
-      MOZ_ASSERT(contentClient);
-
-      contentClient->SwapBuffers(obs.frontUpdatedRegion());
-      break;
-    }
-    default:
-      MOZ_CRASH("not reached");
-    }
-  }
-}
-
 RefPtr<CompositableClient>
 ShadowLayerForwarder::FindCompositable(const CompositableHandle& aHandle)
 {

gfx/layers/ipc/ShadowLayers.h

@@ -33,7 +33,6 @@ namespace layers {
 
 class ClientLayerManager;
 class CompositorBridgeChild;
-class EditReply;
 class FixedSizeSmallShmemSectionAllocator;
 class ImageContainer;
 class Layer;
@@ -421,8 +420,6 @@ protected:
   void CheckSurfaceDescriptor(const SurfaceDescriptor* aDescriptor) const {}
 #endif
 
-  void ProcessReplies(const nsTArray<EditReply>& aReplies);
-
   RefPtr<CompositableClient> FindCompositable(const CompositableHandle& aHandle);
 
   bool InWorkerThread();

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -336,9 +336,7 @@ WebRenderBridgeParent::ProcessWebrenderCommands(InfallibleTArray<WebRenderComman
         break;
       }
       case WebRenderCommand::TCompositableOperation: {
-        EditReplyVector replyv;
+        if (!ReceiveCompositableUpdate(cmd.get_CompositableOperation())) {
-        if (!ReceiveCompositableUpdate(cmd.get_CompositableOperation(),
-                                       replyv)) {
           NS_ERROR("ReceiveCompositableUpdate failed");
         }
         break;