Introduce new constructor to PlatformThread.

webrtc/call/rampup_tests.cc

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 264 matching lines @@
     (*receive_configs)[0].transport_cc = true;
     (*receive_configs)[0].rtp_header_extensions.push_back(RtpExtension(
         extension_type_.c_str(), kTransportSequenceNumberExtensionId));
   }
 }
 
 void RampUpTester::OnCallsCreated(Call* sender_call, Call* receiver_call) {
   sender_call_ = sender_call;
 }
 
-bool RampUpTester::BitrateStatsPollingThread(void* obj) {
-  return static_cast<RampUpTester*>(obj)->PollStats();
+void RampUpTester::BitrateStatsPollingThread(void* obj) {
+  static_cast<RampUpTester*>(obj)->PollStats();
 }
 
-bool RampUpTester::PollStats() {
-  if (sender_call_) {
-    Call::Stats stats = sender_call_->GetStats();
+void RampUpTester::PollStats() {
+  do {
+    if (sender_call_) {
+      Call::Stats stats = sender_call_->GetStats();
 
-    EXPECT_GE(stats.send_bandwidth_bps, start_bitrate_bps_);
-    EXPECT_GE(expected_bitrate_bps_, 0);
-    if (stats.send_bandwidth_bps >= expected_bitrate_bps_ &&
-        (min_run_time_ms_ == -1 ||
-         clock_->TimeInMilliseconds() - test_start_ms_ >= min_run_time_ms_)) {
-      ramp_up_finished_ms_ = clock_->TimeInMilliseconds();
-      observation_complete_.Set();
+      EXPECT_GE(stats.send_bandwidth_bps, start_bitrate_bps_);
+      EXPECT_GE(expected_bitrate_bps_, 0);
+      if (stats.send_bandwidth_bps >= expected_bitrate_bps_ &&
+          (min_run_time_ms_ == -1 ||
+           clock_->TimeInMilliseconds() - test_start_ms_ >= min_run_time_ms_)) {
+        ramp_up_finished_ms_ = clock_->TimeInMilliseconds();
+        observation_complete_.Set();
+      }
     }
-  }
-
-  return !event_.Wait(kPollIntervalMs);
+  } while (!event_.Wait(kPollIntervalMs));

[the sun, 2017/02/22 13:42:57]

It looks like RampUpTester::PerformTest() intends the thread to stop running
when it calls poller_thread_.Stop(), but AFAICT, after this change, that won't
happen until the RampUpTester dtor does event_.Set().

I must be getting it wrong since all tests pass on the try bots? Tell me how!

[tommi, 2017/02/22 15:04:01]

Indeed! Thanks for catching.  It turns out that these tests don't run on the
trybots, only perf bots.

The stop event is currently not being signalled in the right place, so it must
have always been timing out.  Now fixed. Btw, I fixed similar code yesterday, so
maybe this pattern has been copied to more places:
https://codereview.webrtc.org/2706223002/

[the sun, 2017/02/22 16:31:30]

I looked at the other places in your CL, and they appear to do it correctly.
That's the beauty of copy+paste - it will be *almost* the same. :)

 }
 
 void RampUpTester::ReportResult(const std::string& measurement,
                                 size_t value,
                                 const std::string& units) const {
   webrtc::test::PrintResult(
       measurement, "",
       ::testing::UnitTest::GetInstance()->current_test_info()->name(), value,
       units, false);
 }
@@ skipping 94 matching lines @@
       interval_start_ms_(clock_->TimeInMilliseconds()),
       sent_bytes_(0),
       loss_rates_(loss_rates) {
   forward_transport_config_.link_capacity_kbps = link_rates_[test_state_];
   forward_transport_config_.queue_delay_ms = 100;
   forward_transport_config_.loss_percent = loss_rates_[test_state_];
 }
 
 RampUpDownUpTester::~RampUpDownUpTester() {}
 
-bool RampUpDownUpTester::PollStats() {
-  if (send_stream_) {
-    webrtc::VideoSendStream::Stats stats = send_stream_->GetStats();
-    int transmit_bitrate_bps = 0;
-    for (auto it : stats.substreams) {
-      transmit_bitrate_bps += it.second.total_bitrate_bps;
+void RampUpDownUpTester::PollStats() {

[the sun, 2017/02/22 13:42:58]

oh, crikey, PollStats is overloaded...

[tommi, 2017/02/22 15:04:01]

Acknowledged.

+  do {
+    if (send_stream_) {
+      webrtc::VideoSendStream::Stats stats = send_stream_->GetStats();
+      int transmit_bitrate_bps = 0;
+      for (auto it : stats.substreams) {
+        transmit_bitrate_bps += it.second.total_bitrate_bps;
+      }
+      EvolveTestState(transmit_bitrate_bps, stats.suspended);
+    } else if (num_audio_streams_ > 0 && sender_call_ != nullptr) {
+      // An audio send stream doesn't have bitrate stats, so the call send BW is
+      // currently used instead.
+      int transmit_bitrate_bps = sender_call_->GetStats().send_bandwidth_bps;
+      EvolveTestState(transmit_bitrate_bps, false);
     }
-    EvolveTestState(transmit_bitrate_bps, stats.suspended);
-  } else if (num_audio_streams_ > 0 && sender_call_ != nullptr) {
-    // An audio send stream doesn't have bitrate stats, so the call send BW is
-    // currently used instead.
-    int transmit_bitrate_bps = sender_call_->GetStats().send_bandwidth_bps;
-    EvolveTestState(transmit_bitrate_bps, false);
-  }
-
-  return !event_.Wait(kPollIntervalMs);
+  } while (!event_.Wait(kPollIntervalMs));
 }
 
 Call::Config RampUpDownUpTester::GetReceiverCallConfig() {
   Call::Config config(&event_log_);
   config.bitrate_config.min_bitrate_bps = 10000;
   return config;
 }
 
 std::string RampUpDownUpTester::GetModifierString() const {
   std::string str("_");
@@ skipping 195 matching lines @@
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, AudioTransportSequenceNumber) {
   RampUpTester test(0, 1, 0, 300000, 10000,
                     RtpExtension::kTransportSequenceNumberUri, false, false,
                     false);
   RunBaseTest(&test);
 }
 }  // namespace webrtc