[ChromeDriver] Subscribe PerformanceLogger to CommandListener interface

chrome/test/chromedriver/logging.cc

Index: chrome/test/chromedriver/logging.cc
diff --git a/chrome/test/chromedriver/logging.cc b/chrome/test/chromedriver/logging.cc
index c9ddf25c24dd3e49be22855b9dbcf343c11c1a0a..2643bebe7d233de727249b1210f035883c06d2f6 100644
--- a/chrome/test/chromedriver/logging.cc
+++ b/chrome/test/chromedriver/logging.cc
@@ -5,6 +5,7 @@
 #include "chrome/test/chromedriver/logging.h"
 
 #include <stdio.h>
+#include <list>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
@@ -14,8 +15,9 @@
 #include "base/time/time.h"
 #include "chrome/test/chromedriver/capabilities.h"
 #include "chrome/test/chromedriver/chrome/console_logger.h"
-#include "chrome/test/chromedriver/chrome/performance_logger.h"
 #include "chrome/test/chromedriver/chrome/status.h"
+#include "chrome/test/chromedriver/command_listener_proxy.h"
+#include "chrome/test/chromedriver/performance_logger.h"
 #include "chrome/test/chromedriver/session.h"
 
 #if defined(OS_POSIX)
@@ -239,9 +241,10 @@ bool InitLogging() {
 
 Status CreateLogs(const Capabilities& capabilities,
                   ScopedVector<WebDriverLog>* out_logs,
-                  ScopedVector<DevToolsEventListener>* out_listeners) {
+                  ScopedVector<DevToolsEventListener>* out_devtools_listeners,
+                  std::list<CommandListener*>* out_command_listeners) {
   ScopedVector<WebDriverLog> logs;
-  ScopedVector<DevToolsEventListener> listeners;
+  ScopedVector<DevToolsEventListener> devtools_listeners;
   Log::Level browser_log_level = Log::kWarning;
   const LoggingPrefs& prefs = capabilities.logging_prefs;
 
@@ -254,7 +257,13 @@ Status CreateLogs(const Capabilities& capabilities,
       if (level != Log::kOff) {
         WebDriverLog* log = new WebDriverLog(type, Log::kAll);
         logs.push_back(log);
-        listeners.push_back(new PerformanceLogger(log));
+        PerformanceLogger* perf_log = new PerformanceLogger(log);
+        // We use a proxy for perf_log's CommandListener interface.
+        // session->chrome will own perf_log, and session will own proxy.

[stgao, 2014/06/27 16:48:24]

The proxy is a good idea.
Nice work!

+        // session->command_listeners (the proxy) will be destroyed first.

[stgao, 2014/06/27 16:48:24]

Why it is destroyed first? What happens if it is not?

[johnmoore, 2014/06/27 21:58:03]

In session.h, |command_listeners| is declared after |chrome|. So when a
|Session| is destroyed, |command_listeners| is destroyed first, as per the C++
spec (members are destroyed in the reverse order of their construction). It will
always be destroyed first so long as the order of declaration isn't changed in
session.h. I placed a comment in session.h noting the importance of the order.

+        CommandListenerProxy* proxy = new CommandListenerProxy(perf_log);
+        devtools_listeners.push_back(perf_log);
+        out_command_listeners->push_back(proxy);
       }
     } else if (type == WebDriverLog::kBrowserType) {
       browser_log_level = level;
@@ -271,9 +280,9 @@ Status CreateLogs(const Capabilities& capabilities,
   logs.push_back(browser_log);
   // If the level is OFF, don't even bother listening for DevTools events.
   if (browser_log_level != Log::kOff)
-    listeners.push_back(new ConsoleLogger(browser_log));
+    devtools_listeners.push_back(new ConsoleLogger(browser_log));
 
   out_logs->swap(logs);
-  out_listeners->swap(listeners);
+  out_devtools_listeners->swap(devtools_listeners);
   return Status(kOk);
 }