Fix flakiness in MultithreadedMetrics test for single sample metrics.

components/metrics/single_sample_metrics_factory_impl_unittest.cc

 // 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 "components/metrics/single_sample_metrics_factory_impl.h"
 
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/test/gtest_util.h"
 #include "base/test/histogram_tester.h"
@@ skipping 16 matching lines @@
   SingleSampleMetricsFactoryImplTest() : thread_("TestThread") {
     InitializeSingleSampleMetricsFactory(
         base::BindRepeating(&SingleSampleMetricsFactoryImplTest::CreateProvider,
                             base::Unretained(this)));
     factory_ = static_cast<SingleSampleMetricsFactoryImpl*>(
         base::SingleSampleMetricsFactory::Get());
   }
 
   ~SingleSampleMetricsFactoryImplTest() override {
     factory_->DestroyProviderForTesting();
-    if (thread_.IsRunning()) {
-      thread_.task_runner()->PostTask(
-          FROM_HERE,
-          base::Bind(&SingleSampleMetricsFactoryImpl::DestroyProviderForTesting,
-                     base::Unretained(factory_)));
-      thread_.Stop();
-    }
+    if (thread_.IsRunning())
+      ShutdownThread();
     base::SingleSampleMetricsFactory::DeleteFactoryForTesting();
   }
 
  protected:
   void StartThread() { ASSERT_TRUE(thread_.Start()); }
 
+  void ShutdownThread() {
+    thread_.task_runner()->PostTask(
+        FROM_HERE,
+        base::Bind(&SingleSampleMetricsFactoryImpl::DestroyProviderForTesting,
+                   base::Unretained(factory_)));
+    thread_.Stop();

[Ilya Sherman, 2017/05/19 00:12:41]

I'm honestly a bit confused: I'd expect PostTask() followed immediately by
Stop() to lose the posted task.  Is there not any need to wait for the task to
execute?

[DaleCurtis, 2017/05/19 00:13:29]

Thread::Stop() joins, so all posted tasks are waited for.

[Ilya Sherman, 2017/05/19 00:18:55]

Ah, great! =)  LGTM in that case -- thanks!

+  }
+
   void CreateProvider(mojom::SingleSampleMetricsProviderRequest request) {
     CreateSingleSampleMetricsProvider(service_manager::BindSourceInfo(),
                                       std::move(request));
     provider_count_++;
   }
 
   std::unique_ptr<base::SingleSampleMetric> CreateMetricOnThread() {
     std::unique_ptr<base::SingleSampleMetric> metric;
     base::RunLoop run_loop;
     thread_.task_runner()->PostTaskAndReply(
@@ skipping 64 matching lines @@
   // Verify construction implicitly by requesting a histogram with the same
   // parameters; this test relies on the fact that histogram objects are unique
   // per name. Different parameters will result in a nullptr being returned.
   EXPECT_FALSE(base::Histogram::FactoryGet(kMetricName, 1, 3, 3,
                                            base::HistogramBase::kNoFlags));
   EXPECT_TRUE(base::Histogram::FactoryGet(
       kMetricName, kMin, kMax, kBucketCount,
       base::HistogramBase::kUmaTargetedHistogramFlag));
 }
 
-// Flaky on Android N5X builders. https://crbug.com/719497
-#if defined(OS_ANDROID)
-#define MAYBE_MultithreadedMetrics DISABLED_MultithreadedMetrics
-#else
-#define MAYBE_MultithreadedMetrics MultithreadedMetrics
-#endif
-TEST_F(SingleSampleMetricsFactoryImplTest, MAYBE_MultithreadedMetrics) {
+TEST_F(SingleSampleMetricsFactoryImplTest, MultithreadedMetrics) {
   base::HistogramTester tester;
   std::unique_ptr<base::SingleSampleMetric> metric =
       factory_->CreateCustomCountsMetric(kMetricName, kMin, kMax, kBucketCount);
   EXPECT_EQ(1u, provider_count_);
 
   StartThread();
 
   std::unique_ptr<base::SingleSampleMetric> threaded_metric =
       CreateMetricOnThread();
   ASSERT_TRUE(threaded_metric);
@@ skipping 13 matching lines @@
 
     base::RunLoop run_loop;
     thread_.task_runner()->PostTaskAndReply(
         FROM_HERE,
         base::Bind(&base::SingleSampleMetric::SetSample,
                    base::Unretained(threaded_metric.get()), kSample),
         run_loop.QuitClosure());
     run_loop.Run();
   }
 
-  // Release metrics and cycle threads to ensure destruction completes.
-  {
-    thread_.task_runner()->DeleteSoon(FROM_HERE, threaded_metric.release());
-
-    base::RunLoop run_loop;
-    thread_.task_runner()->PostTaskAndReply(
-        FROM_HERE, base::Bind(&base::DoNothing), run_loop.QuitClosure());
-    run_loop.Run();
-  }
+  // Release metrics and shutdown thread to ensure destruction completes.
+  thread_.task_runner()->DeleteSoon(FROM_HERE, threaded_metric.release());
+  ShutdownThread();
 
   metric.reset();
   base::RunLoop().RunUntilIdle();
+
   tester.ExpectUniqueSample(kMetricName, kSample, 2);
 }
 
 }  // namespace metrics