Ted, could you take a look at this ASAP, please?

+xjz, who modified the PipelineIntegrationTestBase implementation as per of the media remoting work.

Looking at this test-base, I've found two issues, though I'm not sure if either actually explains these flakes:

1. The PipelineIntegrationTestBase mixes EXPECT_CALL()s in with application logic. Interleaving call-expectations with calls to the mocked methods actually has undefined behaviour.

2. Various tests call WaitUntilEnded(), which appears to be designed to run until either ended or error, and then return True if no error occurred, but is actually not waiting on errors.

#2 looks the likely cause of the hangs; I expect that fixing it will cause the test to fail, in a more helpful way!

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

commit 157985202218231a93bd09f12236d1f3687b851e
Author: Wez <wez@chromium.org>
Date: Thu May 03 17:09:06 2018

Fix PipelineIntegrationTestBase::WaitUntilEnded() to fail on error.

Previously WaitUntilEnded() would not install an on-error closure, and
so would fail to exit on pipeline errors, causing the test to hang
rather than fail.

WaitUntilEnded() now waits for either end or error, and EXPECTs a
PIPELINE_OK status before returning.

Also tweaked the naming of RunUntilIdle* to RunUntilQuit to avoid
confusion with the RunLoop::RunIntilIdle() API behaviour.

Bug:  836785 
Change-Id: Idcf9bbec5ac0d7d78e2ba0530eaed0748a1626c8
Reviewed-on: https://chromium-review.googlesource.com/1041807
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555777}
[modify] https://crrev.com/157985202218231a93bd09f12236d1f3687b851e/media/test/pipeline_integration_test_base.cc
[modify] https://crrev.com/157985202218231a93bd09f12236d1f3687b851e/media/test/pipeline_integration_test_base.h

We are still seeing this test hang, presumably still in the WaitUntilOnEnded(). I can't immediately see how we can manage to reach the RunUntilQuitOrEndedOrError() call in there if |ended_| is already true, so it seems the OnEnded is never dispatched, somehow?

tmathmeyer: Any ideas?

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

commit 11f4ce4066cbd1bda4f00ea56e6d9b41ac27adcb
Author: Michał Pichliński <mpichlinski@opera.com>
Date: Tue May 22 16:32:27 2018

Wait for completion of changing enabled tracks in PipelineIntegrationTests.

Calling Seek/Resume on Pipeline when OnEnabledAudioTracksChanged or
OnSelectedVideoTrackChanged have not finished their job will end with a crash:
- PlaybackWithVideoTrackDisabledThenEnabled crashes on
  DCHECK(!(pending_audio_track_change_ || pending_video_track_change_));
  in RendererImpl::Flush when calling Seek after unfinished
  OnSelectedVideoTrackChanged,
- TrackStatusChangesWhileSuspended crashes on
  return time_source_->CurrentMediaTime();
  in RendererImpl::GetMediaTime() due to nullptr time_source_
  when OnEnabledAudioTracksChanged finishes its job after calling Resume.

In production code PipelineController is responsible for gating operations
via callbacks in order to prevent race conditions.

Sometimes RunUntilIdle may be insufficient for changing track state, instead
of it wait in the RunLoop until completion callback has been called.

Bug:  836785 
Change-Id: I8a784d668b6d92bebd90fafd6e6dbddb18c9ceb4
Reviewed-on: https://chromium-review.googlesource.com/1066015
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560640}
[modify] https://crrev.com/11f4ce4066cbd1bda4f00ea56e6d9b41ac27adcb/media/test/pipeline_integration_test.cc

Ted, we are still seeing the TrackStatusChangesWhileSuspended test hanging pretty frequently under Fuchsia.

Ted, can you prioritize this? Last we talked I indicated there may be an issue where the track change and ended may race and prevent delivery of the ended event.

We should make sure there isn't a lingering issue around ended signaling and track changes before M68 goes to stable or we may end up with some nasty hangs due to background track disable/enable.

Since I'll be OOO soon, +xhwang who is familiar with our pipeline and can help advise investigation.

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

commit c348474f11c937b8c3a19df36c9bb55211facf55
Author: Kevin Marshall <kmarshall@chromium.org>
Date: Thu May 24 18:32:15 2018

Fuchsia: Filter PipelineIntegrationTest.TrackStatusChangesWhileSuspended.

PipelineIntegrationTest.TrackStatusChangesWhileSuspended times out
frequently on the FYI bots, so filter it out for now.

Bug:  836785 
Change-Id: Id209d83a72e16a52cd7cc8b9281bf167a63cf636
Reviewed-on: https://chromium-review.googlesource.com/1068617
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561561}
[modify] https://crrev.com/c348474f11c937b8c3a19df36c9bb55211facf55/testing/buildbot/filters/fuchsia.base_unittests.filter

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

commit ebac442c103f42e897de49c16f6d831c6eae44b7
Author: Wez <wez@chromium.org>
Date: Fri May 25 17:21:00 2018

Revert "Fuchsia: Filter PipelineIntegrationTest.TrackStatusChangesWhileSuspended."

This reverts commit c348474f11c937b8c3a19df36c9bb55211facf55.

Reason for revert: Filter was added to the wrong test suite, so had no effect.

Original change's description:
> Fuchsia: Filter PipelineIntegrationTest.TrackStatusChangesWhileSuspended.
> 
> PipelineIntegrationTest.TrackStatusChangesWhileSuspended times out
> frequently on the FYI bots, so filter it out for now.
> 
> Bug:  836785 
> Change-Id: Id209d83a72e16a52cd7cc8b9281bf167a63cf636
> Reviewed-on: https://chromium-review.googlesource.com/1068617
> Reviewed-by: Wez <wez@chromium.org>
> Commit-Queue: Wez <wez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#561561}

TBR=wez@chromium.org,kmarshall@chromium.org

Change-Id: I057d4db914418cfd2a65a03a827c36a72c9439b0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  836785 
Reviewed-on: https://chromium-review.googlesource.com/1073491
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561928}
[modify] https://crrev.com/ebac442c103f42e897de49c16f6d831c6eae44b7/testing/buildbot/filters/fuchsia.base_unittests.filter

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

commit cb593be96c0552c08d75c43076b569884874ba94
Author: Ted Meyer <tmathmeyer@chromium.org>
Date: Fri May 25 21:03:31 2018

Fix for flaky track change test.

Omitting kNoClockless allowed the renderer to pull frames quickly rather
than trying to ratelimit them at the video fps. This was causing the
video to finish playback in the short timespan between being resumed and
suspended, causing the 'WaitUntilOnEnded' call to incorrectly return
early, as the 'ended_' attribute on the PipelineIntegrationTest had been
set to true. This ended_ boolean is only reset on a seek or a resume,
none of which had come after the suspend. This was flaking because
sometimes the video _wouldn't_ finish before the Suspend call, leaving
the pipeline test in an unended state.

Bug:  836785 
Change-Id: I89d4382354e4cfcd6593b69f6cc02d6821492e41
Reviewed-on: https://chromium-review.googlesource.com/1072955
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562008}
[modify] https://crrev.com/cb593be96c0552c08d75c43076b569884874ba94/media/test/pipeline_integration_test.cc