Prevent DCHECKs if MessageLoop is destroyed during TextureLayer shutdown

If the compositor thread has released the resource in a TextureLayer
and posted that release to the main thread, but then the MessageLoop
on which the posted task sits shuts down, the TextureLayer's
TransferableResourceHolder will be destroyed. This is because the
holder is at that time singly owned by callback, as the resource is
being released due to being removed (and unowned) by the main thread
side.

In this case, there is a posted-but-not-run dereference sitting in
the MessageLoop which we can track, and avoid DCHECK-failing in the
TransferableResourceHolder destructor.

However, there is also the ReleaseCallback for the main thread in
the TransferableResourceHolder that will DCHECK if not run, so we
can run that SingleReleaseCallback from the TransferableResourceHolder
destructor. This is done on the assumption that the MessageLoop is
destroyed from the main thread (we can DCHECK this instead), and
that the only case we have a SingleReleaseCallback in the destructor
still is when all missing derefs have been posted but not run (we
can DCHECK this as well).

This fixes the flaky SoftwareTextureLayerMultipleRegisterTest,
which could be resolved by using DidReceiveCompositorFrameAck()
instead of DidCommitAndDrawFrame(), as that waits for the release
callback to be run. But also resolves production issues where
DCHECKs are hit during shutdown due to the MessageLoop being
destroyed in this scenario (809604).

R=piman@chromium.org

Bug: 834613, 809604
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I3faa1cdcc58f1be99cf5f80732c63e51e9aff3d9
Reviewed-on: https://chromium-review.googlesource.com/1019614
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552151}

cc/layers/texture_layer.cc

@@ -273,20 +273,40 @@ TextureLayer::TransferableResourceHolder::MainThreadReference::
 
 TextureLayer::TransferableResourceHolder::MainThreadReference::
     ~MainThreadReference() {
+#if DCHECK_IS_ON()
+  {
+    base::AutoLock hold(holder_->posted_internal_derefs_lock_);
+    ++holder_->posted_internal_derefs_;
+  }
+#endif
   holder_->InternalRelease();
 }
 
 TextureLayer::TransferableResourceHolder::TransferableResourceHolder(
     const viz::TransferableResource& resource,
     std::unique_ptr<viz::SingleReleaseCallback> release_callback)
-    : internal_references_(0),
-      resource_(resource),
+    : resource_(resource),
       release_callback_(std::move(release_callback)),
-      sync_token_(resource.mailbox_holder.sync_token),
-      is_lost_(false) {}
+      sync_token_(resource.mailbox_holder.sync_token) {}
 
 TextureLayer::TransferableResourceHolder::~TransferableResourceHolder() {
-  DCHECK_EQ(0u, internal_references_);
+#if DCHECK_IS_ON()
+  {
+    // If the MessageLoop is destroyed while a posted deref is waiting to run,
+    // this object will be destroyed with an internal_references_ still present.
+    // So we must also include the outstanding posted derefences.
+    base::AutoLock hold(posted_internal_derefs_lock_);
+    DCHECK_EQ(internal_references_, posted_internal_derefs_);
+  }
+#endif
+  if (release_callback_) {
+    // We land here if the dereferences are posted but not run and the
+    // MessageLoop is destroyed, destroying those tasks and this object with it.
+    // We run the ReleaseCallback in that case assuming the MessageLoop is being
+    // destroyed on the main thread.
+    DCHECK(main_thread_checker_.CalledOnValidThread());
+    release_callback_->Run(sync_token_, is_lost_);
+  }
 }
 
 std::unique_ptr<TextureLayer::TransferableResourceHolder::MainThreadReference>
@@ -310,7 +330,7 @@ TextureLayer::TransferableResourceHolder::GetCallbackForImplThread(
     scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner) {
   // We can't call GetCallbackForImplThread if we released the main thread
   // reference.
-  DCHECK_GT(internal_references_, 0u);
+  DCHECK_GT(internal_references_, 0);
   InternalAddRef();
   return viz::SingleReleaseCallback::Create(
       base::Bind(&TransferableResourceHolder::ReturnAndReleaseOnImplThread,
@@ -323,6 +343,12 @@ void TextureLayer::TransferableResourceHolder::InternalAddRef() {
 
 void TextureLayer::TransferableResourceHolder::InternalRelease() {
   DCHECK(main_thread_checker_.CalledOnValidThread());
+#if DCHECK_IS_ON()
+  {
+    base::AutoLock hold(posted_internal_derefs_lock_);
+    --posted_internal_derefs_;
+  }
+#endif
   if (!--internal_references_) {
     release_callback_->Run(sync_token_, is_lost_);
     resource_ = viz::TransferableResource();
@@ -335,6 +361,12 @@ void TextureLayer::TransferableResourceHolder::ReturnAndReleaseOnImplThread(
     const gpu::SyncToken& sync_token,
     bool is_lost) {
   Return(sync_token, is_lost);
+#if DCHECK_IS_ON()
+  {
+    base::AutoLock hold(posted_internal_derefs_lock_);
+    ++posted_internal_derefs_;
+  }
+#endif
   main_thread_task_runner->PostTask(
       FROM_HERE,
       base::Bind(&TransferableResourceHolder::InternalRelease, this));

cc/layers/texture_layer.h

@@ -84,7 +84,13 @@ class CC_EXPORT TextureLayer : public Layer, SharedBitmapIdRegistrar {
 
     // These members are only accessed on the main thread, or on the impl thread
     // during commit where the main thread is blocked.
-    unsigned internal_references_;
+    int internal_references_ = 0;
+#if DCHECK_IS_ON()
+    // The number of derefs posted from the impl thread, and a lock for
+    // accessing it.
+    base::Lock posted_internal_derefs_lock_;
+    int posted_internal_derefs_ = 0;
+#endif
     viz::TransferableResource resource_;
     std::unique_ptr<viz::SingleReleaseCallback> release_callback_;
 
@@ -94,7 +100,7 @@ class CC_EXPORT TextureLayer : public Layer, SharedBitmapIdRegistrar {
     // ReturnAndReleaseOnImplThread() defines their values.
     base::Lock arguments_lock_;
     gpu::SyncToken sync_token_;
-    bool is_lost_;
+    bool is_lost_ = false;
     base::ThreadChecker main_thread_checker_;
     DISALLOW_COPY_AND_ASSIGN(TransferableResourceHolder);
   };