I think it means a texture was set on a TextureLayer and not removed before destroying it. All the TextureLayer unit tests for example remove the texture from the layer before exiting to avoid DCHECKs.

Printing a stack trace in TextureLayer::TransferableResourceHolder::InternalAddRef() doesn't really point to the cause; what's the best way to pinpoint the errant texture?

#0 0x7f0681e5df5c base::debug::StackTrace::StackTrace()
#1 0x7f067e4068c1 cc::TextureLayer::TransferableResourceHolder::MainThreadReference::MainThreadReference()
#2 0x7f067e40613f cc::TextureLayer::SetTransferableResourceInternal()
#3 0x7f067e406380 cc::TextureLayer::Update()
#4 0x7f067e4929da cc::LayerTreeHost::PaintContent()
#5 0x7f067e491d47 cc::LayerTreeHost::DoUpdateLayers()
#6 0x7f067e491432 cc::LayerTreeHost::UpdateLayers()
#7 0x7f067e4e5ec8 cc::SingleThreadProxy::BeginMainFrame()
#8 0x7f067e4e6aa0 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc17SingleThreadProxyEFvRKN3viz14BeginFrameArgsEEJNS_7WeakPtrIS4_EES6_EEEFvvEE7RunOnceEPNS0_13BindStateBaseE
#9 0x7f0681e5e835 base::debug::TaskAnnotator::RunTask()
#10 0x7f0681e90029 base::internal::IncomingTaskQueue::RunTask()
#11 0x7f0681e93ccb base::MessageLoop::RunTask()
#12 0x7f0681e94063 base::MessageLoop::DeferOrRunPendingTask()
#13 0x7f0681e942f6 base::MessageLoop::DoWork()
#14 0x7f0681e96829 base::MessagePumpLibevent::Run()
#15 0x7f0681e935c9 base::MessageLoop::Run()
#16 0x7f0681eca579 base::RunLoop::Run()
#17 0x563bdbc7821a ChromeBrowserMainParts::MainMessageLoopRun()
#18 0x7f067f0e14e7 content::BrowserMainLoop::RunMainMessageLoopParts()
#19 0x7f067f0e4526 content::BrowserMainRunnerImpl::Run()
#20 0x7f067f0dd55a content::BrowserMain()
#21 0x7f067fabe302 content::ContentMainRunnerImpl::Run()
#22 0x7f06823a100b service_manager::Main()
#23 0x7f067fabc8e4 content::ContentMain()
#24 0x563bdb17dbfd ChromeMain
#25 0x7f0674f7e2b1 __libc_start_main
#26 0x563bdb17d8ca _start

The TextureLayer::Update() goes to a client that is setting the texture.

Hmm, I landed a CL recently that changed the destruction order of Reflector relative to the mirroring window tree hosts: https://chromium-review.googlesource.com/c/chromium/src/+/892054

That leaves mirroring layers in the reflector during shutdown, but we hit the same DCHECK before that CL and with a WIP workaround fix: https://chromium-review.googlesource.com/c/chromium/src/+/905423

Can you help me figure out what I'm missing? Is there something else we need to do with ReflectorImpl::mirrored_compositor_, mailbox_, or output_surface_? I'm really not familiar with compositor usage patterns.

Some more thinking about it..

[239839:239839:0206/101743.505154:FATAL:texture_layer.cc(236)] Check failed: 0u == internal_references_ (0 vs. 1)
#0 0x7fc0c4a90f5c base::debug::StackTrace::StackTrace()
#1 0x7fc0c4ab81ac logging::LogMessage::~LogMessage()
#2 0x7fc0c1039a56 cc::TextureLayer::TransferableResourceHolder::~TransferableResourceHolder()
#3 0x7fc0c1039ace cc::TextureLayer::TransferableResourceHolder::~TransferableResourceHolder()
#4 0x7fc0c103a287 _ZN4base8internal9BindStateIMN2cc12TextureLayer26TransferableResourceHolderEFvvEJ13scoped_refptrIS4_EEE7DestroyEPKNS0_13BindStateBaseE

This is the TransferableResourceHolder::InternalRelease() posted from ReturnAndReleaseOnImplThread()? That is bound as the callback passed to the ResourceProvider in the compositor.

The InternalRelease callback owns a ref on the TransferableResourceHolder, and it looks like it was the last reference on the TransferalbeResourceHolder, but not the last reference on internal_references_.

#5 0x7fc0c4ac356e base::internal::IncomingTaskQueue::TriageQueue::Clear()
#6 0x7fc0c4ac555f base::MessageLoop::DeletePendingTasks()
#7 0x7fc0c4ac7878 base::MessageLoop::~MessageLoop()

This looks like the MessageLoop is shutting down while the posted TransferableResourceHolder::InternalRelease() call is still waiting to run. So it never runs InternalRelease() and just destroys the callback. Then that destroys the TransferableResourceHolder without decrementing the internal_references_.

Maybe if main_thread_task_runner->PostTask() fails, it needs to handle that.

I'm sorry, but I doubt I could offer any useful thoughts here. Is it reasonable to allow TransferableResourceHolder destruction without all the internal references being released? AFAIK, we shouldn't add latency to shutdown (so I wouldn't suggest trying to block MessageLoop shutdown for this task to run).

who can we assign this bug to?

danakj@ would you be able to work on this?

Sorry I don't have time to prioritize this.

> Is it reasonable to allow TransferableResourceHolder destruction without all the internal references being released? 

It seems reasonable when the message loop is destroyed, but otherwise it points at a bug.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0841105055f52f0efbfb330b6e5970a859d916c1

commit 0841105055f52f0efbfb330b6e5970a859d916c1
Author: danakj <danakj@chromium.org>
Date: Thu Apr 19 20:52:24 2018

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}
[modify] https://crrev.com/0841105055f52f0efbfb330b6e5970a859d916c1/cc/layers/texture_layer.cc
[modify] https://crrev.com/0841105055f52f0efbfb330b6e5970a859d916c1/cc/layers/texture_layer.h

This should no longer happen