[devtools] Resolve less uiLocations to non-existent CSS/script locations

Both Bindings.cssWorkspaceBinding.uiLocationToRawLocations and well as
Bindings.debuggerWorkspaceBinding.uiLocationToRawLocations resolve
locations to non-existing CSS and Script locations. This patch fixes
Bindings.debuggerWorkspaceBinding.uiLocationToRawLocations to not do
this anymore, and hacks
Bindings.cssWorkspaceBinding.uiLocationToRawLocations to not do it
for formatted sources anymore. For the ultimate fix, we need to get
the end position for inline CSS from the back-end. This will allow
us to clean-up the hack as well.

The significance of this change is that it enables precise coverage
gutters for inline CSS and inline scripts.


Bug: chromium:1004203, chromium:1005708, chromium:1005789
Change-Id: Ieadb2aef7e610ecec35e28f5f1cc4516a28a0780
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1814299
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698431}

third_party/blink/renderer/devtools/front_end/bindings/ResourceMapping.js

@@ -107,7 +107,9 @@ Bindings.ResourceMapping = class {
     if (!debuggerModel)
       return [];
     const location = debuggerModel.createRawLocationByURL(uiSourceCode.url(), lineNumber, columnNumber);
-    return location ? [location] : [];
+    if (location && location.script().containsLocation(lineNumber, columnNumber))
+      return [location];
+    return [];
   }
 
   /**

third_party/blink/renderer/devtools/front_end/coverage/CoverageDecorationManager.js

@@ -140,29 +140,19 @@ Coverage.CoverageDecorationManager = class {
     if (contentType.hasScripts()) {
       let locations = Bindings.debuggerWorkspaceBinding.uiLocationToRawLocations(uiSourceCode, line, column);
       locations = locations.filter(location => !!location.script());
-      for (let location of locations) {
+      for (const location of locations) {
         const script = location.script();
         if (script.isInlineScript() && contentType.isDocument()) {
-          // TODO(chromium:1005789): Remove this check, once the bindings only return locations that actually belong to
-          // the script they claim to belong to.
-          if (comparePositions(script.lineOffset, script.columnOffset, location.lineNumber, location.columnNumber) >
-                  0 ||
-              comparePositions(script.endLine, script.endColumn, location.lineNumber, location.columnNumber) <= 0) {
-            location = null;
-          } else {
-            location.lineNumber -= script.lineOffset;
-            if (!location.lineNumber)
-              location.columnNumber -= script.columnOffset;
-          }
-        }
-        if (location) {
-          result.push({
-            id: `js:${location.scriptId}`,
-            contentProvider: location.script(),
-            line: location.lineNumber,
-            column: location.columnNumber
-          });
+          location.lineNumber -= script.lineOffset;
+          if (!location.lineNumber)
+            location.columnNumber -= script.columnOffset;
         }
+        result.push({
+          id: `js:${location.scriptId}`,
+          contentProvider: location.script(),
+          line: location.lineNumber,
+          column: location.columnNumber
+        });
       }
     }
     if (contentType.isStyleSheet() || contentType.isDocument()) {
@@ -173,13 +163,6 @@ Coverage.CoverageDecorationManager = class {
         if (!header)
           continue;
         if (header.isInline && contentType.isDocument()) {
-          // TODO(chromium:1005789): Remove this check, once the bindings only return locations that actually belong to
-          // the CSS header they claim to belong to.
-          if (comparePositions(header.startLine, header.startColumn, location.lineNumber, location.columnNumber) > 0) {
-            // TODO(chromium:1005708): Also check that the location is still inside the script once we have the line:column
-            // for the end of the inline script.
-            continue;
-          }
           location.lineNumber -= header.startLine;
           if (!location.lineNumber)
             location.columnNumber -= header.startColumn;
@@ -192,19 +175,7 @@ Coverage.CoverageDecorationManager = class {
         });
       }
     }
-    result.sort(Coverage.CoverageDecorationManager._compareLocations);
-
-    /**
-     * @param {number} aLine
-     * @param {number} aColumn
-     * @param {number} bLine
-     * @param {number} bColumn
-     * @return {number}
-     */
-    function comparePositions(aLine, aColumn, bLine, bColumn) {
-      return aLine - bLine || aColumn - bColumn;
-    }
-    return result;
+    return result.sort(Coverage.CoverageDecorationManager._compareLocations);
   }
 
   /**

third_party/blink/renderer/devtools/front_end/formatter/ScriptFormatter.js

@@ -130,11 +130,13 @@ Formatter.FormatterSourceMapping.prototype = {
   originalToFormatted(lineNumber, columnNumber) {},
 
   /**
+   * TODO(1005708): Remove the offset parameter once end positions of inline CSS is known.
    * @param {number} lineNumber
    * @param {number=} columnNumber
+   * @param {number=} offset
    * @return {!Array.<number>}
    */
-  formattedToOriginal(lineNumber, columnNumber) {}
+  formattedToOriginal(lineNumber, columnNumber, offset) {}
 };
 
 /**
@@ -153,12 +155,14 @@ Formatter.IdentityFormatterSourceMapping = class {
   }
 
   /**
+   * TODO(1005708): Remove the offset parameter once end positions of inline CSS is known.
    * @override
    * @param {number} lineNumber
    * @param {number=} columnNumber
+   * @param {number=} offset
    * @return {!Array.<number>}
    */
-  formattedToOriginal(lineNumber, columnNumber) {
+  formattedToOriginal(lineNumber, columnNumber, offset) {
     return [lineNumber, columnNumber || 0];
   }
 };
@@ -194,16 +198,18 @@ Formatter.FormatterSourceMappingImpl = class {
   }
 
   /**
+   * TODO(chromium:1005708): Remove the offset parameter once end positions of inline CSS is known.
    * @override
    * @param {number} lineNumber
    * @param {number=} columnNumber
+   * @param {number=} offset
    * @return {!Array.<number>}
    */
-  formattedToOriginal(lineNumber, columnNumber) {
+  formattedToOriginal(lineNumber, columnNumber, offset) {
     const formattedPosition =
         Formatter.Formatter.locationToPosition(this._formattedLineEndings, lineNumber, columnNumber || 0);
     const originalPosition = this._convertPosition(this._mapping.formatted, this._mapping.original, formattedPosition);
-    return Formatter.Formatter.positionToLocation(this._originalLineEndings, originalPosition || 0);
+    return Formatter.Formatter.positionToLocation(this._originalLineEndings, (originalPosition || 0) + (offset || 0));
   }
 
   /**

third_party/blink/renderer/devtools/front_end/sdk/CSSStyleSheetHeader.js

@@ -99,6 +99,25 @@ SDK.CSSStyleSheetHeader = class {
     return (lineNumberInStyleSheet ? 0 : this.startColumn) + columnNumberInStyleSheet;
   }
 
+  /**
+   * Checks whether the position is in this style sheet. Assumes that the
+   * position's columnNumber is consistent with line endings.
+   * TODO(chromium:1005708): Ensure that this object knows its end position,
+   * and remove {line,column}NumberOfCSSEnd parameters.
+   * @param {number} lineNumber
+   * @param {number} columnNumber
+   * @param {number} lineNumberOfCSSEnd
+   * @param {number} columnNumberOfCSSEnd
+   * @return {boolean}
+   */
+  containsLocation(lineNumber, columnNumber, lineNumberOfCSSEnd, columnNumberOfCSSEnd) {
+    const afterStart =
+        (lineNumber === this.startLine && columnNumber >= this.startColumn) || lineNumber > this.startLine;
+    const beforeEnd =
+        lineNumber < lineNumberOfCSSEnd || (lineNumber === lineNumberOfCSSEnd && columnNumber <= columnNumberOfCSSEnd);
+    return afterStart && beforeEnd;
+  }
+
   /**
    * @override
    * @return {string}

third_party/blink/renderer/devtools/front_end/sdk/Script.js

@@ -261,6 +261,13 @@ SDK.Script = class {
         {scriptId: this.scriptId, positions});
     return !response[Protocol.Error];
   }
+
+  containsLocation(lineNumber, columnNumber) {
+    const afterStart =
+        (lineNumber === this.lineOffset && columnNumber >= this.columnOffset) || lineNumber > this.lineOffset;
+    const beforeEnd = lineNumber < this.endLine || (lineNumber === this.endLine && columnNumber <= this.endColumn);
+    return afterStart && beforeEnd;
+  }
 };
 
 SDK.Script.sourceURLRegex = /^[\040\t]*\/\/[@#] sourceURL=\s*(\S*?)\s*$/m;

third_party/blink/renderer/devtools/front_end/sources/SourceFormatter.js

@@ -182,12 +182,10 @@ Sources.SourceFormatter.ScriptMapping = class {
     const formatData = Sources.SourceFormatData._for(uiSourceCode);
     if (!formatData)
       return [];
-    const originalLocation = formatData.mapping.formattedToOriginal(lineNumber, columnNumber);
-    const scripts = this._scriptsForUISourceCode(formatData.originalSourceCode);
-    if (!scripts.length)
-      return [];
-    return scripts.map(
-        script => script.debuggerModel.createRawLocation(script, originalLocation[0], originalLocation[1]));
+    const [originalLine, originalColumn] = formatData.mapping.formattedToOriginal(lineNumber, columnNumber);
+    const scripts = this._scriptsForUISourceCode(formatData.originalSourceCode)
+                        .filter(script => script.containsLocation(originalLine, originalColumn));
+    return scripts.map(script => script.debuggerModel.createRawLocation(script, originalLine, originalColumn));
   }
 
   /**
@@ -264,9 +262,20 @@ Sources.SourceFormatter.StyleMapping = class {
     const formatData = Sources.SourceFormatData._for(uiLocation.uiSourceCode);
     if (!formatData)
       return [];
-    const originalLocation = formatData.mapping.formattedToOriginal(uiLocation.lineNumber, uiLocation.columnNumber);
+    const [originalLine, originalColumn] =
+        formatData.mapping.formattedToOriginal(uiLocation.lineNumber, uiLocation.columnNumber);
     const headers = formatData.originalSourceCode[this._headersSymbol];
-    return headers.map(header => new SDK.CSSLocation(header, originalLocation[0], originalLocation[1]));
+    const locations = [];
+    for (const header of headers) {
+      // TODO(chromium:1005708): Remove this computation and use the end location from `header` once the back-end provides it.
+      const [formattedStartLine, formattedStartColumn] =
+          formatData.mapping.originalToFormatted(header.startLine, header.startColumn);
+      const [originalEndLine, originalEndColumn] =
+          formatData.mapping.formattedToOriginal(formattedStartLine, formattedStartColumn, header.contentLength);
+      if (header.containsLocation(originalLine, originalColumn, originalEndLine, originalEndColumn))
+        locations.push(new SDK.CSSLocation(header, originalLine, originalColumn));
+    }
+    return locations;
   }
 
   /**

third_party/blink/web_tests/http/tests/devtools/sources/inline-styles-scripts-locations-expected.txt

+Bindings should only generate locations for an inline script (style) if the location is inside of the inline script (style).
+
+Content:
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Website</title>
+</head>
+<body>
+    <style>
+        body {
+            --var-1: one;
+        }
+    </style>
+    Hello World
+    <script>
+        function firstFunction() {
+        }
+    </script>
+    <style>body {--var-2: two;} </style><script>function secondFunction() { }</script>
+</body>
+</html>
+
+The following output for css locations on the unformatted source is currently wrong due to BUG(1005708).
+Scanning 21 lines for css locations. Note that location line/column numbers are zero-based.
+uiLocation 8:0 resolves to: 8:0 (css)
+uiLocation 9:0 resolves to: 9:0 (css)
+uiLocation 10:0 resolves to: 10:0 (css)
+uiLocation 11:0 resolves to: 11:0 (css)
+uiLocation 12:0 resolves to: 12:0 (not in css range 7:11-11:4)
+uiLocation 13:0 resolves to: 13:0 (not in css range 7:11-11:4)
+uiLocation 14:0 resolves to: 14:0 (not in css range 7:11-11:4)
+uiLocation 15:0 resolves to: 15:0 (not in css range 7:11-11:4)
+uiLocation 16:0 resolves to: 16:0 (not in css range 7:11-11:4)
+uiLocation 17:0 resolves to: 17:0 (not in css range 7:11-11:4)
+uiLocation 18:0 resolves to: 18:0 (not in css range 17:11-17:32)
+uiLocation 19:0 resolves to: 19:0 (not in css range 17:11-17:32)
+uiLocation 20:0 resolves to: 20:0 (not in css range 17:11-17:32)
+Scanning 21 lines for script locations. Note that location line/column numbers are zero-based.
+uiLocation 14:0 resolves to: 14:0 (script)
+uiLocation 15:0 resolves to: 15:0 (script)
+uiLocation 16:0 resolves to: 16:0 (script)
+
+
+Formatting source now...
+
+
+Formatted Content:
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Website</title>
+    </head>
+    <body>
+        <style>
+            body {
+                --var-1: one; }
+        </style>
+        Hello World
+    <script>
+            function firstFunction() {}
+        </script>
+        <style>
+            body {
+                --var-2: two;}
+        </style>
+        <script>
+            function secondFunction() {}
+        </script>
+    </body>
+</html>
+
+Scanning 25 lines for css locations. Note that location line/column numbers are zero-based.
+uiLocation 8:0 resolves to: 8:0 (css)
+uiLocation 9:0 resolves to: 9:0 (css)
+uiLocation 10:0 resolves to: 11:0 (css)
+uiLocation 16:0 resolves to: 17:11 (css)
+uiLocation 17:0 resolves to: 17:17 (css)
+uiLocation 18:0 resolves to: 17:32 (css)
+Scanning 25 lines for script locations. Note that location line/column numbers are zero-based.
+uiLocation 13:0 resolves to: 14:0 (script)
+uiLocation 14:0 resolves to: 16:0 (script)
+uiLocation 20:0 resolves to: 17:48 (script)
+uiLocation 21:0 resolves to: 17:77 (script)
+

third_party/blink/web_tests/http/tests/devtools/sources/inline-styles-scripts-locations.js

+// Copyright 2019 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.
+
+(async function() {
+  TestRunner.addResult(`Bindings should only generate locations for an inline script (style) if the location is inside of the inline script (style).\n`);
+  await TestRunner.loadModule('sources_test_runner');
+  await TestRunner.showPanel('sources');
+
+  await TestRunner.navigatePromise('../bindings/resources/inline-style.html');
+  const source = await TestRunner.waitForUISourceCode('inline-style.html', Workspace.projectTypes.Network);
+  const content = await source.requestContent();
+  TestRunner.addResult(`Content:\n${content}`);
+  const sourceText = new TextUtils.Text(content);
+
+  TestRunner.addResult(`The following output for css locations on the unformatted source is currently wrong due to BUG(1005708).`)
+  await dumpLocations("css", sourceText.lineCount(), source);
+  await dumpLocations("script", sourceText.lineCount(), source);
+
+  TestRunner.addResult("\n\nFormatting source now...\n\n");
+
+  const formatData = await Sources.sourceFormatter.format(source);
+  const formattedSource = formatData.formattedSourceCode;
+  var formattedContent = await formatData.formattedSourceCode.requestContent();
+  TestRunner.addResult(`Formatted Content:\n${formattedContent}`);
+  const formattedSourceText = new TextUtils.Text(formattedContent);
+  await dumpLocations("css", formattedSourceText.lineCount(), formattedSource);
+  await dumpLocations("script", formattedSourceText.lineCount(), formattedSource);
+
+
+  async function dumpLocations(type, lineCount, source) {
+    TestRunner.addResult(`Scanning ${lineCount} lines for ${type} locations. Note that location line/column numbers are zero-based.`);
+    for (let line = 0; line < lineCount; ++line) {
+      const rawLocations = getLocations(line);
+      if (rawLocations.length) {
+        const results = rawLocations.map(async loc => `${loc.lineNumber}:${loc.columnNumber} (${await checkValidity(loc)})`);
+        const rawLocationsString = (await Promise.all(results)).join('  ');
+        TestRunner.addResult(`uiLocation ${line}:0 resolves to: ${rawLocationsString}`)
+      }
+    }
+    function getLocations(line) {
+      if (type === "css")
+        return Bindings.cssWorkspaceBinding.uiLocationToRawLocations(source.uiLocation(line, 0));
+      if (type === "script")
+        return Bindings.debuggerWorkspaceBinding.uiLocationToRawLocations(source, line, 0);
+      return null;
+    }
+    async function checkValidity(location) {
+      if (location instanceof SDK.CSSLocation) {
+        const h = location.header();
+        if (!h) return "invalid css header";
+        const {endLine, endColumn} = await computeEndPosition(h);
+        if (!h.containsLocation(location.lineNumber, location.columnNumber, endLine, endColumn))
+          return `not in css range ${h.startLine}:${h.startColumn}-${endLine}:${endColumn}`;
+        return "css";
+      }
+      if (location instanceof SDK.DebuggerModel.Location) {
+        const s = location.script();
+        if (!s) return "invalid script";
+        if (!s.containsLocation(location.lineNumber, location.columnNumber))
+          return `not in script range ${s.lineOffset}:${s.columnOffset}-${s.endLine}:${s.endColumn})`;
+        return "script";
+      }
+      return "invalid (wrong instance)"
+    }
+    async function computeEndPosition(header) {
+      const content = await header.requestContent();
+      const contentText = new TextUtils.Text(content);
+      const {lineNumber, columnNumber} = contentText.positionFromOffset(header.contentLength);
+      const endLine = lineNumber + header.startLine;
+      const endColumn = columnNumber + (lineNumber == 0 ? header.startColumn : 0);
+      return {endLine, endColumn};
+    }
+  }
+
+
+  TestRunner.completeTest();
+})();