Cleanup of page_measurement_results object

tools/telemetry/telemetry/page/page_measurement_results.py

Index: tools/telemetry/telemetry/page/page_measurement_results.py
diff --git a/tools/telemetry/telemetry/page/page_measurement_results.py b/tools/telemetry/telemetry/page/page_measurement_results.py
index 02ebd2a03b48bfebecc360996074fedc6a0043c3..7db06d7f2582894327362af959d11f9b17d26c84 100644
--- a/tools/telemetry/telemetry/page/page_measurement_results.py
+++ b/tools/telemetry/telemetry/page/page_measurement_results.py
@@ -1,115 +1,90 @@
-# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Copyright (c) 2013 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.
 
 from telemetry.page import page_test_results
-from telemetry.page import page_measurement_value
-
-class ValuesForSinglePage(object):
-  def __init__(self, page):
-    self.page = page
-    self.values = []
-
-  def AddValue(self, value):
-    self.values.append(value)
-
-  @property
-  def measurement_names(self):
-    return [value.measurement_name for value in self.values]
-
-  def FindValueByMeasurementName(self, measurement_name):
-    values = [value for value in self.values
-              if value.measurement_name == measurement_name]
-    assert len(values) <= 1
-    if len(values):
-      return values[0]
-    return None
-
-  def __getitem__(self, trace_name):
-    return self.FindValueByTraceName(trace_name)
-
-  def __contains__(self, trace_name):
-    return self.FindValueByTraceName(trace_name) != None
-
-  def FindValueByTraceName(self, trace_name):
-    values = [value for value in self.values
-              if value.trace_name == trace_name]
-    assert len(values) <= 1
-    if len(values):
-      return values[0]
-    return None
+from telemetry.page import value as value_module
+from telemetry.page import value_backcompat
 
 class PageMeasurementResults(page_test_results.PageTestResults):
   def __init__(self, trace_tag=''):
     super(PageMeasurementResults, self).__init__()
+    self._done = False
     self._trace_tag = trace_tag
-    self._page_results = []
-    self._overall_results = []
 
-    self._all_measurements_that_have_been_seen = {}
+    self._current_page = None
+    self._page_specific_values_for_current_page = None
 
-    self._values_for_current_page = {}
+    self._representative_values_for_each_value_name = {}
 
-  def __getitem__(self, i):
-    """Shorthand for self.page_results[i]"""
-    return self._page_results[i]
+    self._all_summary_values = []
+    self._all_page_specific_values = []
 
-  def __len__(self):
-    return len(self._page_results)
+  @property
+  def pages_that_succeeded(self):
+    pages = set([value.page for value in self._all_page_specific_values])
+    pages.difference_update(self.pages_that_had_errors_or_failures)

[dtu, 2013/11/01 00:16:31]

You never use pages_that_had_errors_or_failures directly. Maybe you should get a
list of successes instead. Or do [value.page for value in
self._all_page_specific_values if value.page.url in self.successes]

+    return pages
 
   @property
-  def values_for_current_page(self):
-    return self._values_for_current_page
+  def current_page(self):
+    return self._current_page
 
   @property
-  def page_results(self):
-    return self._page_results
+  def all_value_names_that_have_been_seen(self):
+    return list(self._representative_values_for_each_value_name.keys())
 
-  def WillMeasurePage(self, page):
-    self._values_for_current_page = ValuesForSinglePage(page)
+  def GetUnitsForValueName(self, value_name):
+    return self._representative_values_for_each_value_name[value_name].units
+
+  @property
+  def all_page_specific_values(self):
+    return self._all_page_specific_values
 
   @property
-  def all_measurements_that_have_been_seen(self):
-    return self._all_measurements_that_have_been_seen
+  def page_specific_values_for_current_page(self):
+    assert self._current_page
+    return self._page_specific_values_for_current_page
+
+  def GetAllPageSpecificValuesForSuccessfulPages(self):
+    pages_that_had_errors_or_failures = self.pages_that_had_errors_or_failures
+    return [
+      value for value in self._all_page_specific_values
+      if value.page not in pages_that_had_errors_or_failures]

[dtu, 2013/11/01 00:16:31]

use self.pages_that_succeeded (defined above), instead.

+
+  def WillMeasurePage(self, page):
+    assert not self._current_page
+    self._current_page = page
+    self._page_specific_values_for_current_page = []
 
   def Add(self, trace_name, units, value, chart_name=None, data_type='default'):
-    value = self._GetPageMeasurementValue(trace_name, units, value, chart_name,
-                                        data_type)
-    self._values_for_current_page.AddValue(value)
+    value = value_backcompat.ConvertOldCallingConventionToValue(
+      self._current_page,
+      trace_name, units, value, chart_name, data_type)
+    self._ValidateValue(value)
+    self._page_specific_values_for_current_page.append(value)
+    self._all_page_specific_values.append(value)
 
   def AddSummary(self, trace_name, units, value, chart_name=None,
                  data_type='default'):
-    value = self._GetPageMeasurementValue(trace_name, units, value, chart_name,
-                                        data_type)
-    self._overall_results.append(value)
-
-  def _GetPageMeasurementValue(self, trace_name, units, value, chart_name,
-                             data_type):
-    value = page_measurement_value.PageMeasurementValue(
-        trace_name, units, value, chart_name, data_type)
-    measurement_name = value.measurement_name
-
-    # Sanity checks.
-    assert measurement_name != 'url', 'The name url cannot be used'
-    if measurement_name in self._all_measurements_that_have_been_seen:
-      measurement_data = \
-          self._all_measurements_that_have_been_seen[measurement_name]
-      last_seen_units = measurement_data['units']
-      last_seen_data_type = measurement_data['type']
-      assert last_seen_units == units, \
-          'Unit cannot change for a name once it has been provided'
-      assert last_seen_data_type == data_type, \
-          'Unit cannot change for a name once it has been provided'
-    else:
-      self._all_measurements_that_have_been_seen[measurement_name] = {
-        'units': units,
-        'type': data_type}
-    return value
+    value = value_backcompat.ConvertOldCallingConventionToValue(
+      None,
+      trace_name, units, value, chart_name, data_type)
+    self._ValidateValue(value)
+    self._all_summary_values.append(value)
+
+  def _ValidateValue(self, value):
+    assert isinstance(value, value_module.Value)
+
+    if value.name not in self._representative_values_for_each_value_name:
+      self._representative_values_for_each_value_name[value.name] = value
+
+    representative_value = self._representative_values_for_each_value_name[
+        value.name]
+
+    assert value.IsMergableWith(representative_value)
 
   def DidMeasurePage(self):
-    assert self._values_for_current_page, 'Failed to call WillMeasurePage'
-    if not self.values_for_current_page.values:
-      # Do not store to page_results list if no results were added on this page.
-      return
-    self._page_results.append(self._values_for_current_page)
-    self._values_for_current_page = None
+    assert self._current_page, 'Failed to call WillMeasurePage'
+    self._current_page = None
+    self._page_specific_values_for_current_page = None