Report the duration of sampled tasks with histogram.

components/scheduler/renderer/renderer_task_duration_unittest.cc

Index: components/scheduler/renderer/renderer_task_duration_unittest.cc
diff --git a/components/scheduler/renderer/renderer_task_duration_unittest.cc b/components/scheduler/renderer/renderer_task_duration_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0c7823ed069b234403c50ee6ee19ddc2f125314c
--- /dev/null
+++ b/components/scheduler/renderer/renderer_task_duration_unittest.cc
@@ -0,0 +1,26 @@
+// Copyright 2016 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/scheduler/renderer/renderer_task_duration.h"
+
+#include "base/test/histogram_tester.h"
+#include "base/metrics/histogram_macros.h"

[majidvp, 2016/07/27 13:50:59]

histogram_macros.h seems to be an unnecessary include.

[sunyunjia, 2016/07/27 17:28:43]

Done.

+#include "testing/gmock/include/gmock/gmock.h"

[majidvp, 2016/07/27 13:50:59]

ditto gmock.h

[sunyunjia, 2016/07/27 17:28:43]

Done.

+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace scheduler {
+typedef testing::Test RendererTaskDurationTest;
+TEST_F(RendererTaskDurationTest, LoggingDuration) {

[majidvp, 2016/07/27 13:50:59]

Using a text fixture is not necessary here. You can use a plain old TEST.
This make the above typedef unnecessary as well.

[sunyunjia, 2016/07/27 17:28:43]

Done.

+  base::HistogramTester tester;
+
+  RendererTaskDuration rtd;
+  for (int i = 1; i <= rtd.kSampleInterval + 1; ++i) {

[majidvp, 2016/07/27 13:50:59]

Please spell out 100 in the test rather than using the constant from the code
that is being tested. Otherwise the test will not verify the expected
definition.

[sunyunjia, 2016/07/27 17:28:43]

Done.

+    int num = i % rtd.kSampleInterval;
+    int msecond = 10 * 1000 / rtd.kSampleInterval * num + 1;

[majidvp, 2016/07/27 13:50:59]

s/msceond/milliseconds/

[majidvp, 2016/07/27 13:50:59]

This expression seems a bit too overcomplicated for what you want to achieve. 
Is it trying to make sure different task duration fall into different buckets.


For this test we don't have to have a different interval for each task, 
just two different intervals one for 100th task and one for the rest.
This should also make the assertion simpler to read.

[sunyunjia, 2016/07/27 17:28:43]

Done.

[sunyunjia, 2016/07/27 17:28:43]

Done.

+    base::TimeDelta duration = base::TimeDelta::FromMilliseconds(msecond);
+    rtd.ReportSampleDuration(duration);
+  }
+  tester.ExpectUniqueSample("RendererScheduler.TaskDuration", 1, 1);

[majidvp, 2016/07/27 13:50:59]

Why are we asserting for 1 here? It is hard to correlate that with duration of
100th sample.
See above for a suggestion of a simpler way to do this which should help make
the relation
more clear.

[sunyunjia, 2016/07/27 17:28:43]

Done.

+}
+}