Handle multiple duplicate timeline interaction records.

BUG=

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282739 0039d316-1c4b-4281-b951-d872f2087c98

tools/telemetry/telemetry/page/actions/action_runner.py

@@ -29,7 +29,8 @@ class ActionRunner(object):
     action.WillRunAction(self._tab)
     action.RunAction(self._tab)
 
-  def BeginInteraction(self, label, is_smooth=False, is_responsive=False):
+  def BeginInteraction(self, label, is_smooth=False, is_responsive=False,
+                       repeatable=False):
     """Marks the beginning of an interaction record.
 
     An interaction record is a labeled time period containing
@@ -44,19 +45,24 @@ class ActionRunner(object):
       is_smooth: Whether to check for smoothness metrics for this interaction.
       is_responsive: Whether to check for responsiveness metrics for
           this interaction.
+      repeatable: Whether other interactions may use the same logical name
+          as this interaction. All interactions with the same logical name must
+          have the same flags.
     """
     flags = []
     if is_smooth:
       flags.append(tir_module.IS_SMOOTH)
     if is_responsive:
       flags.append(tir_module.IS_RESPONSIVE)
+    if repeatable:
+      flags.append(tir_module.REPEATABLE)
 
     interaction = Interaction(self._tab, label, flags)
     interaction.Begin()
     return interaction
 
-  def BeginGestureInteraction(
-      self, label, is_smooth=False, is_responsive=False):
+  def BeginGestureInteraction(self, label, is_smooth=False, is_responsive=False,
+                              repeatable=False):
     """Marks the beginning of a gesture-based interaction record.
 
     This is similar to normal interaction record, but it will
@@ -73,8 +79,12 @@ class ActionRunner(object):
       is_smooth: Whether to check for smoothness metrics for this interaction.
       is_responsive: Whether to check for responsiveness metrics for
           this interaction.
+      repeatable: Whether other interactions may use the same logical name
+          as this interaction. All interactions with the same logical name must
+          have the same flags.
     """
-    return self.BeginInteraction('Gesture_' + label, is_smooth, is_responsive)
+    return self.BeginInteraction('Gesture_' + label, is_smooth, is_responsive,
+                                 repeatable)
 
   def NavigateToPage(self, page, timeout_in_seconds=60):
     """Navigate to the given page.

tools/telemetry/telemetry/web_perf/timeline_based_measurement.py

@@ -5,12 +5,13 @@
 import logging
 import os
 
+from collections import defaultdict
 from telemetry.core import util
 from telemetry.core.backends.chrome import tracing_backend
 from telemetry.timeline import model as model_module
 from telemetry.web_perf import timeline_interaction_record as tir_module
-from telemetry.web_perf.metrics import smoothness
 from telemetry.web_perf.metrics import responsiveness_metric
+from telemetry.web_perf.metrics import smoothness
 from telemetry.page import page_measurement
 from telemetry.value import string as string_value_module
 
@@ -30,28 +31,43 @@ ALL_OVERHEAD_LEVELS = [
 ]
 
 
+class InvalidInteractions(Exception):
+  pass
+
+
+def _GetMetricFromMetricType(metric_type):
+  if metric_type == tir_module.IS_SMOOTH:
+    return smoothness.SmoothnessMetric()
+  if metric_type == tir_module.IS_RESPONSIVE:
+    return responsiveness_metric.ResponsivenessMetric()
+  raise Exception('Unrecognized metric type: %s' % metric_type)
+
+
 class _ResultsWrapper(object):
-  def __init__(self, results, interaction_record):
+  def __init__(self, results, logical_name):
     self._results = results
-    self._interaction_record = interaction_record
+    self._result_prefix = logical_name
+
+  def _GetResultName(self, trace_name):
+    return '%s-%s' % (self._result_prefix, trace_name)
 
   def Add(self, trace_name, units, value, chart_name=None, data_type='default'):
-    trace_name = self._interaction_record.GetResultNameFor(trace_name)
-    self._results.Add(trace_name, units, value, chart_name, data_type)
+    result_name = self._GetResultName(trace_name)
+    self._results.Add(result_name, units, value, chart_name, data_type)
 
   def AddSummary(self, trace_name, units, value, chart_name=None,
-                  data_type='default'):
-    trace_name = self._interaction_record.GetResultNameFor(trace_name)
-    self._results.AddSummary(trace_name, units, value, chart_name, data_type)
+                 data_type='default'):
+    result_name = self._GetResultName(trace_name)
+    self._results.AddSummary(result_name, units, value, chart_name, data_type)
 
 
 class _TimelineBasedMetrics(object):
   def __init__(self, model, renderer_thread,
-               create_metrics_for_interaction_record_callback):
+               get_metric_from_metric_type_callback):
     self._model = model
     self._renderer_thread = renderer_thread
-    self._create_metrics_for_interaction_record_callback = \
-        create_metrics_for_interaction_record_callback
+    self._get_metric_from_metric_type_callback = \
+        get_metric_from_metric_type_callback
 
   def FindTimelineInteractionRecords(self):
     # TODO(nduca): Add support for page-load interaction record.
@@ -60,16 +76,37 @@ class _TimelineBasedMetrics(object):
             if tir_module.IsTimelineInteractionRecord(event.name)]
 
   def AddResults(self, results):
-    interactions = self.FindTimelineInteractionRecords()
-    if len(interactions) == 0:
-      raise Exception('Expected at least one Interaction on the page')
-    for interaction in interactions:
-      metrics = \
-          self._create_metrics_for_interaction_record_callback(interaction)
-      wrapped_results = _ResultsWrapper(results, interaction)
-      for m in metrics:
-        m.AddResults(self._model, self._renderer_thread,
-                     [interaction], wrapped_results)
+    all_interactions = self.FindTimelineInteractionRecords()
+    if len(all_interactions) == 0:
+      raise InvalidInteractions('Expected at least one interaction record on '
+                                'the page')
+
+    interactions_by_logical_name = defaultdict(list)
+    for i in all_interactions:
+      interactions_by_logical_name[i.logical_name].append(i)
+
+    for logical_name, interactions in interactions_by_logical_name.iteritems():
+      are_repeatable = [i.repeatable for i in interactions]
+      if not all(are_repeatable) and len(interactions) > 1:
+        raise InvalidInteractions('Duplicate unrepeatable interaction records '
+                                  'on the page')
+      wrapped_results = _ResultsWrapper(results, logical_name)
+      self.UpdateResultsByMetric(interactions, wrapped_results)
+
+  def UpdateResultsByMetric(self, interactions, wrapped_results):
+    for metric_type in tir_module.METRICS:
+      # For each metric type, either all or none of the interactions should
+      # have that metric.
+      interactions_with_metric = [i for i in interactions if
+                                  i.HasMetric(metric_type)]
+      if not interactions_with_metric:
+        continue
+      if len(interactions_with_metric) != len(interactions):
+        raise InvalidInteractions('Interaction records with the same logical '
+                                  'name must have the same flags.')
+      metric = self._get_metric_from_metric_type_callback(metric_type)
+      metric.AddResults(self._model, self._renderer_thread,
+                        interactions, wrapped_results)
 
 
 class TimelineBasedMeasurement(page_measurement.PageMeasurement):
@@ -124,17 +161,6 @@ class TimelineBasedMeasurement(page_measurement.PageMeasurement):
     categories = ','.join([categories] + page.GetSyntheticDelayCategories())
     tab.browser.StartTracing(categories)
 
-  def CreateMetricsForTimelineInteractionRecord(self, interaction):
-    """ Subclass of TimelineBasedMeasurement overrides this method to customize
-    the binding of interaction's flags to metrics.
-    """
-    res = []
-    if interaction.is_smooth:
-      res.append(smoothness.SmoothnessMetric())
-    if interaction.is_responsive:
-      res.append(responsiveness_metric.ResponsivenessMetric())
-    return res
-
   def MeasurePage(self, page, tab, results):
     """ Collect all possible metrics and added them to results. """
     trace_result = tab.browser.StopTracing()
@@ -153,7 +179,7 @@ class TimelineBasedMeasurement(page_measurement.PageMeasurement):
     model = model_module.TimelineModel(trace_result)
     renderer_thread = model.GetRendererThreadFromTabId(tab.id)
     meta_metrics = _TimelineBasedMetrics(
-      model, renderer_thread, self.CreateMetricsForTimelineInteractionRecord)
+        model, renderer_thread, _GetMetricFromMetricType)
     meta_metrics.AddResults(results)
 
   def CleanUpAfterPage(self, page, tab):

tools/telemetry/telemetry/web_perf/timeline_based_measurement_unittest.py

@@ -7,96 +7,145 @@ import unittest
 
 from telemetry import benchmark
 from telemetry.core import wpr_modes
-from telemetry.timeline import model as model_module
-from telemetry.timeline import async_slice
+from telemetry.page import page as page_module
 from telemetry.page import page_measurement_unittest_base
 from telemetry.page import page_set
 from telemetry.page import page as page_module
 from telemetry.results import page_measurement_results
+from telemetry.timeline import async_slice
+from telemetry.timeline import model as model_module
 from telemetry.unittest import options_for_unittests
 from telemetry.web_perf import timeline_based_measurement as tbm_module
+from telemetry.web_perf import timeline_interaction_record as tir_module
 from telemetry.web_perf.metrics import timeline_based_metric
 
 
-class TimelineBasedMetricsTests(unittest.TestCase):
+class FakeSmoothMetric(timeline_based_metric.TimelineBasedMetric):
 
-  def setUp(self):
-    model = model_module.TimelineModel()
-    renderer_thread = model.GetOrCreateProcess(1).GetOrCreateThread(2)
-    renderer_thread.name = 'CrRendererMain'
-
-    # [      X       ]
-    #      [  Y  ]
-    renderer_thread.BeginSlice('cat1', 'x.y', 10, 0)
-    renderer_thread.EndSlice(20, 20)
-
-    renderer_thread.async_slices.append(async_slice.AsyncSlice(
-        'cat', 'Interaction.LogicalName1/is_smooth',
-        timestamp=0, duration=20,
-        start_thread=renderer_thread, end_thread=renderer_thread,
-        thread_start=5, thread_duration=15))
-    renderer_thread.async_slices.append(async_slice.AsyncSlice(
-        'cat', 'Interaction.LogicalName2/is_responsive',
-        timestamp=25, duration=5,
-        start_thread=renderer_thread, end_thread=renderer_thread,
-        thread_start=25, thread_duration=5))
-    model.FinalizeImport()
-
-    self.model = model
-    self.renderer_thread = renderer_thread
+  def AddResults(self, model, renderer_thread,
+                 interaction_records, results):
+    results.Add('FakeSmoothMetric', 'ms', 1)
+    results.Add('SmoothMetricRecords', 'count', len(interaction_records))
 
-  def testFindTimelineInteractionRecords(self):
-    metric = tbm_module._TimelineBasedMetrics(  # pylint: disable=W0212
-      self.model, self.renderer_thread, lambda _: [])
-    interactions = metric.FindTimelineInteractionRecords()
-    self.assertEquals(2, len(interactions))
-    self.assertTrue(interactions[0].is_smooth)
-    self.assertEquals(0, interactions[0].start)
-    self.assertEquals(20, interactions[0].end)
 
-    self.assertTrue(interactions[1].is_responsive)
-    self.assertEquals(25, interactions[1].start)
-    self.assertEquals(30, interactions[1].end)
+class FakeLoadingMetric(timeline_based_metric.TimelineBasedMetric):
 
-  def testAddResults(self):
-    results = page_measurement_results.PageMeasurementResults()
+  def AddResults(self, model, renderer_thread,
+                 interaction_records, results):
+    results.Add('FakeLoadingMetric', 'ms', 2)
+    results.Add('LoadingMetricRecords', 'count', len(interaction_records))
+
+
+def GetMetricFromMetricType(metric_type):
+  if metric_type == tir_module.IS_SMOOTH:
+    return FakeSmoothMetric()
+  if metric_type == tir_module.IS_RESPONSIVE:
+    return FakeLoadingMetric()
+  raise Exception('Unrecognized metric type: %s' % metric_type)
 
-    class FakeSmoothMetric(timeline_based_metric.TimelineBasedMetric):
 
-      def AddResults(self, model, renderer_thread,
-                     interaction_records, results):
-        results.Add('FakeSmoothMetric', 'ms', 1)
+class TimelineBasedMetricTestData(object):
 
-    class FakeLoadingMetric(timeline_based_metric.TimelineBasedMetric):
+  def __init__(self):
+    self._model = model_module.TimelineModel()
+    renderer_process = self._model.GetOrCreateProcess(1)
+    self._renderer_thread = renderer_process.GetOrCreateThread(2)
+    self._renderer_thread.name = 'CrRendererMain'
+    self._results = page_measurement_results.PageMeasurementResults()
+    self._metric = None
 
-      def AddResults(self, model, renderer_thread,
-                     interaction_records, results):
-        for r in interaction_records:
-          assert r.logical_name == 'LogicalName2'
-        results.Add('FakeLoadingMetric', 'ms', 2)
+  @property
+  def results(self):
+    return self._results
 
-    def CreateMetricsForTimelineInteractionRecord(interaction):
-      res = []
-      if interaction.is_smooth:
-        res.append(FakeSmoothMetric())
-      if interaction.is_responsive:
-        res.append(FakeLoadingMetric())
-      return res
+  @property
+  def metric(self):
+    return self._metric
 
-    metric = tbm_module._TimelineBasedMetrics(  # pylint: disable=W0212
-        self.model, self.renderer_thread,
-        CreateMetricsForTimelineInteractionRecord)
+  def AddInteraction(self, marker='', ts=0, duration=5):
+    self._renderer_thread.async_slices.append(async_slice.AsyncSlice(
+        'category', marker, timestamp=ts, duration=duration,
+        start_thread=self._renderer_thread, end_thread=self._renderer_thread,
+        thread_start=ts, thread_duration=duration))
+
+  def FinalizeImport(self):
+    self._model.FinalizeImport()
+    self._metric = tbm_module._TimelineBasedMetrics(  # pylint: disable=W0212
+        self._model, self._renderer_thread, GetMetricFromMetricType)
     ps = page_set.PageSet(file_path=os.path.dirname(__file__))
     ps.AddPageWithDefaultRunNavigate('http://www.bar.com/')
+    self._results.WillMeasurePage(ps.pages[0])
 
-    results.WillMeasurePage(ps.pages[0])
-    metric.AddResults(results)
-    results.DidMeasurePage()
+  def AddResults(self):
+    self._metric.AddResults(self._results)
+    self._results.DidMeasurePage()
 
-    v = results.FindAllPageSpecificValuesNamed('LogicalName1-FakeSmoothMetric')
-    self.assertEquals(len(v), 1)
-    v = results.FindAllPageSpecificValuesNamed('LogicalName2-FakeLoadingMetric')
-    self.assertEquals(len(v), 1)
+
+class TimelineBasedMetricsTests(unittest.TestCase):
+
+  def testFindTimelineInteractionRecords(self):
+    d = TimelineBasedMetricTestData()
+    d.AddInteraction(ts=0, duration=20,
+                     marker='Interaction.LogicalName1/is_smooth')
+    d.AddInteraction(ts=25, duration=5,
+                     marker='Interaction.LogicalName2/is_responsive')
+    d.FinalizeImport()
+    interactions = d.metric.FindTimelineInteractionRecords()
+    self.assertEquals(2, len(interactions))
+    self.assertTrue(interactions[0].is_smooth)
+    self.assertEquals(0, interactions[0].start)
+    self.assertEquals(20, interactions[0].end)
+
+    self.assertTrue(interactions[1].is_responsive)
+    self.assertEquals(25, interactions[1].start)
+    self.assertEquals(30, interactions[1].end)
+
+  def testAddResults(self):
+    d = TimelineBasedMetricTestData()
+    d.AddInteraction(ts=0, duration=20,
+                     marker='Interaction.LogicalName1/is_smooth')
+    d.AddInteraction(ts=25, duration=5,
+                     marker='Interaction.LogicalName2/is_responsive')
+    d.FinalizeImport()
+    d.AddResults()
+    self.assertEquals(1, len(d.results.FindAllPageSpecificValuesNamed(
+        'LogicalName1-FakeSmoothMetric')))
+    self.assertEquals(1, len(d.results.FindAllPageSpecificValuesNamed(
+        'LogicalName2-FakeLoadingMetric')))
+
+  def testNoInteractions(self):
+    d = TimelineBasedMetricTestData()
+    d.FinalizeImport()
+    self.assertRaises(tbm_module.InvalidInteractions, d.AddResults)
+
+  def testDuplicateUnrepeatableInteractions(self):
+    d = TimelineBasedMetricTestData()
+    d.AddInteraction(ts=10, duration=5,
+                     marker='Interaction.LogicalName/is_smooth')
+    d.AddInteraction(ts=20, duration=5,
+                     marker='Interaction.LogicalName/is_smooth')
+    d.FinalizeImport()
+    self.assertRaises(tbm_module.InvalidInteractions, d.AddResults)
+
+  def testDuplicateRepeatableInteractions(self):
+    d = TimelineBasedMetricTestData()
+    d.AddInteraction(ts=10, duration=5,
+                     marker='Interaction.LogicalName/is_smooth,repeatable')
+    d.AddInteraction(ts=20, duration=5,
+                     marker='Interaction.LogicalName/is_smooth,repeatable')
+    d.FinalizeImport()
+    d.AddResults()
+    self.assertEquals(1, len(d.results.pages_that_succeeded))
+
+  def testDuplicateRepeatableInteractionsWithDifferentMetrics(self):
+    d = TimelineBasedMetricTestData()
+
+    responsive_marker = 'Interaction.LogicalName/is_responsive,repeatable'
+    d.AddInteraction(ts=10, duration=5, marker=responsive_marker)
+    smooth_marker = 'Interaction.LogicalName/is_smooth,repeatable'
+    d.AddInteraction(ts=20, duration=5, marker=smooth_marker)
+    d.FinalizeImport()
+    self.assertRaises(tbm_module.InvalidInteractions, d.AddResults)
 
 
 class TestTimelinebasedMeasurementPage(page_module.Page):

tools/telemetry/telemetry/web_perf/timeline_interaction_record.py

@@ -8,13 +8,18 @@ from telemetry import decorators
 import telemetry.timeline.bounds as timeline_bounds
 
 
+# Enables the smoothness metric for this interaction
 IS_SMOOTH = 'is_smooth'
+# Enables the responsiveness metric for this interaction
 IS_RESPONSIVE = 'is_responsive'
+# Allows multiple duplicate interactions of the same type
+REPEATABLE = 'repeatable'
 
-FLAGS = [
-    IS_SMOOTH,
-    IS_RESPONSIVE
+METRICS = [
+    IS_RESPONSIVE,
+    IS_SMOOTH
 ]
+FLAGS = METRICS + [REPEATABLE]
 
 
 class ThreadTimeRangeOverlappedException(Exception):
@@ -29,6 +34,12 @@ class NoThreadTimeDataException(ThreadTimeRangeOverlappedException):
 def IsTimelineInteractionRecord(event_name):
   return event_name.startswith('Interaction.')
 
+def _AssertFlagsAreValid(flags):
+  assert isinstance(flags, list)
+  for f in flags:
+    if f not in FLAGS:
+      raise AssertionError(
+          'Unrecognized flag for a timeline Interaction record: %s' % f)
 
 class TimelineInteractionRecord(object):
   """Represents an interaction that took place during a timeline recording.
@@ -66,6 +77,10 @@ class TimelineInteractionRecord(object):
   smoothness and network metrics to be reported for the marked up 1000ms
   time-range.
 
+  The valid interaction flags are:
+     * is_smooth: Enables the smoothness metrics
+     * is_responsive: Enables the responsiveness metrics
+     * repeatable: Allows other interactions to use the same logical name
   """
 
   def __init__(self, logical_name, start, end, async_event=None):
@@ -75,6 +90,7 @@ class TimelineInteractionRecord(object):
     self.end = end
     self.is_smooth = False
     self.is_responsive = False
+    self.repeatable = False
     self._async_event = async_event
 
   # TODO(nednguyen): After crbug.com/367175 is marked fixed, we should be able
@@ -105,17 +121,12 @@ class TimelineInteractionRecord(object):
 
     record = TimelineInteractionRecord(logical_name, async_event.start,
                                        async_event.end, async_event)
-    for f in flags:
-      if not f in FLAGS:
-        raise Exception(
-            'Unrecognized flag in timeline Interaction record: %s' % f)
+    _AssertFlagsAreValid(flags)
     record.is_smooth = IS_SMOOTH in flags
     record.is_responsive = IS_RESPONSIVE in flags
+    record.repeatable = REPEATABLE in flags
     return record
 
-  def GetResultNameFor(self, result_name):
-    return '%s-%s' % (self.logical_name, result_name)
-
   @decorators.Cache
   def GetBounds(self):
     bounds = timeline_bounds.Bounds()
@@ -128,13 +139,15 @@ class TimelineInteractionRecord(object):
     """ Get the marker string of an interaction record with logical_name
     and flags.
     """
-    assert isinstance(flags, list)
-    for f in flags:
-      if f not in FLAGS:
-        raise AssertionError(
-            'Unrecognized flag for a timeline Interaction record: %s' % f)
+    _AssertFlagsAreValid(flags)
     return 'Interaction.%s/%s' % (logical_name, ','.join(flags))
 
+  def HasMetric(self, metric_type):
+    if metric_type not in METRICS:
+      raise AssertionError('Unrecognized metric type for a timeline '
+                           'interaction record: %s' % metric_type)
+    return getattr(self, metric_type)
+
   def GetOverlappedThreadTimeForSlice(self, timeline_slice):
     """Get the thread duration of timeline_slice that overlaps with this record.