Bug 1337761 - Part 2: Don't send external images to the parent side if a transaction is incomplete. r=nical
authorMason Chang <mchang@mozilla.com>
Wed, 19 Apr 2017 15:39:46 -0700
changeset 403269 73569501177174b1022ed18e439b1c397d5ebe3b
parent 403268 8a3a9bb423224f0d662947aa9fa11bbabdff78ba
child 403270 8b33bdca334e290ef6833e06a2c2919a21ae162b
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1337761
milestone55.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 1337761 - Part 2: Don't send external images to the parent side if a transaction is incomplete. r=nical
gfx/layers/wr/WebRenderBridgeChild.cpp
gfx/layers/wr/WebRenderBridgeChild.h
gfx/layers/wr/WebRenderLayerManager.cpp
gfx/layers/wr/WebRenderLayerManager.h
gfx/layers/wr/WebRenderPaintedLayer.cpp
--- a/gfx/layers/wr/WebRenderBridgeChild.cpp
+++ b/gfx/layers/wr/WebRenderBridgeChild.cpp
@@ -60,56 +60,60 @@ WebRenderBridgeChild::AddWebRenderParent
   MOZ_ASSERT(mIsInTransaction);
   mParentCommands.AppendElements(aCommands);
 }
 
 bool
 WebRenderBridgeChild::DPBegin(const gfx::IntSize& aSize)
 {
   MOZ_ASSERT(!mDestroyed);
-  MOZ_ASSERT(!mIsInTransaction);
 
   UpdateFwdTransactionId();
   this->SendDPBegin(aSize);
   mIsInTransaction = true;
   mReadLockSequenceNumber = 0;
   mReadLocks.AppendElement();
   return true;
 }
 
 void
-WebRenderBridgeChild::DPEnd(wr::DisplayListBuilder &aBuilder, const gfx::IntSize& aSize, bool aIsSync, uint64_t aTransactionId)
+WebRenderBridgeChild::ClearReadLocks()
 {
-  MOZ_ASSERT(!mDestroyed);
-  MOZ_ASSERT(mIsInTransaction);
-
   for (nsTArray<ReadLockInit>& locks : mReadLocks) {
     if (locks.Length()) {
       if (!SendInitReadLocks(locks)) {
         NS_WARNING("WARNING: sending read locks failed!");
         return;
       }
     }
   }
 
+  mReadLocks.Clear();
+}
+
+void
+WebRenderBridgeChild::DPEnd(wr::DisplayListBuilder &aBuilder, const gfx::IntSize& aSize, bool aIsSync, uint64_t aTransactionId)
+{
+  MOZ_ASSERT(!mDestroyed);
+  MOZ_ASSERT(mIsInTransaction);
+
   wr::BuiltDisplayList dl = aBuilder.Finalize();
   ByteBuffer dlData(Move(dl.dl));
   ByteBuffer auxData(Move(dl.aux));
 
   if (aIsSync) {
     this->SendDPSyncEnd(aSize, mParentCommands, mDestroyedActors, GetFwdTransactionId(), aTransactionId,
                         dlData, dl.dl_desc, auxData, dl.aux_desc);
   } else {
     this->SendDPEnd(aSize, mParentCommands, mDestroyedActors, GetFwdTransactionId(), aTransactionId,
                     dlData, dl.dl_desc, auxData, dl.aux_desc);
   }
 
   mParentCommands.Clear();
   mDestroyedActors.Clear();
-  mReadLocks.Clear();
   mIsInTransaction = false;
 }
 
 wr::ExternalImageId
 WebRenderBridgeChild::GetNextExternalImageId()
 {
   return GetCompositorBridgeChild()->GetNextExternalImageId();
 }
--- a/gfx/layers/wr/WebRenderBridgeChild.h
+++ b/gfx/layers/wr/WebRenderBridgeChild.h
@@ -91,16 +91,17 @@ public:
 
   void PushGlyphs(wr::DisplayListBuilder& aBuilder, const nsTArray<GlyphArray>& aGlyphs,
                   gfx::ScaledFont* aFont, const gfx::Point& aOffset, const gfx::Rect& aBounds,
                   const gfx::Rect& aClip);
 
   wr::FontKey GetFontKeyForScaledFont(gfx::ScaledFont* aScaledFont);
 
   void RemoveExpiredFontKeys();
+  void ClearReadLocks();
 
 private:
   friend class CompositorBridgeChild;
 
   ~WebRenderBridgeChild() {}
 
   wr::ExternalImageId GetNextExternalImageId();
 
--- a/gfx/layers/wr/WebRenderLayerManager.cpp
+++ b/gfx/layers/wr/WebRenderLayerManager.cpp
@@ -330,16 +330,17 @@ WebRenderLayerManager::BeginTransaction(
 
 bool
 WebRenderLayerManager::EndEmptyTransaction(EndTransactionFlags aFlags)
 {
   if (!mRoot) {
     return false;
   }
 
+  // We might used painted layer images so don't delete them yet.
   return EndTransactionInternal(nullptr, nullptr, aFlags);
 }
 
 void
 WebRenderLayerManager::EndTransaction(DrawPaintedLayerCallback aCallback,
                                       void* aCallbackData,
                                       EndTransactionFlags aFlags)
 {
@@ -368,33 +369,42 @@ WebRenderLayerManager::EndTransactionInt
   if (!WrBridge()->DPBegin(size.ToUnknownSize())) {
     return false;
   }
   DiscardCompositorAnimations();
   mRoot->StartPendingAnimations(mAnimationReadyTime);
 
   wr::DisplayListBuilder builder(WrBridge()->GetPipeline());
   WebRenderLayer::ToWebRenderLayer(mRoot)->RenderLayer(builder);
+  WrBridge()->ClearReadLocks();
+
+  // We can't finish this transaction so return. This usually
+  // happens in an empty transaction where we can't repaint a painted layer.
+  // In this case, leave the transaction open and let a full transaction happen.
+  if (mTransactionIncomplete) {
+    DiscardLocalImages();
+    return false;
+  }
 
   bool sync = mTarget != nullptr;
   mLatestTransactionId = mTransactionIdAllocator->GetTransactionId();
 
   WrBridge()->DPEnd(builder, size.ToUnknownSize(), sync, mLatestTransactionId);
 
   MakeSnapshotIfRequired(size);
   mNeedsComposite = false;
 
   ClearDisplayItemLayers();
 
   // this may result in Layers being deleted, which results in
   // PLayer::Send__delete__() and DeallocShmem()
   mKeepAlive.Clear();
   ClearMutatedLayers();
 
-  return !mTransactionIncomplete;
+  return true;
 }
 
 void
 WebRenderLayerManager::MakeSnapshotIfRequired(LayoutDeviceIntSize aSize)
 {
   if (!mTarget || aSize.IsEmpty()) {
     return;
   }
@@ -479,16 +489,26 @@ void
 WebRenderLayerManager::DiscardCompositorAnimations()
 {
   for (auto id : mDiscardedCompositorAnimationsIds) {
     WrBridge()->AddWebRenderParentCommand(OpRemoveCompositorAnimations(id));
   }
   mDiscardedCompositorAnimationsIds.clear();
 }
 
+void
+WebRenderLayerManager::DiscardLocalImages()
+{
+  // Removes images but doesn't tell the parent side about them
+  // This is useful in empty / failed transactions where we created
+  // image keys but didn't tell the parent about them yet.
+  mImageKeys.clear();
+}
+
+void
 WebRenderLayerManager::Mutated(Layer* aLayer)
 {
   LayerManager::Mutated(aLayer);
   AddMutatedLayer(aLayer);
 }
 
 void
 WebRenderLayerManager::MutatedSimple(Layer* aLayer)
--- a/gfx/layers/wr/WebRenderLayerManager.h
+++ b/gfx/layers/wr/WebRenderLayerManager.h
@@ -149,16 +149,17 @@ public:
 
   void* GetPaintedLayerCallbackData() const
   { return mPaintedLayerCallbackData; }
 
   // adds an imagekey to a list of keys that will be discarded on the next
   // transaction or destruction
   void AddImageKeyForDiscard(wr::ImageKey);
   void DiscardImages();
+  void DiscardLocalImages();
 
   // Before destroying a layer with animations, add its compositorAnimationsId
   // to a list of ids that will be discarded on the next transaction
   void AddCompositorAnimationsIdForDiscard(uint64_t aId);
   void DiscardCompositorAnimations();
 
   WebRenderBridgeChild* WrBridge() const { return mWrChild; }
 
--- a/gfx/layers/wr/WebRenderPaintedLayer.cpp
+++ b/gfx/layers/wr/WebRenderPaintedLayer.cpp
@@ -76,17 +76,17 @@ WebRenderPaintedLayer::UpdateImageClient
 
     if (gfxPrefs::WebRenderHighlightPaintedLayers()) {
       target->SetTransform(Matrix());
       target->FillRect(Rect(0, 0, imageSize.width, imageSize.height), ColorPattern(Color(1.0, 0.0, 0.0, 0.5)));
     }
   }
 
   if (!helper.UpdateImage()) {
-    return;
+    return false;
   }
 
   return true;
 }
 
 void
 WebRenderPaintedLayer::CreateWebRenderDisplayList(wr::DisplayListBuilder& aBuilder)
 {