Bug 1419539 - Don't update content client if all we do is buffer operations. r=bas, a=jcristau
authorRyan Hunt <rhunt@eqrion.net>
Mon, 27 Nov 2017 11:26:49 -0500
changeset 442805 d4455df557afa84b651b2529fc4aa84bfef02b0c
parent 442804 a5f6db550124044d2c877aeba4308a3f7ddf1394
child 442806 f36855025582c0c6c88b41d3b207c462769ad839
push id8343
push userryanvm@gmail.com
push dateThu, 07 Dec 2017 13:36:26 +0000
treeherdermozilla-beta@1cc170b6689a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbas, jcristau
bugs1419539
milestone58.0
Bug 1419539 - Don't update content client if all we do is buffer operations. r=bas, a=jcristau PaintOffMainThread uses didUpdate to track whether we queued work on the paint thread and also if we painted new content into the buffer and need to update the content client. These are independent and should have different flags. We may flip the buffers incorrectly if we don't do this. This is also important because ContentClient can return a BufferState even if there are no operations and no backing buffer, causing a crash in Updated(). MozReview-Commit-ID: C6OW3jDJ3B4
gfx/layers/client/ClientPaintedLayer.cpp
--- a/gfx/layers/client/ClientPaintedLayer.cpp
+++ b/gfx/layers/client/ClientPaintedLayer.cpp
@@ -176,16 +176,38 @@ ClientPaintedLayer::PaintThebes(nsTArray
 
   mContentClient->EndPaint(aReadbackUpdates);
 
   if (didUpdate) {
     UpdateContentClient(state);
   }
 }
 
+class MOZ_RAII AutoQueuedAsyncPaint
+{
+public:
+  explicit AutoQueuedAsyncPaint(ClientLayerManager* aLayerManager)
+    : mLayerManager(aLayerManager)
+    , mQueuedAsyncPaints(false)
+  { }
+
+  void Queue() { mQueuedAsyncPaints = true; }
+
+  ~AutoQueuedAsyncPaint()
+  {
+    if (mQueuedAsyncPaints) {
+      mLayerManager->SetQueuedAsyncPaints();
+    }
+  }
+
+private:
+  ClientLayerManager* mLayerManager;
+  bool mQueuedAsyncPaints;
+};
+
 /***
  * If we can, let's paint this ClientPaintedLayer's contents off the main thread.
  * The essential idea is that we ask the ContentClient for a DrawTarget and record
  * the moz2d commands. On the Paint Thread, we replay those commands to the
  * destination draw target. There are a couple of lifetime issues here though:
  *
  * 1) TextureClient owns the underlying buffer and DrawTarget. Because of this
  *    we have to keep the TextureClient and DrawTarget alive but trick the
@@ -202,33 +224,31 @@ ClientPaintedLayer::PaintThebes(nsTArray
  *    from trying to paint again. The underlying API however is NOT thread safe.
  *  4) We have both "sync" and "async" OMTP. Sync OMTP means we paint on the main thread
  *     but block the main thread while the paint thread paints. Async OMTP doesn't block
  *     the main thread. Sync OMTP is only meant to be used as a debugging tool.
  */
 void
 ClientPaintedLayer::PaintOffMainThread()
 {
-  uint32_t flags = GetPaintFlags();
+  AutoQueuedAsyncPaint asyncPaints(ClientManager());
 
+  uint32_t flags = GetPaintFlags();
   PaintState state = mContentClient->BeginPaint(this, flags | ContentClient::PAINT_ASYNC);
-  bool didUpdate = false;
 
   if (state.mBufferState) {
     PaintThread::Get()->PrepareBuffer(state.mBufferState);
-    didUpdate = true;
+    asyncPaints.Queue();
   }
 
   if (!UpdatePaintRegion(state)) {
-    if (didUpdate) {
-      ClientManager()->SetQueuedAsyncPaints();
-    }
     return;
   }
 
+  bool didUpdate = false;
   RotatedBuffer::DrawIterator iter;
 
   // Debug Protip: Change to BorrowDrawTargetForPainting if using sync OMTP.
   while (RefPtr<CapturedPaintState> captureState =
           mContentClient->BorrowDrawTargetForRecording(state, &iter))
   {
     DrawTarget* target = captureState->mTargetDual;
     if (!target || !target->IsValid()) {
@@ -260,27 +280,26 @@ ClientPaintedLayer::PaintOffMainThread()
     ctx = nullptr;
 
     captureState->mCapture = captureDT.forget();
     PaintThread::Get()->PaintContents(captureState,
                                       ContentClient::PrepareDrawTargetForPainting);
 
     mContentClient->ReturnDrawTarget(target);
 
+    asyncPaints.Queue();
     didUpdate = true;
   }
 
   PaintThread::Get()->EndLayer();
   mContentClient->EndPaint(nullptr);
 
   if (didUpdate) {
     UpdateContentClient(state);
-    ClientManager()->SetQueuedAsyncPaints();
   }
-  return;
 }
 
 void
 ClientPaintedLayer::RenderLayerWithReadback(ReadbackProcessor *aReadback)
 {
   RenderMaskLayers(this);
 
   if (!EnsureContentClient()) {