Revert 193195 "[Telemetry] Make page cyclers more telemetric."

Page cyclers are hanging on the bots.

> [Telemetry] Make page cyclers more telemetric.
> 
> This causes the page cyclers to use page_sets instead of just navigating to the
> start page and letting the pages cycle themselves. This means we'll be using
> Telemetry "properly" which will allow us to record new page sets to update the
> page cyclers and it means that page cycler page sets are interchangeable with
> other benchmarks.
> 
> The page cyclers still measure the same thing as they measured previously: the
> time to load the page and perform a layout. However, they measure it differently
> now. Instead of setting a new Date().getTime() in the cookie immediately before
> navigating and then grabbing the finish Date in the onload handler, this now
> just grabs performance.now() in the onload handler. This returns the number of
> milliseconds since navigationStart where navigation start is the same as the
> immediately before navigation time marked before. I've verified locally that the
> times reported are consistent, and will monitor the bots to verify.
> 
> This also involved a couple of minor telemetry changes:
> 1) Move the serving_dirs property up from the page to the page_set.
> 2) Move WillRunPageSet to a point at which browser.http_server is valid.
> 3) The display URL was slightly wrong before: "foo.html" and "foo1.html" would
>    return ".html" and "1.html". This fixes the method to properly strip just
>    the common directory prefix, not the common prefix of the file.
> 
> BUG=None
> TEST=All page cyclers on linux
> NOTRY=True
> 
> Review URL: https://chromiumcodereview.appspot.com/13467015

TBR=tonyg@chromium.org
Review URL: https://codereview.chromium.org/13955004

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

4 files changed, 24 insertions, 74 deletions

diff --git a/tools/telemetry/telemetry/page/page.py b/tools/telemetry/telemetry/page/page.py
index 1ec09e0..92272ad 100644
--- a/tools/telemetry/telemetry/page/page.py
+++ b/tools/telemetry/telemetry/page/page.py
@@ -51,10 +51,10 @@ class Page(object):
 
     path = self.base_dir + parsed_url.netloc + parsed_url.path
 
-    if hasattr(self.page_set, 'serving_dirs'):
-      url_base_dir = os.path.commonprefix(self.page_set.serving_dirs)
+    if hasattr(self, 'serving_dirs'):
+      url_base_dir = os.path.commonprefix(self.serving_dirs)
       base_path = self.base_dir + '/' + url_base_dir
-      return ([self.base_dir + '/' + d for d in self.page_set.serving_dirs],
+      return ([self.base_dir + '/' + d for d in self.serving_dirs],
               path.replace(base_path, ''))
 
     return os.path.split(path)
@@ -67,13 +67,15 @@ class Page(object):
 
   @property
   def display_url(self):
-    if self.url.startswith('http'):
-      return self.url
-    url_paths = ['/'.join(p.url.strip('/').split('/')[:-1])
-                 for p in self.page_set
-                 if p.url.startswith('file://')]
-    common_prefix = os.path.commonprefix(url_paths)
-    return self.url[len(common_prefix):].strip('/')
+    file_urls = [p.url for p in self.page_set if p.url.startswith('file://')]
+    common_prefix = ''
+    if len(file_urls) > 1:
+      common_prefix = os.path.commonprefix(file_urls)
+    url = self.url
+    # Trim trailing slash from file URLs.
+    if url.startswith('file://') and url.endswith('/'):
+      url = url[:-1]
+    return url[len(common_prefix):]
 
   @property
   def archive_path(self):
diff --git a/tools/telemetry/telemetry/page/page_benchmark_results.py b/tools/telemetry/telemetry/page/page_benchmark_results.py
index 32eab68..b13e2b9 100644
--- a/tools/telemetry/telemetry/page/page_benchmark_results.py
+++ b/tools/telemetry/telemetry/page/page_benchmark_results.py
@@ -123,14 +123,12 @@ class PageBenchmarkResults(page_test.PageTestResults):
     if self.page_failures:
       return
 
-    # Print out the list of unique pages.
     unique_page_urls = []
     for page_values in self._page_results:
       url = page_values.page.display_url
       if unique_page_urls and unique_page_urls[0] == url:
         break
       unique_page_urls.append(url)
-    print 'Pages: [%s]' % ','.join(unique_page_urls)
 
     # Build the results summary.
     results_summary = defaultdict(list)
diff --git a/tools/telemetry/telemetry/page/page_runner.py b/tools/telemetry/telemetry/page/page_runner.py
index 99c44b2..ab24745 100644
--- a/tools/telemetry/telemetry/page/page_runner.py
+++ b/tools/telemetry/telemetry/page/page_runner.py
@@ -60,7 +60,6 @@ class PageRunner(object):
   """Runs a given test against a given test."""
   def __init__(self, page_set):
     self.page_set = page_set
-    self.has_called_will_run_page_set = False
 
   def __enter__(self):
     return self
@@ -142,6 +141,7 @@ class PageRunner(object):
 
     state = _RunState()
     last_archive_path = None
+    is_first_run = True
     results_for_current_run = out_results
 
     try:
@@ -155,8 +155,7 @@ class PageRunner(object):
           state.Close()
           state = _RunState()
           last_archive_path = page.archive_path
-        if (test.discard_first_result and
-            not self.has_called_will_run_page_set):
+        if (test.discard_first_result and is_first_run):
           # If discarding results, substitute a dummy object.
           results_for_current_run = (
             page_benchmark_results.PageBenchmarkResults())
@@ -182,6 +181,10 @@ class PageRunner(object):
 
             self._WaitForThermalThrottlingIfNeeded(state.browser.platform)
 
+            if is_first_run:
+              is_first_run = False
+              test.WillRunPageSet(state.tab, results_for_current_run)
+
             try:
               self._RunPage(options, page, state.tab, test,
                             results_for_current_run)
@@ -336,10 +339,6 @@ class PageRunner(object):
         results.AddFailure(page, msg, "")
         return False
 
-    if not self.has_called_will_run_page_set:
-      self.has_called_will_run_page_set = True
-      test.WillRunPageSet(tab, results)
-
     test.WillNavigateToPage(page, tab)
     tab.Navigate(target_side_url, page.script_to_evaluate_on_commit)
     test.DidNavigateToPage(page, tab)
diff --git a/tools/telemetry/telemetry/page/page_unittest.py b/tools/telemetry/telemetry/page/page_unittest.py
index 9ef41ac..e1c6e24 100644
--- a/tools/telemetry/telemetry/page/page_unittest.py
+++ b/tools/telemetry/telemetry/page/page_unittest.py
@@ -25,14 +25,11 @@ class TestPage(unittest.TestCase):
     self.assertEqual(filename, 'file.html')
 
   def testGetUrlBaseDirAndFileForUrlBaseDir(self):
-    ps = page_set.PageSet.FromDict({
-        'description': 'hello',
-        'archive_path': 'foo.wpr',
-        'serving_dirs': ['../../somedir/'],
-        'pages': [
-          {'url': 'file:///../../somedir/otherdir/file.html'}
-        ]}, 'basedir/')
-    serving_dirs, filename = ps[0].serving_dirs_and_file
+    apage = page.Page('file:///../../somedir/otherdir/file.html',
+                      None, # In this test, we don't need a page set.
+                      base_dir='basedir')
+    setattr(apage, 'serving_dirs', ['../../somedir/'])
+    serving_dirs, filename = apage.serving_dirs_and_file
     self.assertEqual(serving_dirs, ['basedir/../../somedir/'])
     self.assertEqual(filename, 'otherdir/file.html')
 
@@ -48,18 +45,6 @@ class TestPage(unittest.TestCase):
     self.assertEquals(ps[0].display_url, 'http://www.foo.com/')
     self.assertEquals(ps[1].display_url, 'http://www.bar.com/')
 
-  def testDisplayUrlForHttps(self):
-    ps = page_set.PageSet.FromDict({
-      "description": "hello",
-      "archive_path": "foo.wpr",
-      "pages": [
-        {"url": "http://www.foo.com/"},
-        {"url": "https://www.bar.com/"}
-        ]
-      }, os.path.dirname(__file__))
-    self.assertEquals(ps[0].display_url, 'http://www.foo.com/')
-    self.assertEquals(ps[1].display_url, 'https://www.bar.com/')
-
   def testDisplayUrlForFile(self):
     ps = page_set.PageSet.FromDict({
       "description": "hello",
@@ -72,30 +57,6 @@ class TestPage(unittest.TestCase):
     self.assertEquals(ps[0].display_url, 'foo.html')
     self.assertEquals(ps[1].display_url, 'bar.html')
 
-  def testDisplayUrlForFilesDifferingBySuffix(self):
-    ps = page_set.PageSet.FromDict({
-      "description": "hello",
-      "archive_path": "foo.wpr",
-      "pages": [
-        {"url": "file:///../../otherdir/foo.html"},
-        {"url": "file:///../../otherdir/foo1.html"},
-        ]
-      }, os.path.dirname(__file__))
-    self.assertEquals(ps[0].display_url, 'foo.html')
-    self.assertEquals(ps[1].display_url, 'foo1.html')
-
-  def testDisplayUrlForFileOfDifferentPaths(self):
-    ps = page_set.PageSet.FromDict({
-      "description": "hello",
-      "archive_path": "foo.wpr",
-      "pages": [
-        {"url": "file:///../../somedir/foo.html"},
-        {"url": "file:///../../otherdir/bar.html"},
-        ]
-      }, os.path.dirname(__file__))
-    self.assertEquals(ps[0].display_url, 'somedir/foo.html')
-    self.assertEquals(ps[1].display_url, 'otherdir/bar.html')
-
   def testDisplayUrlForFileDirectories(self):
     ps = page_set.PageSet.FromDict({
       "description": "hello",
@@ -116,14 +77,4 @@ class TestPage(unittest.TestCase):
         {"url": "file:///../../otherdir/foo.html"},
         ]
       }, os.path.dirname(__file__))
-    self.assertEquals(ps[0].display_url, 'foo.html')
-
-  def testDisplayUrlForSingleDirectory(self):
-    ps = page_set.PageSet.FromDict({
-      "description": "hello",
-      "archive_path": "foo.wpr",
-      "pages": [
-        {"url": "file:///../../otherdir/foo/"},
-        ]
-      }, os.path.dirname(__file__))
-    self.assertEquals(ps[0].display_url, 'foo')
+    self.assertEquals(ps[0].display_url, 'file:///../../otherdir/foo.html')