Commit 0f07fbef authored by nednguyen's avatar nednguyen Committed by Commit bot

[Telemetry] Make chrome_tracing_agent state more resilient to tracing exception.

Because ChromeTracingAgent class has global states, when any
exception was raised inside StopTracing method, the active tracing
state of this class is not set to False, making subsequent unittest that
try to StartTracing fails because of this.

Long term fix of this depends on crbug.com/317481 be fixed so that
ChromeTracingAgent no longer have global state.

BUG=459807

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

Cr-Commit-Position: refs/heads/master@{#320799}
parent b386883d
# Copyright 2014 The Chromium Authors. All rights reserved. # Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
import sys
import traceback
from telemetry.core.platform import tracing_agent from telemetry.core.platform import tracing_agent
...@@ -9,6 +11,10 @@ class ChromeTracingStartedError(Exception): ...@@ -9,6 +11,10 @@ class ChromeTracingStartedError(Exception):
pass pass
class ChromeTracingStoppedError(Exception):
pass
class ChromeTracingAgent(tracing_agent.TracingAgent): class ChromeTracingAgent(tracing_agent.TracingAgent):
# A singleton map from platform backends to maps of uniquely-identifying # A singleton map from platform backends to maps of uniquely-identifying
# remote port (which may be the same as local port) to DevToolsClientBackend. # remote port (which may be the same as local port) to DevToolsClientBackend.
...@@ -90,9 +96,21 @@ class ChromeTracingAgent(tracing_agent.TracingAgent): ...@@ -90,9 +96,21 @@ class ChromeTracingAgent(tracing_agent.TracingAgent):
def Stop(self, trace_data_builder): def Stop(self, trace_data_builder):
devtools_clients_map = ( devtools_clients_map = (
self._platform_backends_to_devtools_clients_maps[self._platform_backend]) self._platform_backends_to_devtools_clients_maps[self._platform_backend])
for _, devtools_client in devtools_clients_map.iteritems(): 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 # We do not check for stale DevTools client, so that we get an
# exception if there is a stale client. This is because we # exception if there is a stale client. This is because we
# will potentially lose data if there is a stale client. # will potentially lose data if there is a stale client.
try:
devtools_client.StopChromeTracing(trace_data_builder) 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 self._is_active = False
if raised_execption_messages:
raise ChromeTracingStoppedError(
'Exceptions raised when trying to stop devtool tracing\n:' +
'\n'.join(raised_execption_messages))
...@@ -16,6 +16,7 @@ class FakeDevtoolsClient(object): ...@@ -16,6 +16,7 @@ class FakeDevtoolsClient(object):
self.is_alive = True self.is_alive = True
self.tracing_started = False self.tracing_started = False
self.remote_port = remote_port self.remote_port = remote_port
self.will_raise_exception_in_stop_tracing = False
def IsAlive(self): def IsAlive(self):
return self.is_alive return self.is_alive
...@@ -25,6 +26,8 @@ class FakeDevtoolsClient(object): ...@@ -25,6 +26,8 @@ class FakeDevtoolsClient(object):
def StopChromeTracing(self, _trace_data_builder): def StopChromeTracing(self, _trace_data_builder):
self.tracing_started = False self.tracing_started = False
if self.will_raise_exception_in_stop_tracing:
raise Exception
def IsChromeTracingSupported(self): def IsChromeTracingSupported(self):
return True return True
...@@ -142,3 +145,31 @@ class ChromeTracingAgentUnittest(unittest.TestCase): ...@@ -142,3 +145,31 @@ class ChromeTracingAgentUnittest(unittest.TestCase):
self.assertTrue(devtool4.tracing_started) self.assertTrue(devtool4.tracing_started)
self.StopTracing(tracing_agent2) self.StopTracing(tracing_agent2)
self.assertFalse(devtool4.tracing_started) self.assertFalse(devtool4.tracing_started)
def testExceptionRaisedInStopTracing(self):
devtool1 = FakeDevtoolsClient(1)
devtool2 = FakeDevtoolsClient(2)
# Register devtools 1, 2 on platform 1
chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
devtool1, self.platform1)
chrome_tracing_agent.ChromeTracingAgent.RegisterDevToolsClient(
devtool2, self.platform1)
tracing_agent1 = self.StartTracing(self.platform1)
self.assertTrue(devtool1.tracing_started)
self.assertTrue(devtool2.tracing_started)
devtool2.will_raise_exception_in_stop_tracing = True
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(
devtool3, self.platform1)
# Start & Stop tracing on platform 1 should work just fine.
tracing_agent2 = self.StartTracing(self.platform1)
self.StopTracing(tracing_agent2)
...@@ -8,8 +8,7 @@ from telemetry.core.platform import tracing_options ...@@ -8,8 +8,7 @@ from telemetry.core.platform import tracing_options
from telemetry.unittest_util import tab_test_case from telemetry.unittest_util import tab_test_case
class TracingControllerTest(tab_test_case.TabTestCase): class TracingControllerTest(tab_test_case.TabTestCase):
# Disabled due to http://crbug.com/459807
@decorators.Disabled
@decorators.Isolated @decorators.Isolated
def testModifiedConsoleTime(self): def testModifiedConsoleTime(self):
category_filter = tracing_category_filter.TracingCategoryFilter() category_filter = tracing_category_filter.TracingCategoryFilter()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment