Merge "ftrace_reader: Automatically start/stop tracing"
diff --git a/include/perfetto/ftrace_reader/ftrace_controller.h b/include/perfetto/ftrace_reader/ftrace_controller.h
index 2829437..848940e 100644
--- a/include/perfetto/ftrace_reader/ftrace_controller.h
+++ b/include/perfetto/ftrace_reader/ftrace_controller.h
@@ -111,9 +111,6 @@
 
   std::unique_ptr<FtraceSink> CreateSink(FtraceConfig, FtraceSink::Delegate*);
 
-  void Start();
-  void Stop();
-
  protected:
   // Protected for testing.
   FtraceController(std::unique_ptr<FtraceProcfs>,
@@ -132,6 +129,9 @@
   void RegisterForEvent(const std::string& event_name);
   void UnregisterForEvent(const std::string& event_name);
 
+  void StartIfNeeded();
+  void StopIfNeeded();
+
   // Called when we know there is data for the raw pipe
   // for the given |cpu|. Kicks off the reading/parsing
   // of the pipe.
diff --git a/src/ftrace_reader/end_to_end_integrationtest.cc b/src/ftrace_reader/end_to_end_integrationtest.cc
index 7e35ffa..4201796 100644
--- a/src/ftrace_reader/end_to_end_integrationtest.cc
+++ b/src/ftrace_reader/end_to_end_integrationtest.cc
@@ -111,15 +111,9 @@
   // Let some events build up.
   sleep(1);
 
-  // Start watching pipe fds.
-  ftrace->Start();
-
   // Start processing the tasks (OnBundleComplete will quit the task runner).
   runner()->Run();
 
-  // Stop listening to fds.
-  ftrace->Stop();
-
   // Disable events.
   sink.reset();
 
diff --git a/src/ftrace_reader/ftrace_controller.cc b/src/ftrace_reader/ftrace_controller.cc
index 81961b2..f68853f 100644
--- a/src/ftrace_reader/ftrace_controller.cc
+++ b/src/ftrace_reader/ftrace_controller.cc
@@ -67,13 +67,17 @@
       ftrace_procfs_->DisableEvent(event->group, event->name);
     }
   }
+  if (listening_for_raw_trace_data_) {
+    sinks_.clear();
+    StopIfNeeded();
+  }
 }
 
-void FtraceController::Start() {
-  if (listening_for_raw_trace_data_) {
-    PERFETTO_DLOG("FtraceController is already started.");
+void FtraceController::StartIfNeeded() {
+  if (sinks_.size() > 1)
     return;
-  }
+  PERFETTO_CHECK(sinks_.size() != 0);
+  PERFETTO_CHECK(!listening_for_raw_trace_data_);
   listening_for_raw_trace_data_ = true;
   ftrace_procfs_->EnableTracing();
   for (size_t cpu = 0; cpu < ftrace_procfs_->NumberOfCpus(); cpu++) {
@@ -90,17 +94,18 @@
   }
 }
 
-void FtraceController::Stop() {
-  if (!listening_for_raw_trace_data_) {
-    PERFETTO_DLOG("FtraceController is already stopped.");
+void FtraceController::StopIfNeeded() {
+  if (sinks_.size() != 0)
     return;
-  }
+  PERFETTO_CHECK(listening_for_raw_trace_data_);
   listening_for_raw_trace_data_ = false;
   for (size_t cpu = 0; cpu < ftrace_procfs_->NumberOfCpus(); cpu++) {
     CpuReader* reader = GetCpuReader(cpu);
     int fd = reader->GetFileDescriptor();
     task_runner_->RemoveFileDescriptorWatch(fd);
   }
+  readers_.clear();
+  ftrace_procfs_->DisableTracing();
 }
 
 void FtraceController::OnRawFtraceDataAvailable(size_t cpu) {
@@ -151,6 +156,8 @@
   PERFETTO_DCHECK_THREAD(thread_checker_);
   auto it_and_inserted = sinks_.insert(sink);
   PERFETTO_DCHECK(it_and_inserted.second);
+
+  StartIfNeeded();
   for (const std::string& name : sink->enabled_events())
     RegisterForEvent(name);
 }
@@ -183,8 +190,10 @@
   PERFETTO_DCHECK_THREAD(thread_checker_);
   size_t removed = sinks_.erase(sink);
   PERFETTO_DCHECK(removed == 1);
+
   for (const std::string& name : sink->enabled_events())
     UnregisterForEvent(name);
+  StopIfNeeded();
 }
 
 FtraceSink::FtraceSink(base::WeakPtr<FtraceController> controller_weak,
diff --git a/src/ftrace_reader/ftrace_controller_unittest.cc b/src/ftrace_reader/ftrace_controller_unittest.cc
index 9f58211..e03f0b9 100644
--- a/src/ftrace_reader/ftrace_controller_unittest.cc
+++ b/src/ftrace_reader/ftrace_controller_unittest.cc
@@ -32,6 +32,7 @@
 using testing::Return;
 using testing::ByMove;
 using testing::AnyNumber;
+using testing::NiceMock;
 
 using Table = perfetto::ProtoTranslationTable;
 using FtraceEventBundle = perfetto::protos::pbzero::FtraceEventBundle;
@@ -40,6 +41,9 @@
 
 namespace {
 
+const char kFooEnablePath[] = "/root/events/group/foo/enable";
+const char kBarEnablePath[] = "/root/events/group/bar/enable";
+
 class MockTaskRunner : public base::TaskRunner {
  public:
   MOCK_METHOD1(PostTask, void(std::function<void()>));
@@ -127,9 +131,9 @@
 }  // namespace
 
 TEST(FtraceControllerTest, NoExistentEventsDontCrash) {
-  MockTaskRunner task_runner;
+  NiceMock<MockTaskRunner> task_runner;
   auto ftrace_procfs =
-      std::unique_ptr<MockFtraceProcfs>(new MockFtraceProcfs());
+      std::unique_ptr<MockFtraceProcfs>(new NiceMock<MockFtraceProcfs>());
   TestFtraceController controller(std::move(ftrace_procfs), &task_runner,
                                   FakeTable());
 
@@ -151,12 +155,14 @@
   MockDelegate delegate;
   FtraceConfig config({"foo"});
 
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/foo/enable", "1"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile("/root/tracing_on", "1"));
+  EXPECT_CALL(task_runner, AddFileDescriptorWatch(_, _));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kFooEnablePath, "1"));
   std::unique_ptr<FtraceSink> sink = controller.CreateSink(config, &delegate);
 
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/foo/enable", "0"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kFooEnablePath, "0"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile("/root/tracing_on", "0"));
+  EXPECT_CALL(task_runner, RemoveFileDescriptorWatch(_));
   sink.reset();
 }
 
@@ -173,20 +179,20 @@
   FtraceConfig configA({"foo"});
   FtraceConfig configB({"foo", "bar"});
 
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/foo/enable", "1"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile("/root/tracing_on", "1"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kFooEnablePath, "1"));
+  EXPECT_CALL(task_runner, AddFileDescriptorWatch(_, _));
   std::unique_ptr<FtraceSink> sinkA = controller.CreateSink(configA, &delegate);
 
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/bar/enable", "1"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kBarEnablePath, "1"));
   std::unique_ptr<FtraceSink> sinkB = controller.CreateSink(configB, &delegate);
 
   sinkA.reset();
 
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/foo/enable", "0"));
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/bar/enable", "0"));
+  EXPECT_CALL(task_runner, RemoveFileDescriptorWatch(_));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kFooEnablePath, "0"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kBarEnablePath, "0"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile("/root/tracing_on", "0"));
   sinkB.reset();
 }
 
@@ -201,38 +207,17 @@
   MockDelegate delegate;
   FtraceConfig config({"foo"});
 
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/foo/enable", "1"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile("/root/tracing_on", "1"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kFooEnablePath, "1"));
+  EXPECT_CALL(task_runner, AddFileDescriptorWatch(_, _));
   std::unique_ptr<FtraceSink> sink = controller->CreateSink(config, &delegate);
 
-  EXPECT_CALL(*raw_ftrace_procfs,
-              WriteToFile("/root/events/group/foo/enable", "0"));
+  EXPECT_CALL(task_runner, RemoveFileDescriptorWatch(_));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile(kFooEnablePath, "0"));
+  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile("/root/tracing_on", "0"));
   controller.reset();
 
   sink.reset();
 }
 
-TEST(FtraceControllerTest, StartStop) {
-  MockTaskRunner task_runner;
-  auto ftrace_procfs =
-      std::unique_ptr<MockFtraceProcfs>(new MockFtraceProcfs());
-  auto raw_ftrace_procfs = ftrace_procfs.get();
-  TestFtraceController controller(std::move(ftrace_procfs), &task_runner,
-                                  FakeTable());
-
-  // Stopping before we start does nothing.
-  controller.Stop();
-
-  EXPECT_CALL(*raw_ftrace_procfs, WriteToFile("/root/tracing_on", "1"));
-  EXPECT_CALL(task_runner, AddFileDescriptorWatch(_, _));
-  controller.Start();
-  // Double start does nothing.
-  controller.Start();
-
-  EXPECT_CALL(task_runner, RemoveFileDescriptorWatch(_));
-  controller.Stop();
-  // Double stop does nothing.
-  controller.Stop();
-}
-
 }  // namespace perfetto
diff --git a/src/ftrace_reader/ftrace_procfs.cc b/src/ftrace_reader/ftrace_procfs.cc
index 225aa62..c838df3 100644
--- a/src/ftrace_reader/ftrace_procfs.cc
+++ b/src/ftrace_reader/ftrace_procfs.cc
@@ -83,6 +83,11 @@
   return WriteToFile(path, "0");
 }
 
+bool FtraceProcfs::DisableAllEvents() {
+  std::string path = root_ + "events/enable";
+  return WriteToFile(path, "0");
+}
+
 std::string FtraceProcfs::ReadEventFormat(const std::string& group,
                                           const std::string& name) const {
   std::string path = root_ + "events/" + group + "/" + name + "/format";
diff --git a/src/ftrace_reader/ftrace_procfs.h b/src/ftrace_reader/ftrace_procfs.h
index abd0412..8169816 100644
--- a/src/ftrace_reader/ftrace_procfs.h
+++ b/src/ftrace_reader/ftrace_procfs.h
@@ -34,6 +34,9 @@
   // Disable the event under with the given |group| and |name|.
   bool DisableEvent(const std::string& group, const std::string& name);
 
+  // Disable all events by writing to the global enable file.
+  bool DisableAllEvents();
+
   // Read the format for event with the given |group| and |name|.
   std::string ReadEventFormat(const std::string& group,
                               const std::string& name) const;
diff --git a/src/ftrace_reader/ftrace_procfs_integrationtest.cc b/src/ftrace_reader/ftrace_procfs_integrationtest.cc
index fa7b3dd..ccf2154 100644
--- a/src/ftrace_reader/ftrace_procfs_integrationtest.cc
+++ b/src/ftrace_reader/ftrace_procfs_integrationtest.cc
@@ -30,6 +30,12 @@
 const char kTracingPath[] = "/sys/kernel/debug/tracing/";
 const char kTracePath[] = "/sys/kernel/debug/tracing/trace";
 
+void ResetFtrace(FtraceProcfs* ftrace) {
+  ftrace->DisableAllEvents();
+  ftrace->ClearTrace();
+  ftrace->EnableTracing();
+}
+
 std::string GetTraceOutput() {
   std::ifstream fin(kTracePath, std::ios::in);
   if (!fin) {
@@ -46,6 +52,7 @@
 
 TEST(FtraceProcfsIntegrationTest, ClearTrace) {
   FtraceProcfs ftrace(kTracingPath);
+  ResetFtrace(&ftrace);
   ftrace.WriteTraceMarker("Hello, World!");
   ftrace.ClearTrace();
   EXPECT_THAT(GetTraceOutput(), Not(HasSubstr("Hello, World!")));
@@ -53,12 +60,14 @@
 
 TEST(FtraceProcfsIntegrationTest, TraceMarker) {
   FtraceProcfs ftrace(kTracingPath);
+  ResetFtrace(&ftrace);
   ftrace.WriteTraceMarker("Hello, World!");
   EXPECT_THAT(GetTraceOutput(), HasSubstr("Hello, World!"));
 }
 
 TEST(FtraceProcfsIntegrationTest, EnableDisableEvent) {
   FtraceProcfs ftrace(kTracingPath);
+  ResetFtrace(&ftrace);
   ftrace.EnableEvent("sched", "sched_switch");
   sleep(1);
   EXPECT_THAT(GetTraceOutput(), HasSubstr("sched_switch"));
@@ -71,7 +80,7 @@
 
 TEST(FtraceProcfsIntegrationTest, EnableDisableTracing) {
   FtraceProcfs ftrace(kTracingPath);
-  ftrace.ClearTrace();
+  ResetFtrace(&ftrace);
   EXPECT_TRUE(ftrace.IsTracingEnabled());
   ftrace.WriteTraceMarker("Before");
   ftrace.DisableTracing();
@@ -92,6 +101,12 @@
   EXPECT_THAT(format, HasSubstr("field:char buf"));
 }
 
+TEST(FtraceProcfsIntegrationTest, ReadAvailableEvents) {
+  FtraceProcfs ftrace(kTracingPath);
+  std::string format = ftrace.ReadAvailableEvents();
+  EXPECT_THAT(format, HasSubstr("sched:sched_switch"));
+}
+
 TEST(FtraceProcfsIntegrationTest, CanOpenTracePipeRaw) {
   FtraceProcfs ftrace(kTracingPath);
   EXPECT_TRUE(ftrace.OpenPipeForCpu(0));