Add TimeToScrollUpdateSwapBegin2 to UKM

content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc

Index: content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
diff --git a/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc b/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
index 7655c4c9523a2ea4fa1022ccc0b7cc7439cf29c9..58e35f1c570f3797275e0f8fdc585f976588214b 100644
--- a/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
+++ b/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
@@ -2,10 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "content/browser/renderer_host/input/render_widget_host_latency_tracker.h"
+#include "base/metrics/metrics_hashes.h"
 #include "base/test/histogram_tester.h"
+#include "components/metrics/proto/ukm/entry.pb.h"
 #include "components/rappor/public/rappor_utils.h"
 #include "components/rappor/test_rappor_service.h"
-#include "content/browser/renderer_host/input/render_widget_host_latency_tracker.h"
+#include "components/ukm/test_ukm_recorder.h"
+#include "components/ukm/ukm_source.h"
 #include "content/common/input/synthetic_web_input_event_builders.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "content/test/test_content_browser_client.h"
@@ -21,6 +25,8 @@ using testing::ElementsAre;
 namespace content {
 namespace {
 
+const char kUrl[] = "http://www.foo.bar.com/subpage/1";
+
 void AddFakeComponentsWithTimeStamp(
     const RenderWidgetHostLatencyTracker& tracker,
     ui::LatencyInfo* latency,
@@ -77,8 +83,13 @@ class RenderWidgetHostLatencyTrackerTestBrowserClient
     return &rappor_service_;
   }
 
+  ukm::UkmRecorder* GetUkmRecorder() override { return &test_ukm_recorder_; }
+
+  ukm::TestUkmRecorder* GetTestUkmRecorder() { return &test_ukm_recorder_; }
+
  private:
   rappor::TestRapporServiceImpl rappor_service_;
+  ukm::TestUkmRecorder test_ukm_recorder_;
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostLatencyTrackerTestBrowserClient);
 };
 
@@ -133,6 +144,43 @@ class RenderWidgetHostLatencyTrackerTest
     }
   }
 
+  ::testing::AssertionResult UkmReportAssert(const char* event_name) {
+    const ukm::TestUkmRecorder* ukm_recoder =
+        test_browser_client_.GetTestUkmRecorder();
+    size_t expected_metrics_index = 0;

[tdresser, 2017/05/30 16:52:07]

It isn't clear from the name what this is.

[Navid Zolghadr, 2017/05/31 20:37:15]

renamed.

+    for (size_t i = 0; i < ukm_recoder->entries_count(); ++i) {

[tdresser, 2017/05/30 16:52:07]

Could this be a range based for?

[Navid Zolghadr, 2017/05/31 20:37:15]

It seems that ukm_recorder is not (and does not return) an iterable object.

[tdresser, 2017/06/01 12:44:44]

Acknowledged.

+      const ukm::mojom::UkmEntry* entry = ukm_recoder->GetEntry(i);
+      if (entry->event_hash != base::HashMetricName(event_name))
+        continue;
+
+      const ukm::UkmSource* source =
+          ukm_recoder->GetSourceForSourceId(entry->source_id);
+      if (!source)
+        return ::testing::AssertionFailure() << "Source should not be null";
+
+      if (expected_metrics_index >= 1)
+        break;
+
+      if (GURL(kUrl) != source->url())
+        return ::testing::AssertionFailure() << "Incorrect URL is reported.";
+
+      expected_metrics_index++;
+      if (entry->metrics.size() != 1)
+        return ::testing::AssertionFailure()
+               << event_name << " entry expected only 1 metric but got "
+               << entry->metrics.size();
+
+      if (entry->metrics[0]->metric_hash != base::HashMetricName("Latency"))
+        return ::testing::AssertionFailure()
+               << "Expected a metric named 'Latency'";
+    }
+    if (expected_metrics_index != 1)
+      return ::testing::AssertionFailure()
+             << event_name << " expected " << 1 << " entries but got "
+             << expected_metrics_index;
+    return ::testing::AssertionSuccess();
+  }
+
   ::testing::AssertionResult HistogramSizeEq(const char* histogram_name,
                                              int size) {
     uint64_t histogram_size =
@@ -149,6 +197,7 @@ class RenderWidgetHostLatencyTrackerTest
   RenderWidgetHostLatencyTracker* tracker() { return &tracker_; }
   void ResetHistograms() {
     histogram_tester_.reset(new base::HistogramTester());
+    test_browser_client_.GetTestUkmRecorder()->Purge();
   }
 
   const base::HistogramTester& histogram_tester() {
@@ -177,7 +226,7 @@ class RenderWidgetHostLatencyTrackerTest
 };
 
 TEST_F(RenderWidgetHostLatencyTrackerTest, TestWheelToFirstScrollHistograms) {
-  const GURL url("http://www.foo.bar.com/subpage/1");
+  const GURL url(kUrl);
   contents()->NavigateAndCommit(url);
   for (bool rendering_on_main : {false, true}) {
     ResetHistograms();
@@ -265,6 +314,11 @@ TEST_F(RenderWidgetHostLatencyTrackerTest, TestWheelToFirstScrollHistograms) {
           0));
       EXPECT_TRUE(
           HistogramSizeEq("Event.Latency.ScrollUpdate.Wheel.GpuSwap2", 0));
+
+      ukm::TestUkmRecorder* test_ukm_recorder =
+          test_browser_client_.GetTestUkmRecorder();
+      EXPECT_EQ(0U, test_ukm_recorder->sources_count());
+      EXPECT_EQ(0U, test_ukm_recorder->entries_count());
     }
   }
 }
@@ -343,7 +397,7 @@ TEST_F(RenderWidgetHostLatencyTrackerTest, TestWheelToScrollHistograms) {
 }
 
 TEST_F(RenderWidgetHostLatencyTrackerTest, TestTouchToFirstScrollHistograms) {
-  const GURL url("http://www.foo.bar.com/subpage/1");
+  const GURL url(kUrl);
   contents()->NavigateAndCommit(url);
   for (bool rendering_on_main : {false, true}) {
     ResetHistograms();
@@ -455,11 +509,16 @@ TEST_F(RenderWidgetHostLatencyTrackerTest, TestTouchToFirstScrollHistograms) {
         "Event.Latency.ScrollUpdate.Touch.BrowserNotifiedToBeforeGpuSwap2", 0));
     EXPECT_TRUE(
         HistogramSizeEq("Event.Latency.ScrollUpdate.Touch.GpuSwap2", 0));
+
+    ukm::TestUkmRecorder* test_ukm_recorder =
+        test_browser_client_.GetTestUkmRecorder();
+    EXPECT_EQ(0U, test_ukm_recorder->sources_count());
+    EXPECT_EQ(0U, test_ukm_recorder->entries_count());
   }
 }
 
 TEST_F(RenderWidgetHostLatencyTrackerTest, TestTouchToScrollHistograms) {
-  const GURL url("http://www.foo.bar.com/subpage/1");
+  const GURL url(kUrl);
   contents()->NavigateAndCommit(url);
   for (bool rendering_on_main : {false, true}) {
     ResetHistograms();
@@ -508,6 +567,10 @@ TEST_F(RenderWidgetHostLatencyTrackerTest, TestTouchToScrollHistograms) {
       tracker()->OnGpuSwapBuffersCompleted(touch_latency);
     }
 
+    // UKM metrics.
+    EXPECT_TRUE(UkmReportAssert(
+        "Event.Latency.ScrollUpdate.Touch.TimeToScrollUpdateSwapBegin2"));
+
     // Rappor metrics.
     EXPECT_TRUE(
         RapporSampleAssert("Event.Latency.ScrollUpdate.Touch."
@@ -565,6 +628,11 @@ TEST_F(RenderWidgetHostLatencyTrackerTest, TestTouchToScrollHistograms) {
         "Event.Latency.ScrollUpdate.Touch.BrowserNotifiedToBeforeGpuSwap2", 1));
     EXPECT_TRUE(
         HistogramSizeEq("Event.Latency.ScrollUpdate.Touch.GpuSwap2", 1));
+
+    ukm::TestUkmRecorder* test_ukm_recorder =
+        test_browser_client_.GetTestUkmRecorder();
+    EXPECT_EQ(1U, test_ukm_recorder->sources_count());
+    EXPECT_EQ(1U, test_ukm_recorder->entries_count());

[tdresser, 2017/05/30 16:52:07]

Should we be using UkmReportAssert here?

[Navid Zolghadr, 2017/05/31 20:37:15]

It is used earlier in the function. I got rid of this to make it more clear as
I'm going to add the test of the metrics right after this and they will verify
anything that will be reported anyway.

   }
 }