Makes the BattOrAgent asynchronous

tools/battor_agent/battor_agent_bin.cc

Index: tools/battor_agent/battor_agent_bin.cc
diff --git a/tools/battor_agent/battor_agent_bin.cc b/tools/battor_agent/battor_agent_bin.cc
index 5980403dbb8dd93321bf0fb2b8ae10e671aed9e4..5dcc0725c9639398782d80fa7336521fd0c5fc5b 100644
--- a/tools/battor_agent/battor_agent_bin.cc
+++ b/tools/battor_agent/battor_agent_bin.cc
@@ -4,6 +4,9 @@
 

[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;
@@ -12,6 +15,13 @@ 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
@@ -36,6 +46,11 @@ bool GetArg(int argnum, int argc, char* argv[], string* value) {
   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[]) {
@@ -55,10 +70,10 @@ int main(int argc, char* argv[]) {
     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;