Address all review comments

lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js

@@ -6,26 +6,24 @@
 'use strict';
 
 const Audit = require('../audit');
-const i18n = require('../../lib/i18n/i18n.js');
 const CumulativeLQD = require('../../computed/metrics/cumulative-long-queuing-delay.js');
 
-const UIStrings = {
+// TODO(deepanjanroy): i18n strings once metric is final.
+const UIStringsNotExported = {
   title: 'Cumulative Long Queuing Delay',
-  description: '[Experimental metric]. Sum of Task Lengths beyond 50ms, between ' +
-      'First Contentful Paint and Time To Interactive.',
+  description: '[Experimental metric] Total time period between FCP and Time to Interactive ' +
+      'during which queuing time for any input event would be higher than 50ms.',
 };
 
-const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
-
 class CumulativeLongQueuingDelay extends Audit {
   /**
    * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
       id: 'cumulative-long-queuing-delay',
-      title: str_(UIStrings.title),
-      description: str_(UIStrings.description),
+      title: UIStringsNotExported.title,
+      description: UIStringsNotExported.description,
       scoreDisplayMode: Audit.SCORING_MODES.NUMERIC,
       requiredArtifacts: ['traces', 'devtoolsLogs'],
     };
@@ -36,13 +34,14 @@ class CumulativeLongQueuingDelay extends Audit {
    */
   static get defaultOptions() {
     return {
-      // Generally, the scoreMedian and scorePODR value is set to be real world 25th/75th and
-      // 5th/95th percentile value respectively. According to a cluster telemetry run over top 10k
-      // sites on mobile, 25-th percentile was 270ms, 10-th percentile was 22ms, and 5th percentile
-      // was 0ms. These numbers include 404 pages, so rounding up the scoreMedian to 300ms and
-      // picking 25ms as PODR. See curve at https://www.desmos.com/calculator/x3nzenjyln
-      scoreMedian: 300,
-      scorePODR: 25,
+      // According to a cluster telemetry run over top 10k sites on mobile, 5th percentile was 0ms,
+      // 25th percentile was 270ms and median was 895ms. These numbers include 404 pages. Picking
+      // thresholds according to our 25/75-th rule will be quite harsh scoring (a single 350ms task)
+      // after FCP will yield a score of .5. The following coefficients are semi-arbitrarily picked
+      // to give 600ms jank a score of .5 and 100ms jank a score of .999. We can tweak these numbers
+      // in the future. See https://www.desmos.com/calculator/a7ib75kq3g
+      scoreMedian: 600,
+      scorePODR: 200,
     };
   }
 
@@ -72,10 +71,9 @@ class CumulativeLongQueuingDelay extends Audit {
         context.options.scoreMedian
       ),
       numericValue: metricResult.timing,
-      displayValue: str_(i18n.UIStrings.ms, {timeInMs: metricResult.timing}),
+      displayValue: 10 * Math.round(metricResult.timing / 10) + '\xa0ms',
     };
   }
 }
 
 module.exports = CumulativeLongQueuingDelay;
-module.exports.UIStrings = UIStrings;

lighthouse-core/computed/metrics/cumulative-long-queuing-delay.js

@@ -20,6 +20,11 @@ const TimetoInteractive = require('./interactive.js');
  * task, the first 60ms of it is Long Queuing Delay Region, because any input event occuring in
  * that region has to wait more than 50ms. Cumulative Long Queuing Delay is the sum of all Long
  * Queuing Delay Regions between First Contentful Paint and Interactive Time (TTI).
+ *
+ * This is a new metric designed to accompany Time to Interactive. TTI is strict and does not
+ * reflect incremental improvements to the site performance unless the improvement concerns the last
+ * long task. Cumulative Long Queuing Delay on the other hand is designed to be much more responsive
+ * to smaller improvements to main thread responsiveness.
  */
 class CumulativeLongQueuingDelay extends ComputedMetric {
   /**
@@ -35,6 +40,8 @@ class CumulativeLongQueuingDelay extends ComputedMetric {
    * @return {number}
    */
   static calculateSumOfLongQueuingDelay(topLevelEvents, fcpTimeInMs, interactiveTimeMs) {
+    if (interactiveTimeMs <= fcpTimeInMs) return 0;
+
     const threshold = CumulativeLongQueuingDelay.LONG_QUEUING_DELAY_THRESHOLD;
     const longQueuingDelayRegions = [];
     // First identifying the long queuing delay regions.

lighthouse-core/lib/i18n/en-US.json

@@ -671,14 +671,6 @@
     "message": "Minimizes main-thread work",
     "description": "Title of a diagnostic audit that provides detail on the main thread work the browser did to load the page. This descriptive title is shown to users when the amount is acceptable and no user action is required."
   },
-  "lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js | description": {
-    "message": "[Experimental metric]. Sum of Task Lengths beyond 50ms, between First Contentful Paint and Time To Interactive.",
-    "description": ""
-  },
-  "lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js | title": {
-    "message": "Cumulative Long Queuing Delay",
-    "description": ""
-  },
   "lighthouse-core/audits/metrics/estimated-input-latency.js | description": {
     "message": "Estimated Input Latency is an estimate of how long your app takes to respond to user input, in milliseconds, during the busiest 5s window of page load. If your latency is higher than 50 ms, users may perceive your app as laggy. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/estimated-input-latency).",
     "description": "Description of the Estimated Input Latency metric that estimates the amount of time, in milliseconds, that the app takes to respond to user input. This description is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. 'Learn More' becomes link text to additional documentation."

lighthouse-core/test/audits/metrics/cumulative-long-queuing-delay-test.js

@@ -19,15 +19,14 @@ function generateArtifactsWithTrace(trace) {
 /* eslint-env jest */
 
 describe('Performance: cumulative-long-queuing-delay audit', () => {
-  it('evaluates cumulative long queuing delay metric properly', () => {
+  it('evaluates cumulative long queuing delay metric properly', async () => {
     const artifacts = generateArtifactsWithTrace(pwaTrace);
     const settings = {throttlingMethod: 'provided'};
     const context = {options, settings, computedCache: new Map()};
+    const output = await Audit.audit(artifacts, context);
 
-    return Audit.audit(artifacts, context).then(output => {
-      expect(output.numericValue).toBeCloseTo(48.3, 1);
-      expect(output.score).toBeCloseTo(0.97, 2);
-      expect(output.displayValue).toBeDisplayString('50\xa0ms');
-    });
+    expect(output.numericValue).toBeCloseTo(48.3, 1);
+    expect(output.score).toBeCloseTo(1, 2);
+    expect(output.displayValue).toBeDisplayString('50\xa0ms');
   });
 });

lighthouse-core/test/computed/metrics/cumulative-long-queuing-delay-test.js

@@ -45,7 +45,7 @@ Object {
   });
 
   describe('#calculateSumOfLongQueuingDelay', () => {
-    it('reports 0 when no task is longer than 50ms', async () => {
+    it('reports 0 when no task is longer than 50ms', () => {
       const events = [
         {start: 1000, end: 1050, duration: 50},
         {start: 2000, end: 2010, duration: 10},
@@ -85,7 +85,7 @@ Object {
 
       const events = [
         {start: 1000, end: 1110, duration: 110}, // Contributes 10ms.
-        {start: 2000, end: 2100, duration: 100}, // Contributes 50ms.
+        {start: 2000, end: 2200, duration: 200}, // Contributes 50ms.
       ];
 
       expect(CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay(
@@ -94,5 +94,22 @@ Object {
         interactiveTimeMs
       )).toBe(60);
     });
+
+    // This can happen in the lantern metric case, where we use the optimistic
+    // TTI and pessimistic FCP.
+    it('returns 0 if interactiveTime is earlier than FCP', () => {
+      const fcpTimeMs = 2050;
+      const interactiveTimeMs = 1050;
+
+      const events = [
+        {start: 500, end: 3000, duration: 2500},
+      ];
+
+      expect(CumulativeLongQueuingDelay.calculateSumOfLongQueuingDelay(
+        events,
+        fcpTimeMs,
+        interactiveTimeMs
+      )).toBe(0);
+    });
   });
 });

lighthouse-core/test/results/sample_v2.json

@@ -194,8 +194,8 @@
     "cumulative-long-queuing-delay": {
       "id": "cumulative-long-queuing-delay",
       "title": "Cumulative Long Queuing Delay",
-      "description": "[Experimental metric]. Sum of Task Lengths beyond 50ms, between First Contentful Paint and Time To Interactive.",
-      "score": 0.83,
+      "description": "[Experimental metric] Total time period between FCP and Time to Interactive during which queuing time for any input event would be higher than 50ms.",
+      "score": 1,
       "scoreDisplayMode": "numeric",
       "numericValue": 116.79800000000023,
       "displayValue": "120 ms"
@@ -5105,12 +5105,6 @@
           },
           "path": "audits[estimated-input-latency].displayValue"
         },
-        {
-          "values": {
-            "timeInMs": 116.79800000000023
-          },
-          "path": "audits[cumulative-long-queuing-delay].displayValue"
-        },
         {
           "values": {
             "timeInMs": 122.537
@@ -5130,12 +5124,6 @@
           "path": "audits[network-server-latency].displayValue"
         }
       ],
-      "lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js | title": [
-        "audits[cumulative-long-queuing-delay].title"
-      ],
-      "lighthouse-core/audits/metrics/cumulative-long-queuing-delay.js | description": [
-        "audits[cumulative-long-queuing-delay].description"
-      ],
       "lighthouse-core/audits/metrics/max-potential-fid.js | title": [
         "audits[max-potential-fid].title"
       ],

proto/sample_v2_round_trip.json

@@ -368,11 +368,11 @@
             "title": "Minimize Critical Requests Depth"
         },
         "cumulative-long-queuing-delay": {
-            "description": "[Experimental metric]. Sum of Task Lengths beyond 50ms, between First Contentful Paint and Time To Interactive.",
+            "description": "[Experimental metric] Total time period between FCP and Time to Interactive during which queuing time for any input event would be higher than 50ms.",
             "displayValue": "120\u00a0ms",
             "id": "cumulative-long-queuing-delay",
             "numericValue": 116.79800000000023,
-            "score": 0.83,
+            "score": 1.0,
             "scoreDisplayMode": "numeric",
             "title": "Cumulative Long Queuing Delay"
         },