Commit 96b62150 authored by Sigurd Schneider's avatar Sigurd Schneider Committed by Commit Bot

[devtools] Report end position for CSS scripts to DevTools front-end

This simplifies determining whether a location is inside a CSS header
in the front-end (the front-end doesn't have to request the source
to determine the end line:column from the start line:column and the
size).

Before/After: https://imgur.com/a/OFdQBNL

Bug: chromium:1005789, chromium:1005708, chromium:1004203
Change-Id: I4c660c2a5f901cfe24d022b2636926ecf99254ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821721
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: default avatarYang Guo <yangguo@chromium.org>
Reviewed-by: default avatarAndrey Kosyakov <caseq@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704542}
parent bafd44c2
...@@ -844,6 +844,10 @@ experimental domain CSS ...@@ -844,6 +844,10 @@ experimental domain CSS
number startColumn number startColumn
# Size of the content (in characters). # Size of the content (in characters).
number length number length
# Line offset of the end of the stylesheet within the resource (zero based).
number endLine
# Column offset of the end of the stylesheet within the resource (zero based).
number endColumn
# CSS rule representation. # CSS rule representation.
type CSSRule extends object type CSSRule extends object
......
...@@ -1447,6 +1447,29 @@ void InspectorStyleSheet::InnerSetText(const String& text, ...@@ -1447,6 +1447,29 @@ void InspectorStyleSheet::InnerSetText(const String& text,
} }
} }
namespace {
TextPosition TextPositionFromOffsetAndLineEndingsRelativeToStartPosition(
unsigned offset,
const Vector<unsigned>& line_endings,
const TextPosition& start_position) {
TextPosition position =
TextPosition::FromOffsetAndLineEndings(offset, line_endings);
unsigned column = position.column_.ZeroBasedInt();
// A non-zero `start_position.column_` means that the text started in the
// middle of a line, so the start column position must be added if `offset`
// translates to a `position` in the first line of the text.
if (position.line_.ZeroBasedInt() == 0) {
column += start_position.column_.ZeroBasedInt();
}
unsigned line_index =
start_position.line_.ZeroBasedInt() + position.line_.ZeroBasedInt();
return TextPosition(OrdinalNumber::FromZeroBasedInt(line_index),
OrdinalNumber::FromZeroBasedInt(column));
}
} // namespace
std::unique_ptr<protocol::CSS::CSSStyleSheetHeader> std::unique_ptr<protocol::CSS::CSSStyleSheetHeader>
InspectorStyleSheet::BuildObjectForStyleSheetInfo() { InspectorStyleSheet::BuildObjectForStyleSheetInfo() {
CSSStyleSheet* style_sheet = PageStyleSheet(); CSSStyleSheet* style_sheet = PageStyleSheet();
...@@ -1455,8 +1478,16 @@ InspectorStyleSheet::BuildObjectForStyleSheetInfo() { ...@@ -1455,8 +1478,16 @@ InspectorStyleSheet::BuildObjectForStyleSheetInfo() {
Document* document = style_sheet->OwnerDocument(); Document* document = style_sheet->OwnerDocument();
LocalFrame* frame = document ? document->GetFrame() : nullptr; LocalFrame* frame = document ? document->GetFrame() : nullptr;
String text; const LineEndings* line_endings = this->GetLineEndings();
GetText(&text); TextPosition start = style_sheet->StartPositionInSource();
TextPosition end = start;
unsigned text_length = 0;
if (line_endings->size() > 0) {
text_length = line_endings->back();
end = TextPositionFromOffsetAndLineEndingsRelativeToStartPosition(
text_length, *line_endings, start);
}
std::unique_ptr<protocol::CSS::CSSStyleSheetHeader> result = std::unique_ptr<protocol::CSS::CSSStyleSheetHeader> result =
protocol::CSS::CSSStyleSheetHeader::create() protocol::CSS::CSSStyleSheetHeader::create()
.setStyleSheetId(Id()) .setStyleSheetId(Id())
...@@ -1466,11 +1497,11 @@ InspectorStyleSheet::BuildObjectForStyleSheetInfo() { ...@@ -1466,11 +1497,11 @@ InspectorStyleSheet::BuildObjectForStyleSheetInfo() {
.setTitle(style_sheet->title()) .setTitle(style_sheet->title())
.setFrameId(frame ? IdentifiersFactory::FrameId(frame) : "") .setFrameId(frame ? IdentifiersFactory::FrameId(frame) : "")
.setIsInline(style_sheet->IsInline() && !StartsAtZero()) .setIsInline(style_sheet->IsInline() && !StartsAtZero())
.setStartLine( .setStartLine(start.line_.ZeroBasedInt())
style_sheet->StartPositionInSource().line_.ZeroBasedInt()) .setStartColumn(start.column_.ZeroBasedInt())
.setStartColumn( .setLength(text_length)
style_sheet->StartPositionInSource().column_.ZeroBasedInt()) .setEndLine(end.line_.ZeroBasedInt())
.setLength(text.length()) .setEndColumn(end.column_.ZeroBasedInt())
.build(); .build();
if (HasSourceURL()) if (HasSourceURL())
......
...@@ -132,13 +132,11 @@ Formatter.FormatterSourceMapping.prototype = { ...@@ -132,13 +132,11 @@ Formatter.FormatterSourceMapping.prototype = {
originalToFormatted(lineNumber, columnNumber) {}, originalToFormatted(lineNumber, columnNumber) {},
/** /**
* TODO(1005708): Remove the offset parameter once end positions of inline CSS is known.
* @param {number} lineNumber * @param {number} lineNumber
* @param {number=} columnNumber * @param {number=} columnNumber
* @param {number=} offset
* @return {!Array.<number>} * @return {!Array.<number>}
*/ */
formattedToOriginal(lineNumber, columnNumber, offset) {} formattedToOriginal(lineNumber, columnNumber) {}
}; };
/** /**
...@@ -157,14 +155,12 @@ Formatter.IdentityFormatterSourceMapping = class { ...@@ -157,14 +155,12 @@ Formatter.IdentityFormatterSourceMapping = class {
} }
/** /**
* TODO(1005708): Remove the offset parameter once end positions of inline CSS is known.
* @override * @override
* @param {number} lineNumber * @param {number} lineNumber
* @param {number=} columnNumber * @param {number=} columnNumber
* @param {number=} offset
* @return {!Array.<number>} * @return {!Array.<number>}
*/ */
formattedToOriginal(lineNumber, columnNumber, offset) { formattedToOriginal(lineNumber, columnNumber) {
return [lineNumber, columnNumber || 0]; return [lineNumber, columnNumber || 0];
} }
}; };
...@@ -200,18 +196,16 @@ Formatter.FormatterSourceMappingImpl = class { ...@@ -200,18 +196,16 @@ Formatter.FormatterSourceMappingImpl = class {
} }
/** /**
* TODO(chromium:1005708): Remove the offset parameter once end positions of inline CSS is known.
* @override * @override
* @param {number} lineNumber * @param {number} lineNumber
* @param {number=} columnNumber * @param {number=} columnNumber
* @param {number=} offset
* @return {!Array.<number>} * @return {!Array.<number>}
*/ */
formattedToOriginal(lineNumber, columnNumber, offset) { formattedToOriginal(lineNumber, columnNumber) {
const formattedPosition = const formattedPosition =
Formatter.Formatter.locationToPosition(this._formattedLineEndings, lineNumber, columnNumber || 0); Formatter.Formatter.locationToPosition(this._formattedLineEndings, lineNumber, columnNumber || 0);
const originalPosition = this._convertPosition(this._mapping.formatted, this._mapping.original, formattedPosition); const originalPosition = this._convertPosition(this._mapping.formatted, this._mapping.original, formattedPosition);
return Formatter.Formatter.positionToLocation(this._originalLineEndings, (originalPosition || 0) + (offset || 0)); return Formatter.Formatter.positionToLocation(this._originalLineEndings, originalPosition || 0);
} }
/** /**
......
...@@ -101,7 +101,9 @@ export default class CSSModel extends SDK.SDKModel { ...@@ -101,7 +101,9 @@ export default class CSSModel extends SDK.SDKModel {
for (let index = endIndex - 1; for (let index = endIndex - 1;
index >= 0 && headers[index].startLine === last.startLine && headers[index].startColumn === last.startColumn; index >= 0 && headers[index].startLine === last.startLine && headers[index].startColumn === last.startColumn;
--index) { --index) {
locations.push(new CSSLocation(headers[index], lineNumber, columnNumber)); if (headers[index].containsLocation(lineNumber, columnNumber)) {
locations.push(new SDK.CSSLocation(headers[index], lineNumber, columnNumber));
}
} }
...@@ -929,4 +931,4 @@ SDK.SDKModel.register(SDK.CSSModel, SDK.Target.Capability.DOM, true); ...@@ -929,4 +931,4 @@ SDK.SDKModel.register(SDK.CSSModel, SDK.Target.Capability.DOM, true);
SDK.CSSModel.RuleUsage; SDK.CSSModel.RuleUsage;
/** @typedef {{backgroundColors: ?Array<string>, computedFontSize: string, computedFontWeight: string}} */ /** @typedef {{backgroundColors: ?Array<string>, computedFontSize: string, computedFontWeight: string}} */
SDK.CSSModel.ContrastInfo; SDK.CSSModel.ContrastInfo;
\ No newline at end of file
...@@ -22,6 +22,8 @@ export default class CSSStyleSheetHeader { ...@@ -22,6 +22,8 @@ export default class CSSStyleSheetHeader {
this.isInline = payload.isInline; this.isInline = payload.isInline;
this.startLine = payload.startLine; this.startLine = payload.startLine;
this.startColumn = payload.startColumn; this.startColumn = payload.startColumn;
this.endLine = payload.endLine;
this.endColumn = payload.endColumn;
this.contentLength = payload.length; this.contentLength = payload.length;
if (payload.ownerNode) { if (payload.ownerNode) {
this.ownerNode = new SDK.DeferredDOMNode(cssModel.target(), payload.ownerNode); this.ownerNode = new SDK.DeferredDOMNode(cssModel.target(), payload.ownerNode);
...@@ -104,19 +106,14 @@ export default class CSSStyleSheetHeader { ...@@ -104,19 +106,14 @@ export default class CSSStyleSheetHeader {
/** /**
* Checks whether the position is in this style sheet. Assumes that the * Checks whether the position is in this style sheet. Assumes that the
* position's columnNumber is consistent with line endings. * 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} lineNumber
* @param {number} columnNumber * @param {number} columnNumber
* @param {number} lineNumberOfCSSEnd
* @param {number} columnNumberOfCSSEnd
* @return {boolean} * @return {boolean}
*/ */
containsLocation(lineNumber, columnNumber, lineNumberOfCSSEnd, columnNumberOfCSSEnd) { containsLocation(lineNumber, columnNumber) {
const afterStart = const afterStart =
(lineNumber === this.startLine && columnNumber >= this.startColumn) || lineNumber > this.startLine; (lineNumber === this.startLine && columnNumber >= this.startColumn) || lineNumber > this.startLine;
const beforeEnd = const beforeEnd = lineNumber < this.endLine || (lineNumber === this.endLine && columnNumber <= this.endColumn);
lineNumber < lineNumberOfCSSEnd || (lineNumber === lineNumberOfCSSEnd && columnNumber <= columnNumberOfCSSEnd);
return afterStart && beforeEnd; return afterStart && beforeEnd;
} }
......
...@@ -276,19 +276,9 @@ Sources.SourceFormatter.StyleMapping = class { ...@@ -276,19 +276,9 @@ Sources.SourceFormatter.StyleMapping = class {
} }
const [originalLine, originalColumn] = const [originalLine, originalColumn] =
formatData.mapping.formattedToOriginal(uiLocation.lineNumber, uiLocation.columnNumber); formatData.mapping.formattedToOriginal(uiLocation.lineNumber, uiLocation.columnNumber);
const headers = formatData.originalSourceCode[this._headersSymbol]; const headers = formatData.originalSourceCode[this._headersSymbol].filter(
const locations = []; header => header.containsLocation(originalLine, originalColumn));
for (const header of headers) { return headers.map(header => new SDK.CSSLocation(header, originalLine, originalColumn));
// 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;
} }
/** /**
......
...@@ -22,21 +22,11 @@ Content: ...@@ -22,21 +22,11 @@ Content:
</body> </body>
</html> </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. Scanning 21 lines for css locations. Note that location line/column numbers are zero-based.
uiLocation 8:0 resolves to: 8:0 (css) uiLocation 8:0 resolves to: 8:0 (css)
uiLocation 9:0 resolves to: 9:0 (css) uiLocation 9:0 resolves to: 9:0 (css)
uiLocation 10:0 resolves to: 10:0 (css) uiLocation 10:0 resolves to: 10:0 (css)
uiLocation 11:0 resolves to: 11: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. Scanning 21 lines for script locations. Note that location line/column numbers are zero-based.
uiLocation 14:0 resolves to: 14:0 (script) uiLocation 14:0 resolves to: 14:0 (script)
uiLocation 15:0 resolves to: 15:0 (script) uiLocation 15:0 resolves to: 15:0 (script)
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
TestRunner.addResult(`Content:\n${content}`); TestRunner.addResult(`Content:\n${content}`);
const sourceText = new TextUtils.Text(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("css", sourceText.lineCount(), source);
await dumpLocations("script", sourceText.lineCount(), source); await dumpLocations("script", sourceText.lineCount(), source);
...@@ -49,9 +48,8 @@ ...@@ -49,9 +48,8 @@
if (location instanceof SDK.CSSLocation) { if (location instanceof SDK.CSSLocation) {
const h = location.header(); const h = location.header();
if (!h) return "invalid css header"; if (!h) return "invalid css header";
const {endLine, endColumn} = await computeEndPosition(h); if (!h.containsLocation(location.lineNumber, location.columnNumber))
if (!h.containsLocation(location.lineNumber, location.columnNumber, endLine, endColumn)) return `not in css range ${h.startLine}:${h.startColumn}-${h.endLine}:${h.endColumn}`;
return `not in css range ${h.startLine}:${h.startColumn}-${endLine}:${endColumn}`;
return "css"; return "css";
} }
if (location instanceof SDK.DebuggerModel.Location) { if (location instanceof SDK.DebuggerModel.Location) {
...@@ -63,14 +61,6 @@ ...@@ -63,14 +61,6 @@
} }
return "invalid (wrong instance)" 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};
}
} }
......
...@@ -4,6 +4,8 @@ The test verifies events fire for adding, changing, and removing CSSStyleSheets. ...@@ -4,6 +4,8 @@ The test verifies events fire for adding, changing, and removing CSSStyleSheets.
params : { params : {
header : { header : {
disabled : false disabled : false
endColumn : 6
endLine : 4
frameId : <string> frameId : <string>
isInline : false isInline : false
length : <number> length : <number>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment