Introduce new constructor to PlatformThread.

webrtc/call/rampup_tests.cc

Index: webrtc/call/rampup_tests.cc
diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc
index 94153b08ae62287978313b3cfcef526f6037e017..05868fdd0d0418eee2e8ed396013d9528f205701 100644
--- a/webrtc/call/rampup_tests.cc
+++ b/webrtc/call/rampup_tests.cc
@@ -282,25 +282,25 @@ void RampUpTester::OnCallsCreated(Call* sender_call, Call* receiver_call) {
   sender_call_ = sender_call;
 }
 
-bool RampUpTester::BitrateStatsPollingThread(void* obj) {
-  return static_cast<RampUpTester*>(obj)->PollStats();
-}
-
-bool RampUpTester::PollStats() {
-  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();
+void RampUpTester::BitrateStatsPollingThread(void* obj) {
+  static_cast<RampUpTester*>(obj)->PollStats();
+}
+
+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();
+      }
     }
-  }
-
-  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,
@@ -415,22 +415,22 @@ RampUpDownUpTester::RampUpDownUpTester(size_t num_video_streams,
 
 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() {