Makes the BattOrAgent asynchronous

tools/battor_agent/battor_agent_bin.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.
 

[Zhen Wang, 2015/10/29 19:40:22]

Can you also add some comments here to explain why we have battor_agent_bin and
who are the users of it?

[charliea (OOO until 10-5), 2015/10/29 21:12:11]

Done.

 #include <iostream>
 
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/waitable_event.h"
 #include "tools/battor_agent/battor_agent.h"
 
 using std::cout;
 using std::endl;
 using std::string;
 
 namespace {
 
+// An event used to signal that the BattOr Agent has finished executing its
+// command.
+base::WaitableEvent g_stop_tracing_complete_event(false, false);
+
+// A string that contains the trace that the BattOr has finished recording.
+scoped_ptr<std::string> g_trace_output;

[Zhen Wang, 2015/10/29 19:40:22]

Is this only for the purpose of printing it out for now? If we have clients for
this battor_agent_bin, do we still need this global variable?

If we want to use this binary by itself, maybe dump the result to a file instead
of using a global?

[nednguyen, 2015/10/29 19:57:32]

Yeah, I am not a big fan of global. Can your StopTracing also take a string
*trace_output & later call the callback with it? 

[charliea (OOO until 10-5), 2015/10/29 21:12:11]

Done. We wouldn't have needed the global variable when calling from Chromium
code, but I think this way is cleaner anyhow.

+
 void PrintUsage() {
   cout << "Usage: battor_agent <command> <arguments>" << endl << endl
        << "Commands:" << endl << endl
        << "  StartTracing <path>" << endl
        << "  StopTracing <path>" << endl
        << "  SupportsExplicitClockSync <path>" << endl
        << "  RecordClockSyncMarker <path> <marker>" << endl
        << "  IssueClockSyncMarker <path>" << endl
        << "  Help" << endl;
 }
 
 // Retrieves argument argnum from the argument list and stores it into value,
 // returning whether the operation was successful and printing the usage
 // guidelines if it wasn't.
 bool GetArg(int argnum, int argc, char* argv[], string* value) {
   if (argnum >= argc) {
     PrintUsage();
     return false;
   }
 
   *value = argv[argnum];
   return true;
 }
 
+void OnStopTracingComplete(scoped_ptr<std::string> trace_output) {
+  g_trace_output = trace_output.Pass();
+  g_stop_tracing_complete_event.Signal();
+}
+
 } // namespace
 
 int main(int argc, char* argv[]) {
   string cmd;
   if (!GetArg(1, argc, argv, &cmd))
     return 1;
 
   if (cmd == "StartTracing") {
     string path;
     if (!GetArg(2, argc, argv, &path))
       return 1;
 
     cout << "Calling StartTracing()" << endl;
     battor::BattOrAgent(path).StartTracing();
   } else if (cmd == "StopTracing") {
     string path;
     if (!GetArg(2, argc, argv, &path))
       return 1;
 
-    string out_trace;
     cout << "Calling StopTracing()" << endl;
-    battor::BattOrAgent(path).StopTracing(&out_trace);
-    cout << out_trace << endl;
+    battor::BattOrAgent(path).StopTracing(base::Bind(&OnStopTracingComplete));
+    g_stop_tracing_complete_event.Wait();

[Zhen Wang, 2015/10/29 19:40:22]

I am fine with treating this as a blocking call right now. But later when we
start to fill in the implementation details, we may need to rethink about this
from client's point of view. How will the client use this?

In Chrome, a trace data sink is used and a callback function is used to notify
when the trace data is available. In Telemetry, stop tracing is a synchronous
call. That's because it blocks on the Telemetry side to wait for the
notification of trace data being ready from the DevTools.

With additional tracing agents added, I think we need to use StopTracing in an
asynchronous way, so that all agents get stopped very quickly. And the the
controller waits for the agents' trace data to be ready.

+nednguyen for more thoughts on the Telemetry side.

[nednguyen, 2015/10/29 19:57:32]

Sorry, but I feel like this is an unnecessary optimization. What gain do we get
from StopTracing on all agents asynchronously then have the tracing_controller
waiting for all data to be available?

[charliea (OOO until 10-5), 2015/10/29 21:12:11]

(I think we covered this in the meeting - we'll stick with this way for now, as
it's most consistent with how the surrounding code in Chrome works.)

+    cout << *g_trace_output.get() << endl;
   } else if (cmd == "SupportsExplicitClockSync") {
     cout << "Calling SupportsExplicitClockSync" << endl;
     cout << battor::BattOrAgent::SupportsExplicitClockSync() << endl;
   } else if (cmd == "RecordClockSyncMarker") {
     string path, marker;
     if (!GetArg(2, argc, argv, &path) || !GetArg(3, argc, argv, &marker))
       return 1;
 
     cout << "Marker: " << marker << endl;
     cout << "Calling RecordClockSyncMarker()" << endl;
     battor::BattOrAgent(path).RecordClockSyncMarker(marker);
   } else if (cmd == "IssueClockSyncMarker") {
     string path;
     if (!GetArg(2, argc, argv, &path))
       return 1;
 
     cout << "Calling IssueClockSyncMarker" << endl;
     battor::BattOrAgent(path).IssueClockSyncMarker();
   } else {
     PrintUsage();
     return 1;
   }
 
   return 0;
 }