Introducing phased profiling framework

components/metrics/profiler/tracking_synchronizer_unittest.cc

Index: components/metrics/profiler/tracking_synchronizer_unittest.cc
diff --git a/components/metrics/profiler/tracking_synchronizer_unittest.cc b/components/metrics/profiler/tracking_synchronizer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..71bd0fd440f35e1fe12913fd3d7980802a85680d
--- /dev/null
+++ b/components/metrics/profiler/tracking_synchronizer_unittest.cc
@@ -0,0 +1,112 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.

[Ilya Sherman, 2015/03/17 23:57:39]

nit: 2015

[vadimt, 2015/03/19 00:20:44]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/memory/ref_counted.h"
+#include "base/tracked_objects.h"
+#include "components/metrics/profiler/tracking_synchronizer.h"
+#include "components/metrics/profiler/tracking_synchronizer_observer.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using tracked_objects::ProcessDataPhaseSnapshot;
+using tracked_objects::TaskSnapshot;
+
+namespace metrics {
+
+class TestObserver : public TrackingSynchronizerObserver {
+ public:
+  TestObserver() {
+  }

[Ilya Sherman, 2015/03/17 23:57:39]

nit: Could you please "git cl format" this file if you haven't already?  I'm
pretty sure that the braces should be on the same line, i.e. "{}".

[vadimt, 2015/03/19 00:20:44]

I'm using it. Just it was OK. I placed {} on the same like, and it's OK again.

+
+  ~TestObserver() {
+    EXPECT_TRUE(got_phase_0_);
+    EXPECT_TRUE(got_phase_1_);
+  }
+
+  virtual void ReceivedProfilerData(
+      const tracked_objects::ProcessDataPhaseSnapshot& process_data_phase,
+      base::ProcessId process_id,
+      content::ProcessType process_type,
+      int profiling_phase,
+      const base::TimeDelta& phase_start,
+      const base::TimeDelta& phase_finish,
+      const ProfilerEvents& past_events) override {
+    ASSERT_EQ(239, process_id);
+    ASSERT_EQ(content::ProcessType::PROCESS_TYPE_PLUGIN, process_type);

[Ilya Sherman, 2015/03/17 23:57:39]

nit: Can the ASSERT_*'s be EXPECT_*'s throughout this file, except in cases
where the code would actually crash if the expectation failed?

+
+    switch (profiling_phase) {
+      case 0:
+        ASSERT_FALSE(got_phase_0_);
+        got_phase_0_ = true;
+
+        ASSERT_EQ(base::TimeDelta::FromMilliseconds(0), phase_start);
+        ASSERT_EQ(base::TimeDelta::FromMilliseconds(222), phase_finish);
+
+        ASSERT_EQ("death_thread0",
+                  process_data_phase.tasks[0].death_thread_name);
+        ASSERT_EQ(0, past_events.size());
+        break;
+
+      case 1:
+        ASSERT_FALSE(got_phase_1_);
+        got_phase_1_ = true;
+
+        ASSERT_EQ(base::TimeDelta::FromMilliseconds(222), phase_start);
+        ASSERT_EQ(base::TimeDelta::FromMilliseconds(666), phase_finish);
+
+        ASSERT_EQ("death_thread1",
+                  process_data_phase.tasks[0].death_thread_name);
+        ASSERT_EQ(1, past_events.size());
+        ASSERT_EQ(ProfilerEventProto::EVENT_FIRST_NONEMPTY_PAINT,
+                  past_events[0]);
+        break;
+
+      default:
+        ASSERT_FALSE("profiling_phase is neither 0 nor 1");
+    }
+  }
+
+ private:
+  bool got_phase_0_ = false;
+  bool got_phase_1_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(TestObserver);
+};
+
+base::TimeTicks TestTimeFromMs(int64 ms) {
+  return base::TimeTicks() + base::TimeDelta::FromMilliseconds(ms);
+}

[Ilya Sherman, 2015/03/17 23:57:39]

nit: Please move the code above this line into an anonymous namespace.

[vadimt, 2015/03/19 00:20:44]

Done.

+
+TEST(TrackingSynchronizerTest, ProfilerData) {
+  // Testing how TrackingSynchronizer reports 2 phases of profiling to
+  // TrackingSynchronizerObserver.
+  content::TestBrowserThreadBundle thread_bundle;
+
+  scoped_refptr<TrackingSynchronizer> ts =

[Ilya Sherman, 2015/03/17 23:57:39]

nit: Please use complete variable names, rather than abbreviations.

[vadimt, 2015/03/19 00:20:44]

Done.

+      new TrackingSynchronizer(TestTimeFromMs(111));
+
+  // Mimic a phase change event.
+  ts->phase_completion_events_sequence_.push_back(
+      ProfilerEventProto::EVENT_FIRST_NONEMPTY_PAINT);
+  ts->phase_start_times_.push_back(TestTimeFromMs(333));
+
+  tracked_objects::ProcessDataSnapshot profiler_data;
+  ProcessDataPhaseSnapshot snapshot0;
+  tracked_objects::TaskSnapshot task_snapshot0;
+  task_snapshot0.death_thread_name = "death_thread0";
+  snapshot0.tasks.push_back(task_snapshot0);
+  ProcessDataPhaseSnapshot snapshot1;
+  profiler_data.phased_process_data_snapshots[0] = snapshot0;
+  tracked_objects::TaskSnapshot task_snapshot1;
+  task_snapshot1.death_thread_name = "death_thread1";
+  snapshot1.tasks.push_back(task_snapshot1);
+  profiler_data.phased_process_data_snapshots[1] = snapshot1;
+  profiler_data.process_id = 239;
+
+  TestObserver to;

[Ilya Sherman, 2015/03/17 23:57:38]

nit: Please use complete variable names, rather than abbreviations.

[vadimt, 2015/03/19 00:20:44]

Done.

+  ts->SendData(profiler_data, content::ProcessType::PROCESS_TYPE_PLUGIN,
+               TestTimeFromMs(777), &to);
+}
+
+}  // namespace metrics