Fix flakiness in PushPullFIFOSmokeTest.SmokeTests/7

third_party/WebKit/Source/platform/audio/PushPullFIFOMultithreadTest.cpp

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "platform/audio/PushPullFIFO.h"
 
 #include <memory>
 #include <vector>
 #include "platform/CrossThreadFunctional.h"
 #include "platform/WaitableEvent.h"
@@ skipping 29 matching lines @@
 }
 
 // Base FIFOClient with an extra thread for looping and jitter control. The
 // child class must define a specific task to run on the thread.
 class FIFOClient {
  public:
   FIFOClient(PushPullFIFO* fifo, size_t bus_length, size_t jitter_range_ms)
       : fifo_(fifo),
         bus_(AudioBus::Create(fifo->NumberOfChannels(), bus_length)),
         client_thread_(Platform::Current()->CreateThread("client thread")),
+        done_event_(WTF::MakeUnique<WaitableEvent>()),
         jitter_range_ms_(jitter_range_ms) {}
 
-  void Start(double duration_ms, double interval_ms) {
+  ~FIFOClient() {
+    client_thread_.reset();

[o1ka, 2017/05/08 12:11:56]

Why is it needed?

[hongchan, 2017/05/08 18:19:33]

Removed.

+  }
+
+  WaitableEvent* Start(double duration_ms, double interval_ms) {
     duration_ms_ = duration_ms;
     interval_ms_ = interval_ms;
     client_thread_->GetWebTaskRunner()->PostTask(
         BLINK_FROM_HERE,
         CrossThreadBind(&FIFOClient::RunTaskOnOwnThread,
                         CrossThreadUnretained(this)));
+    return done_event_.get();
   }
 
   virtual void Stop(int callback_counter) = 0;
   virtual void RunTask() = 0;
 
   void Pull(size_t frames_to_pull) {
     fifo_->Pull(bus_.Get(), frames_to_pull);
   }
 
   void Push() {
     fifo_->Push(bus_.Get());
   }
 
  private:
   void RunTaskOnOwnThread() {
     double interval_with_jitter = interval_ms_
         + (static_cast<double>(std::rand()) / RAND_MAX) * jitter_range_ms_;
     elapsed_ms_ += interval_with_jitter;
     ++counter_;
     RunTask();
     if (elapsed_ms_ < duration_ms_) {
       client_thread_->GetWebTaskRunner()->PostDelayedTask(
               BLINK_FROM_HERE,
               CrossThreadBind(&FIFOClient::RunTaskOnOwnThread,
                               CrossThreadUnretained(this)),
               interval_with_jitter);
     } else {
       Stop(counter_);
+      done_event_->Signal();
     }
   }
 
   PushPullFIFO* fifo_;
   RefPtr<AudioBus> bus_;
   std::unique_ptr<WebThread> client_thread_;
+  std::unique_ptr<WaitableEvent> done_event_;
 
   // Test duration.
   double duration_ms_;
 
   // Interval between each callback.
   double interval_ms_;
 
   // Jitter added to the regular pushing/pulling interval.
   // (where j is 0 < j < jitter_range_ms)
   double jitter_range_ms_;
@@ skipping 69 matching lines @@
   const double push_interval_ms =
       param.push_buffer_size / sample_rate * 1000;
 
   std::unique_ptr<PushPullFIFO> test_fifo = WTF::WrapUnique(
       new PushPullFIFO(param.number_of_channels, param.fifo_length));
   std::unique_ptr<PullClient> pull_client = WTF::WrapUnique(new PullClient(
       test_fifo.get(), param.pull_buffer_size, param.pull_jitter_range_ms));
   std::unique_ptr<PushClient> push_client = WTF::WrapUnique(new PushClient(
       test_fifo.get(), param.push_buffer_size, param.push_jitter_range_ms));
 
-  LOG(INFO) << "PushPullFIFOSmokeTest - Start";
+  Vector<WaitableEvent*> done_events;
+  done_events.push_back(
+      pull_client->Start(param.test_duration_ms, pull_interval_ms));
+  done_events.push_back(
+      push_client->Start(param.test_duration_ms, push_interval_ms));
 
-  pull_client->Start(param.test_duration_ms, pull_interval_ms);
-  push_client->Start(param.test_duration_ms, push_interval_ms);
+  LOG(INFO) << "PushPullFIFOSmokeTest - Started";
+
+  // We have to wait both of events to be signaled.
+  WaitableEvent::WaitMultiple(done_events);
+  WaitableEvent::WaitMultiple(done_events);
 
   // If the operation does not cause a crash for the test period, it's passed.
-  // Also give a bit more time to finish the tear-down process.
-  HoldTestForDuration(param.test_duration_ms + 150);
+  // Also give a bit more time (10ms per iteration) to finish the tear-down

[o1ka, 2017/05/08 12:11:56]

Not sure what tear down process do you mean?
Nothing will run  after both |done_event_|s are signaled, right?

[hongchan, 2017/05/08 18:19:33]

Done.

+  // process.
+  HoldTestForDuration(10);
 }
 
 FIFOSmokeTestParam smoke_test_params[] = {
   // Test case 0 (OSX): 256 Pull, 128 Push, Minimal jitter.
   // WebThread's priority is lower than the device thread, so its jitter range
   // is slightly bigger than the other.
   {48000, 2, 8192, 1000, 256, 1, 128, 2},
 
   // Test case 1 (Windows): 441 Pull, 128 Push. Moderate Jitter.
   // Windows' audio callback is known to be ~10ms and UMA data shows the
@@ skipping 25 matching lines @@
   {48000, 2, 8192, 10000, 256, 0, 128, 1}
 };
 
 INSTANTIATE_TEST_CASE_P(PushPullFIFOSmokeTest,
                         PushPullFIFOSmokeTest,
                         ::testing::ValuesIn(smoke_test_params));
 
 }  // namespace
 
 }  // namespace blink