[Telemetry] Refactor Chrome devtools tracing

Platform-level tracing doc:
https://docs.google.com/document/d/1Uv8SzOq2ugvzLiOcP85g7qPiaxwsn4tgOdxpe3YjUkM/edit?usp=sharing

BUG=515323

Review URL: https://codereview.chromium.org/1338033002

Cr-Commit-Position: refs/heads/master@{#348740}

tools/telemetry/telemetry/internal/backends/chrome_inspector/devtools_client_backend.py

@@ -11,7 +11,8 @@ from telemetry import decorators
 from telemetry.internal.backends.chrome_inspector import devtools_http
 from telemetry.internal.backends.chrome_inspector import inspector_backend
 from telemetry.internal.backends.chrome_inspector import tracing_backend
-from telemetry.internal.platform.tracing_agent import chrome_tracing_agent
+from telemetry.internal.platform.tracing_agent import (
+    chrome_tracing_devtools_manager)
 from telemetry.timeline import trace_data as trace_data_module
 
 
@@ -61,18 +62,22 @@ class DevToolsClientBackend(object):
     self._devtools_port = devtools_port
     self._remote_devtools_port = remote_devtools_port
     self._devtools_http = devtools_http.DevToolsHttp(devtools_port)
-    self._tracing_backend = None
+    self._tracing_backend = tracing_backend.TracingBackend(self._devtools_port)
     self._app_backend = app_backend
     self._devtools_context_map_backend = _DevToolsContextMapBackend(
         self._app_backend, self)
 
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
-      self, self._app_backend.platform_backend)
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
+        self, self._app_backend.platform_backend)
 
   @property
   def remote_port(self):
     return self._remote_devtools_port
 
+  @property
+  def is_tracing_running(self):
+    return self._tracing_backend.is_tracing_running
+
   def IsAlive(self):
     """Whether the DevTools server is available and connectable."""
     return _IsDevToolsAgentAvailable(self._devtools_http)
@@ -170,13 +175,7 @@ class DevToolsClientBackend(object):
     self._devtools_context_map_backend._Update(contexts)
     return self._devtools_context_map_backend
 
-  def _CreateTracingBackendIfNeeded(self):
-    if not self._tracing_backend:
-      self._tracing_backend = tracing_backend.TracingBackend(
-          self._devtools_port)
-
   def IsChromeTracingSupported(self):
-    self._CreateTracingBackendIfNeeded()
     return self._tracing_backend.IsTracingSupported()
 
   def StartChromeTracing(
@@ -191,7 +190,6 @@ class DevToolsClientBackend(object):
                          those three event categories.
     """
     assert trace_options and trace_options.enable_chrome_trace
-    self._CreateTracingBackendIfNeeded()
     return self._tracing_backend.StartTracing(
         trace_options, custom_categories, timeout)
 
@@ -227,7 +225,6 @@ class DevToolsClientBackend(object):
       TracingUnexpectedResponseException: If the response contains an error
       or does not contain the expected result.
     """
-    self._CreateTracingBackendIfNeeded()
     return self._tracing_backend.DumpMemory(timeout)
 
 

tools/telemetry/telemetry/internal/platform/tracing_agent/chrome_devtools_tracing_backend.py

-# Copyright 2014 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.
-import sys
-import traceback
-
-
-class ChromeTracingStartedError(Exception):
-  pass
-
-
-class ChromeTracingStoppedError(Exception):
-  pass
-
-
-class ChromeDevtoolsTracingBackend(object):
-  # A singleton map from platform backends to maps of uniquely-identifying
-  # remote port (which may be the same as local port) to DevToolsClientBackend.
-  # There is no guarantee that the devtools agent is still alive.
-  _platform_backends_to_devtools_clients_maps = {}
-  _is_tracing_running_for_platform_backend = {}
-  _is_tracing_running_for_platform_backend.setdefault(False)
-
-  def __init__(self, platform_backend):
-    self._platform_backend = platform_backend
-
-  @classmethod
-  def _RemoveStaleDevToolsClient(cls, platform_backend):
-    """Removes DevTools clients that are no longer connectable."""
-    devtools_clients_map = cls._platform_backends_to_devtools_clients_maps.get(
-        platform_backend, {})
-    devtools_clients_map = {
-        port: client
-        for port, client in devtools_clients_map.iteritems()
-        if client.IsAlive()
-        }
-    cls._platform_backends_to_devtools_clients_maps[platform_backend] = (
-        devtools_clients_map)
-
-  @classmethod
-  def RegisterDevToolsClient(cls, devtools_client_backend, platform_backend):
-    is_tracing_running = cls._is_tracing_running_for_platform_backend.get(
-        platform_backend)
-    if is_tracing_running:
-      raise ChromeTracingStartedError(
-          'Cannot add new DevTools client when tracing is running on '
-          'platform backend %s.' % platform_backend)
-    remote_port = str(devtools_client_backend.remote_port)
-    if platform_backend not in cls._platform_backends_to_devtools_clients_maps:
-      cls._platform_backends_to_devtools_clients_maps[platform_backend] = {}
-    devtools_clients_map = (
-      cls._platform_backends_to_devtools_clients_maps[platform_backend])
-    devtools_clients_map[remote_port] = devtools_client_backend
-
-  @classmethod
-  def IsSupported(cls, platform_backend):
-    cls._RemoveStaleDevToolsClient(platform_backend)
-    devtools_clients_map = cls._platform_backends_to_devtools_clients_maps.get(
-        platform_backend, {})
-    for _, devtools_client in devtools_clients_map.iteritems():
-      if devtools_client.IsChromeTracingSupported():
-        return True
-    return False
-
-  @property
-  def _is_active(self):
-    return self._is_tracing_running_for_platform_backend.get(
-        self._platform_backend)
-
-  @_is_active.setter
-  def _is_active(self, value):
-    self._is_tracing_running_for_platform_backend[self._platform_backend] = (
-        value)
-
-  def Start(self, trace_options, category_filter, timeout):
-    if not trace_options.enable_chrome_trace:
-      return False
-
-    if self._is_active:
-      raise ChromeTracingStartedError(
-          'Tracing is already running on platform backend %s.'
-          % self._platform_backend)
-    self._RemoveStaleDevToolsClient(self._platform_backend)
-    devtools_clients_map = self._platform_backends_to_devtools_clients_maps.get(
-        self._platform_backend, {})
-    if not devtools_clients_map:
-      return False
-    for _, devtools_client in devtools_clients_map.iteritems():
-      devtools_client.StartChromeTracing(
-          trace_options, category_filter.filter_string, timeout)
-    self._is_active = True
-    return True
-
-  def Stop(self, trace_data_builder):
-    devtools_clients_map = (
-      self._platform_backends_to_devtools_clients_maps[self._platform_backend])
-    raised_execption_messages = []
-    for devtools_port, devtools_client in devtools_clients_map.iteritems():
-      # We do not check for stale DevTools client, so that we get an
-      # exception if there is a stale client. This is because we
-      # will potentially lose data if there is a stale client.
-      try:
-        devtools_client.StopChromeTracing(trace_data_builder)
-      except Exception:
-        raised_execption_messages.append(
-          'Error when trying to stop tracing on devtools at port %s:\n%s'
-          % (devtools_port,
-             ''.join(traceback.format_exception(*sys.exc_info()))))
-
-    self._is_active = False
-    if raised_execption_messages:
-      raise ChromeTracingStoppedError(
-          'Exceptions raised when trying to stop devtool tracing\n:' +
-          '\n'.join(raised_execption_messages))

tools/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py

@@ -5,11 +5,13 @@
 import os
 import shutil
 import stat
+import sys
 import tempfile
+import traceback
 
 from telemetry.internal.platform import tracing_agent
 from telemetry.internal.platform.tracing_agent import (
-    chrome_devtools_tracing_backend)
+    chrome_tracing_devtools_manager)
 
 _DESKTOP_OS_NAMES = ['linux', 'mac', 'win']
 
@@ -19,34 +21,63 @@ _CHROME_TRACE_CONFIG_DIR_ANDROID = '/data/local/'
 _CHROME_TRACE_CONFIG_FILE_NAME = 'chrome-trace-config.json'
 
 
+class ChromeTracingStartedError(Exception):
+  pass
+
+
+class ChromeTracingStoppedError(Exception):
+  pass
+
+
 class ChromeTracingAgent(tracing_agent.TracingAgent):
   def __init__(self, platform_backend):
     super(ChromeTracingAgent, self).__init__(platform_backend)
-    self._chrome_devtools_tracing_backend = (
-      chrome_devtools_tracing_backend.ChromeDevtoolsTracingBackend(
-        platform_backend))
     self._trace_config_file = None
 
   @property
   def trace_config_file(self):
     return self._trace_config_file
 
-  @classmethod
-  def RegisterDevToolsClient(cls, devtools_client_backend, platform_backend):
-    (chrome_devtools_tracing_backend.ChromeDevtoolsTracingBackend
-        .RegisterDevToolsClient(devtools_client_backend, platform_backend))
-
   @classmethod
   def IsSupported(cls, platform_backend):
-    return (chrome_devtools_tracing_backend.ChromeDevtoolsTracingBackend
-      .IsSupported(platform_backend))
+    return chrome_tracing_devtools_manager.IsSupported(platform_backend)
 
   def Start(self, trace_options, category_filter, timeout):
-    return self._chrome_devtools_tracing_backend.Start(
-        trace_options, category_filter, timeout)
+    if not trace_options.enable_chrome_trace:
+      return False
+    devtools_clients = (chrome_tracing_devtools_manager
+        .GetActiveDevToolsClients(self._platform_backend))
+    if not devtools_clients:
+      return False
+    for client in devtools_clients:
+      if client.is_tracing_running:
+        raise ChromeTracingStartedError(
+            'Tracing is already running on devtools at port %s on platform'
+            'backend %s.' % (client.remote_port, self._platform_backend))
+      client.StartChromeTracing(
+          trace_options, category_filter.filter_string, timeout)
+    return True
 
   def Stop(self, trace_data_builder):
-    self._chrome_devtools_tracing_backend.Stop(trace_data_builder)
+    # We get all DevTools clients including the stale ones, so that we get an
+    # exception if there is a stale client. This is because we will potentially
+    # lose data if there is a stale client.
+    devtools_clients = (chrome_tracing_devtools_manager
+        .GetDevToolsClients(self._platform_backend))
+    raised_execption_messages = []
+    for client in devtools_clients:
+      try:
+        client.StopChromeTracing(trace_data_builder)
+      except Exception:
+        raised_execption_messages.append(
+          'Error when trying to stop tracing on devtools at port %s:\n%s'
+          % (client.remote_port,
+             ''.join(traceback.format_exception(*sys.exc_info()))))
+
+    if raised_execption_messages:
+      raise ChromeTracingStoppedError(
+          'Exceptions raised when trying to stop devtool tracing\n:' +
+          '\n'.join(raised_execption_messages))
 
   def _CreateTraceConfigFile(self, config):
     assert not self._trace_config_file

tools/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent_unittest.py

 # Copyright 2014 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.
-
 import os
 import stat
 import unittest
 
 from telemetry import decorators
-from telemetry.internal.platform.tracing_agent import (
-    chrome_devtools_tracing_backend)
 from telemetry.internal.platform.tracing_agent import chrome_tracing_agent
+from telemetry.internal.platform.tracing_agent import (
+    chrome_tracing_devtools_manager)
 from telemetry.timeline import tracing_category_filter
 from telemetry.timeline import tracing_config
 from telemetry.timeline import tracing_options
@@ -17,11 +16,19 @@ from telemetry.timeline import tracing_options
 from devil.android import device_utils
 
 
+class FakeTracingControllerBackend(object):
+  def __init__(self):
+    self.is_tracing_running = False
+
+
 class FakePlatformBackend(object):
-  pass
+  def __init__(self):
+    self.tracing_controller_backend = FakeTracingControllerBackend()
+
 
 class FakeAndroidPlatformBackend(FakePlatformBackend):
   def __init__(self):
+    super(FakeAndroidPlatformBackend, self).__init__()
     devices = device_utils.DeviceUtils.HealthyDevices(None)
     self.device = devices[0]
 
@@ -44,7 +51,7 @@ class FakeWinPlatformBackend(FakePlatformBackend):
 class FakeDevtoolsClient(object):
   def __init__(self, remote_port):
     self.is_alive = True
-    self.tracing_started = False
+    self.is_tracing_running = False
     self.remote_port = remote_port
     self.will_raise_exception_in_stop_tracing = False
 
@@ -52,10 +59,10 @@ class FakeDevtoolsClient(object):
     return self.is_alive
 
   def StartChromeTracing(self, _trace_options, _filter_string, _timeout=10):
-    self.tracing_started = True
+    self.is_tracing_running = True
 
   def StopChromeTracing(self, _trace_data_builder):
-    self.tracing_started = False
+    self.is_tracing_running = False
     if self.will_raise_exception_in_stop_tracing:
       raise Exception
 
@@ -73,7 +80,7 @@ class FakeCategoryFilter(object):
     self.filter_string = 'foo'
 
 
-class ChromeTracingAgentUnittest(unittest.TestCase):
+class ChromeTracingAgentTest(unittest.TestCase):
   def setUp(self):
     self.platform1 = FakePlatformBackend()
     self.platform2 = FakePlatformBackend()
@@ -84,31 +91,34 @@ class ChromeTracingAgentUnittest(unittest.TestCase):
     agent = chrome_tracing_agent.ChromeTracingAgent(platform_backend)
     trace_options = FakeTraceOptions()
     trace_options.enable_chrome_trace = enable_chrome_trace
+    agent._platform_backend.tracing_controller_backend.is_tracing_running = True
     agent.Start(trace_options, FakeCategoryFilter(), 10)
     return agent
 
-  def StopTracing(self, tracing_agent):
-    tracing_agent.Stop(None)
+  def StopTracing(self, agent):
+    agent._platform_backend.tracing_controller_backend.is_tracing_running = (
+        False)
+    agent.Stop(None)
 
   def testRegisterDevtoolsClient(self):
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         FakeDevtoolsClient(1), self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         FakeDevtoolsClient(2), self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         FakeDevtoolsClient(3), self.platform1)
 
     tracing_agent_of_platform1 = self.StartTracing(self.platform1)
 
     with self.assertRaises(
-        chrome_devtools_tracing_backend.ChromeTracingStartedError):
-      chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+        chrome_tracing_devtools_manager.RegisterDevToolsClientError):
+      chrome_tracing_devtools_manager.RegisterDevToolsClient(
         FakeDevtoolsClient(4), self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         FakeDevtoolsClient(5), self.platform2)
 
     self.StopTracing(tracing_agent_of_platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         FakeDevtoolsClient(6), self.platform1)
 
   def testIsSupport(self):
@@ -121,9 +131,9 @@ class ChromeTracingAgentUnittest(unittest.TestCase):
 
     devtool1 = FakeDevtoolsClient(1)
     devtool2 = FakeDevtoolsClient(2)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool1, self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool2, self.platform2)
     devtool2.is_alive = False
 
@@ -142,65 +152,63 @@ class ChromeTracingAgentUnittest(unittest.TestCase):
     devtool3 = FakeDevtoolsClient(3)
     devtool4 = FakeDevtoolsClient(2)
     # Register devtools 1, 2, 3 on platform1 and devtool 4 on platform 2
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool1, self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool2, self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool3, self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool4, self.platform2)
     devtool2.is_alive = False
 
     tracing_agent1 = self.StartTracing(self.platform1)
-    with self.assertRaises(
-        chrome_devtools_tracing_backend.ChromeTracingStartedError):
+    with self.assertRaises(chrome_tracing_agent.ChromeTracingStartedError):
       self.StartTracing(self.platform1)
 
-    self.assertTrue(devtool1.tracing_started)
-    self.assertFalse(devtool2.tracing_started)
-    self.assertTrue(devtool3.tracing_started)
+    self.assertTrue(devtool1.is_tracing_running)
+    self.assertFalse(devtool2.is_tracing_running)
+    self.assertTrue(devtool3.is_tracing_running)
     # Devtool 4 shouldn't have tracing started although it has the same remote
     # port as devtool 2
-    self.assertFalse(devtool4.tracing_started)
+    self.assertFalse(devtool4.is_tracing_running)
 
     self.StopTracing(tracing_agent1)
-    self.assertFalse(devtool1.tracing_started)
-    self.assertFalse(devtool2.tracing_started)
-    self.assertFalse(devtool3.tracing_started)
-    self.assertFalse(devtool4.tracing_started)
+    self.assertFalse(devtool1.is_tracing_running)
+    self.assertFalse(devtool2.is_tracing_running)
+    self.assertFalse(devtool3.is_tracing_running)
+    self.assertFalse(devtool4.is_tracing_running)
     # Test that it should be ok to start & stop tracing on platform1 again.
     tracing_agent1 = self.StartTracing(self.platform1)
     self.StopTracing(tracing_agent1)
 
     tracing_agent2 = self.StartTracing(self.platform2)
-    self.assertTrue(devtool4.tracing_started)
+    self.assertTrue(devtool4.is_tracing_running)
     self.StopTracing(tracing_agent2)
-    self.assertFalse(devtool4.tracing_started)
+    self.assertFalse(devtool4.is_tracing_running)
 
   def testExceptionRaisedInStopTracing(self):
     devtool1 = FakeDevtoolsClient(1)
     devtool2 = FakeDevtoolsClient(2)
     # Register devtools 1, 2 on platform 1
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool1, self.platform1)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool2, self.platform1)
     tracing_agent1 = self.StartTracing(self.platform1)
 
-    self.assertTrue(devtool1.tracing_started)
-    self.assertTrue(devtool2.tracing_started)
+    self.assertTrue(devtool1.is_tracing_running)
+    self.assertTrue(devtool2.is_tracing_running)
 
     devtool2.will_raise_exception_in_stop_tracing = True
-    with self.assertRaises(
-        chrome_devtools_tracing_backend.ChromeTracingStoppedError):
+    with self.assertRaises(chrome_tracing_agent.ChromeTracingStoppedError):
       self.StopTracing(tracing_agent1)
 
     devtool1.is_alive = False
     devtool2.is_alive = False
     # Register devtools 3 on platform 1 should not raise any exception.
     devtool3 = FakeDevtoolsClient(3)
-    chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
+    chrome_tracing_devtools_manager.RegisterDevToolsClient(
         devtool3, self.platform1)
 
     # Start & Stop tracing on platform 1 should work just fine.

tools/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_devtools_manager.py

+# 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.
+
+class RegisterDevToolsClientError(Exception):
+  pass
+
+
+# A singleton map from platform backends to maps of uniquely-identifying
+# remote port (which may be the same as local port) to DevToolsClientBackend.
+# There is no guarantee that the devtools agent is still alive.
+_platform_backends_to_devtools_clients_maps = {}
+
+
+def _RemoveStaleDevToolsClient(platform_backend):
+  """Removes DevTools clients that are no longer connectable."""
+  devtools_clients_map = _platform_backends_to_devtools_clients_maps.get(
+      platform_backend, {})
+  devtools_clients_map = {
+      port: client
+      for port, client in devtools_clients_map.iteritems()
+      if client.IsAlive()
+      }
+  _platform_backends_to_devtools_clients_maps[platform_backend] = (
+      devtools_clients_map)
+
+
+def RegisterDevToolsClient(devtools_client_backend, platform_backend):
+  """Register DevTools client
+
+  This should only be called from DevToolsClientBackend when it is initialized.
+  """
+  if platform_backend.tracing_controller_backend.is_tracing_running:
+    raise RegisterDevToolsClientError(
+        'Cannot add new DevTools client when tracing is running on '
+        'platform backend %s.' % platform_backend)
+
+  remote_port = str(devtools_client_backend.remote_port)
+  if platform_backend not in _platform_backends_to_devtools_clients_maps:
+    _platform_backends_to_devtools_clients_maps[platform_backend] = {}
+  devtools_clients_map = (
+    _platform_backends_to_devtools_clients_maps[platform_backend])
+  devtools_clients_map[remote_port] = devtools_client_backend
+
+
+def IsSupported(platform_backend):
+  _RemoveStaleDevToolsClient(platform_backend)
+  devtools_clients_map = _platform_backends_to_devtools_clients_maps.get(
+      platform_backend, {})
+  for _, devtools_client in devtools_clients_map.iteritems():
+    if devtools_client.IsChromeTracingSupported():
+      return True
+  return False
+
+
+def GetDevToolsClients(platform_backend):
+  """Get DevTools clients including the ones that are no longer connectable."""
+  devtools_clients_map = _platform_backends_to_devtools_clients_maps.get(
+      platform_backend, {})
+  if not devtools_clients_map:
+    return []
+  return devtools_clients_map.values()
+
+def GetActiveDevToolsClients(platform_backend):
+  """Get DevTools clients that are still connectable."""
+  _RemoveStaleDevToolsClient(platform_backend)
+  return GetDevToolsClients(platform_backend)