commit 4d0ea2d75dd716ea6cb5a6e463fe226b03f32be7
author Etienne Bergeron <etienneb@chromium.org> Fri Jan 11 16:28:18 2019
committer Commit Bot <commit-bot@chromium.org> Fri Jan 11 16:28:18 2019
tree b23c0a56b055ea719e26f5180d49febbf9f88315
parent 94e6649fa3d8e7b59aaa74ed0c78b6d79dddaeb2

Fix flaky tests from background tracing manager

This CL is fixing the cause of the flakiness of this unittest:
  BackgroundTracingManagerBrowserTest.NoWhitelistedArgsStripped


We believe the flakiness was caused by the possibility of start/stop tracing twice.
We added a dump of stackframe when the tracing observer call |OnTraceLogEnabled|
and we realized that on linux it's called twice. The stackframes are below.
Calling sites are:
  1) content::BackgroundTracingManagerImpl::SetActiveScenario()
  2) content::BackgroundTracingManagerImpl::OnFinalizeComplete()

The call 1) is the expected one in the unittest since it's the one starting tracing.
The call 2) is caused by the config not being correctly disable with the call to
|DisableScenarioWhenIdle|.

  BackgroundTracingManager::GetInstance()->WhenIdle(
      base::Bind(&DisableScenarioWhenIdle));

A snippet of code can be found below that shows why |config_| needs to be resetted to avoid multiple start/stop tracing.

#2 0x561c34813aee content::(anonymous namespace)::TestTracingHelper::OnTraceLogEnabled()
#3 0x7f9254ddff30 base::trace_event::TraceLog::SetEnabled()
#4 0x7f925057686c content::BackgroundTracingManagerImpl::StartTracing()
#5 0x7f9250574b1e content::BackgroundTracingManagerImpl::StartTracingIfConfigNeedsIt()
#6 0x7f9250574752 content::BackgroundTracingManagerImpl::SetActiveScenario()
#7 0x561c347f221a content::BackgroundTracingManagerBrowserTest_NoWhitelistedArgsStripped_Test::RunTestOnMainThread()
#8 0x561c35d391dd content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#9 0x561c33f88f0d _ZN4base8internal13FunctorTraitsIMN7content41GenerateMHTMLAndExitRendererMessageFilterEFvvEvE6InvokeIS5_PS3_JEEEvT_OT0_DpOT1_
#10 0x561c33f88e84 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMN7content41GenerateMHTMLAndExitRendererMessageFilterEFvvEJPS5_EEEvOT_DpOT0_
#11 0x561c33f88e35 _ZN4base8internal7InvokerINS0_9BindStateIMN7content41GenerateMHTMLAndExitRendererMessageFilterEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIS6_NSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSD_16integer_sequenceImJXspT1_EEEE
#12 0x561c34164f9c _ZN4base8internal7InvokerINS0_9BindStateIMNS_12MockCallbackINS_17RepeatingCallbackIFvvEEEEEFvvEJNS0_17UnretainedWrapperIS7_EEEEES5_E3RunEPNS0_13BindStateBaseE
#13 0x561c33e2827d _ZNKR4base17RepeatingCallbackIFvvEE3RunEv
#14 0x561c35fb418d content::ShellBrowserMainParts::PreMainMessageLoopRun()
#15 0x7f924f65fb32 content::BrowserMainLoop::PreMainMessageLoopRun()




#2 0x561c34813aee content::(anonymous namespace)::TestTracingHelper::OnTraceLogEnabled()
#3 0x7f9254ddff30 base::trace_event::TraceLog::SetEnabled()
#4 0x7f925057686c content::BackgroundTracingManagerImpl::StartTracing()
#5 0x7f9250574b1e content::BackgroundTracingManagerImpl::StartTracingIfConfigNeedsIt()
#6 0x7f9250579062 content::BackgroundTracingManagerImpl::OnFinalizeComplete()
#7 0x7f924f17f715 _ZN4base8internal13FunctorTraitsIMN4mojo8internal24CallbackWithDeleteHelperIFvbEEEFvbEvE6InvokeIS8_PS6_JbEEEvT_OT0_DpOT1_
#8 0x7f924f17f66f _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMN4mojo8internal24CallbackWithDeleteHelperIFvbEEEFvbEJPS8_bEEEvOT_DpOT0_





void BackgroundTracingManagerImpl::OnFinalizeComplete(bool success) {
  [...]

  if (!idle_callback_.is_null())
    idle_callback_.Run();

  bool is_allowed_begin =
      !delegate_ || (config_ &&
                     delegate_->IsAllowedToBeginBackgroundScenario(
                         *config_.get(), requires_anonymized_data_))

  [...]
  // NOTE: config_ must be empty to avoid starting tracing

  if (is_allowed_begin) {
    StartTracingIfConfigNeedsIt();
  } else {
    AbortScenario();
  }


B=639706,795803
R=oysteine@chromium.org

Change-Id: I5495b542615633c62a3451a75a668866b18592b0
Reviewed-on: https://chromium-review.googlesource.com/c/1341134
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622024}