Chrome Version       : 71.0.3559.6

context: https://chromium-review.googlesource.com/c/chromium/src/+/1242568/13/base/observer_list.h#320

It's unclear whether or not this constitutes a threading error ("innocent" or otherwise). The use-case is complex.

That context link explores adding a sequence checker to base::ObserverList that:
 - binds to a sequence when an ObserverList is first iterated over
 - DCHECKs if the ObserverList changes, or is iterated over, in another sequence

The principal failure seems to code in cc::ElementAnimations. To get past it, we are detaching from the sequence whenever iteration is complete. That's going to make error-detection a lot more reliant on an *actual* race condition occurring. However, it's a closer match to the sequence guarantees currently in place via logic in WeakPtr.

Specifically, cc::LayerTreeHost::FinishCommitOnImplThread() (a thread) can invoke cc::ElementAnimations::ScrollOffsetAnimationWasInterrupted() - it doesn't actually *notify* observers, but reads a bool on KeyFrameEffect, an object which (appears to?) "live" on another thread.

But a KeyFrameEffect actually appears to have two instances - one for each thread - synchronized in  KeyframeEffect::PushPropertiesTo(). I'm not yet certain "which" instance actually lives in the ObserverList...


What steps will reproduce the problem?
1. Comment out `DETACH_FROM_SEQUENCE(iteration_sequence_checker_)` in ObserverList::Compact()
2. Run tryjobs

What is the expected result?

Pass.


What happens instead of that?

Lots of purple: --> https://chromium-review.googlesource.com/c/chromium/src/+/1242568/14

A few bots that don't rely heavily on compositing get past purple, and into red. Errors like:


https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8934174853111035792/+/steps/headless_browsertests__with_patch_/0/logs/BlockWebContentsOpenTest.RunAsyncTest/0

* BlockWebContentsOpenTest.RunAsyncTest
* HeadlessProtocolBrowserTest.VirtualTimeStarvation

  [0927/201019.888479:FATAL:observer_list_internal.h(139)] Check failed: (list_->iteration_sequence_checker_).CalledOnValidSequence().
  #0 0x0000052da95c base::debug::StackTrace::StackTrace()
  #1 0x000005233e0b logging::LogMessage::~LogMessage()
  #2 0x000003502da0 base::ObserverList<>::Iter::Iter()
  #3 0x00000674d35e cc::ElementAnimations::ScrollOffsetAnimationWasInterrupted()
  #4 0x000006745f1b cc::AnimationHost::ScrollOffsetAnimationWasInterrupted()
  #5 0x000006481f68 cc::Layer::PushPropertiesTo()
  #6 0x00000648317a cc::PictureLayer::PushPropertiesTo()
  #7 0x0000064b94c1 cc::TreeSynchronizer::PushLayerProperties()
  #8 0x0000063f4292 cc::LayerTreeHost::FinishCommitOnImplThread()
  #9 0x0000064a3b99 cc::ProxyImpl::ScheduledActionCommit()
  #10 0x0000064aa488 cc::Scheduler::ProcessScheduledActions()
  #11 0x0000064aae9a cc::Scheduler::NotifyReadyToCommit()
  #12 0x0000064a0920 cc::ProxyImpl::NotifyReadyToCommitOnImpl()
  #13 0x00000523d9fd base::debug::TaskAnnotator::RunTask()
  #14 0x000005287e70 base::sequence_manager::internal::ThreadControllerImpl::DoWork()
  #15 0x0000031bc698 _ZN4base8internal7InvokerINS0_9BindStateIMN5media26AudioDebugRecordingManagerEFvjEJNS_7WeakPtrIS4_EEjEEEFvvEE7RunOnceEPNS0_13BindStateBaseE
  #16 0x00000523d9fd base::debug::TaskAnnotator::RunTask()
  #17 0x00000523c99e base::MessageLoop::RunTask()
  #18 0x00000523cda2 base::MessageLoop::DoWork()
  #19 0x000005240526 base::MessagePumpDefault::Run()
  #20 0x00000523c471 base::MessageLoop::Run()
  #21 0x00000525fe36 base::RunLoop::Run()
  #22 0x0000052ac08a base::Thread::Run()
  #23 0x0000052ac405 base::Thread::ThreadMain()
  #24 0x0000052eed1f base::(anonymous namespace)::ThreadFunc()
  #25 0x7f889632e184 start_thread
  #26 0x7f8894d6003d clone


Basically, any test that runs background threads to completion and does any compositing seems to hit this.