Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/

components/metrics/profiler/tracking_synchronizer_unittest.cc

 // Copyright 2015 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 "base/memory/ref_counted.h"
+#include "base/test/simple_test_tick_clock.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 {
@@ skipping 54 matching lines @@
     }
   }
 
  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);
-}
+class TestTrackingSynchronizer : public TrackingSynchronizer {
+ public:
+  explicit TestTrackingSynchronizer(base::TickClock* clock)
+      : TrackingSynchronizer(clock) {}
+
+  ~TestTrackingSynchronizer() {}
+
+  void RegisterPhaseCompletion(
+      ProfilerEventProto::ProfilerEvent profiling_event) {
+    TrackingSynchronizer::RegisterPhaseCompletion(profiling_event);
+  }
+
+  void SendData(const tracked_objects::ProcessDataSnapshot& profiler_data,
+                content::ProcessType process_type,
+                TrackingSynchronizerObserver* observer) const {
+    TrackingSynchronizer::SendData(profiler_data, process_type, observer);
+  }
+};

[Ilya Sherman, 2015/04/07 22:58:35]

Can you test the public API of the class, rather than exposing these
implementation details for the test to call directly?

[vadimt, 2015/04/08 01:09:50]

Not sure it's worth the effort. The purpose of the test if to test the
non-trivial logic of TrackingSynchronizer::SendData rather than the boilerplate
logic of collecting snapshots from the system. I'd need to mock the snapshot
collection (which at the first sight requires significant effort) for
essentially same result. So, testing SendData directly looks like a reasonable
shortcut for me...

 
 }  // namespace
 
 TEST(TrackingSynchronizerTest, ProfilerData) {
   // Testing how TrackingSynchronizer reports 2 phases of profiling.
 #if !defined(OS_IOS)
   content::TestBrowserThreadBundle thread_bundle;
 #endif
-  scoped_refptr<TrackingSynchronizer> tracking_synchronizer =
-      new TrackingSynchronizer(TestTimeFromMs(111));
 
-  // Mimic a phase change event.
-  tracking_synchronizer->phase_completion_events_sequence_.push_back(
+  auto clock = new base::SimpleTestTickClock();

[Ilya Sherman, 2015/04/07 22:58:35]

Please document why this isn't leaked, since it looks a lot like a memory leak.

[vadimt, 2015/04/08 01:09:50]

Done.

+  clock->Advance(base::TimeDelta::FromMilliseconds(111));
+
+  scoped_refptr<TestTrackingSynchronizer> tracking_synchronizer =
+      new TestTrackingSynchronizer(clock);
+
+  clock->Advance(base::TimeDelta::FromMilliseconds(222));
+
+  tracking_synchronizer->RegisterPhaseCompletion(
       ProfilerEventProto::EVENT_FIRST_NONEMPTY_PAINT);
-  tracking_synchronizer->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;
 
+  clock->Advance(base::TimeDelta::FromMilliseconds(444));
   TestObserver test_observer;
-  tracking_synchronizer->SendData(profiler_data,
-                                  content::ProcessType::PROCESS_TYPE_PLUGIN,
-                                  TestTimeFromMs(777), &test_observer);
+  tracking_synchronizer->SendData(
+      profiler_data, content::ProcessType::PROCESS_TYPE_PLUGIN, &test_observer);
 }
 
 }  // namespace metrics