From fa03ae14b0be19c2cb9d653b2b1d3cfa0ae25117 Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Mon, 8 Jan 2018 11:22:15 -0800
Subject: [PATCH] fix(benchpress): work around missing events from Chrome 63
 (#21396)

Chrome 63 can cause the navigationStart event for the first
run to arrive with a different pid than the start of the
benchpress run. This makes the first collected result invalid.

This workaround causes the sampler to ignore runs that have this
condition.

PR Close #21396
---
 .../benchpress/src/metric/perflog_metric.ts    |  3 +++
 packages/benchpress/src/sampler.ts             |  7 ++++++-
 .../test/metric/perflog_metric_spec.ts         | 18 ++++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/packages/benchpress/src/metric/perflog_metric.ts b/packages/benchpress/src/metric/perflog_metric.ts
index 033c1b6da5..ac285e7a95 100644
--- a/packages/benchpress/src/metric/perflog_metric.ts
+++ b/packages/benchpress/src/metric/perflog_metric.ts
@@ -249,6 +249,9 @@ export class PerflogMetric extends Metric {
       // not all events have been received, no further processing for now
       return null;
     }
+    if (markStartEvent.pid !== markEndEvent.pid) {
+      result['invalid'] = 1;
+    }
 
     let gcTimeInScript = 0;
     let renderTimeInScript = 0;
diff --git a/packages/benchpress/src/sampler.ts b/packages/benchpress/src/sampler.ts
index bd9d0fc868..720fe621a5 100644
--- a/packages/benchpress/src/sampler.ts
+++ b/packages/benchpress/src/sampler.ts
@@ -63,7 +63,12 @@ export class Sampler {
     }
     return resultPromise.then((_) => this._driver.waitFor(this._execute))
         .then((_) => this._metric.endMeasure(this._prepare === Options.NO_PREPARE))
-        .then((measureValues) => this._report(lastState, measureValues));
+        .then((measureValues) => {
+          if (!!measureValues['invalid']) {
+            return lastState;
+          }
+          return this._report(lastState, measureValues);
+        });
   }
 
   private _report(state: SampleState, metricValues: {[key: string]: any}): Promise<SampleState> {
diff --git a/packages/benchpress/test/metric/perflog_metric_spec.ts b/packages/benchpress/test/metric/perflog_metric_spec.ts
index bd96f15215..7c068403a7 100644
--- a/packages/benchpress/test/metric/perflog_metric_spec.ts
+++ b/packages/benchpress/test/metric/perflog_metric_spec.ts
@@ -537,6 +537,24 @@ import {TraceEventFactory} from '../trace_event_factory';
            });
          }));
 
+      it('should mark a run as invalid if the start and end marks are different',
+         inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
+           const otherProcessEventFactory = new TraceEventFactory('timeline', 'pid1');
+           const metric = createMetric(
+               [[
+                 eventFactory.markStart('benchpress0', 0), eventFactory.start('script', 0, null),
+                 eventFactory.end('script', 5, null),
+                 otherProcessEventFactory.start('script', 10, null),
+                 otherProcessEventFactory.end('script', 17, null),
+                 otherProcessEventFactory.markEnd('benchpress0', 20)
+               ]],
+               null !);
+           metric.beginMeasure().then((_) => metric.endMeasure(false)).then((data) => {
+             expect(data['invalid']).toBe(1);
+             async.done();
+           });
+         }));
+
       it('should support scriptTime metric',
          inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
            aggregate([