Upstreaming https://code.google.com/p/chromium/issues/detail?id=177976
Created attachment 190046 [details] Patch
Comment on attachment 190046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190046&action=review > Source/WebCore/inspector/InspectorStyleSheet.cpp:149 > + ASSERT(m_hasText); Could you reuse the code in getRegularExpressionMatchesByLines? > Source/WebCore/inspector/InspectorStyleSheet.cpp:176 > +struct LineColumn { wtf/text/TextPosition.h > Source/WebCore/inspector/InspectorStyleSheet.cpp:191 > + const size_t vectorSize = lineEndings.size(); Move to ContentSearchUtil
Created attachment 190212 [details] Patch
Comment on attachment 190212 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190212&action=review > Source/WebCore/inspector/ContentSearchUtils.cpp:74 > + if (!vectorSize) This check should not be needed, see my comment to lineEndings below. > Source/WebCore/inspector/ContentSearchUtils.cpp:79 > + size_t previousLineEndingOffset; const size_t* foundLineEnding = approximateBinarySearch<size_t, size_t>(lineEndings, vectorSize, offset, sizetExtractor); size_t lineIndex = foundLineEnding - &lineEndings.at(0); if (offset > *foundLineEnding) ++lineIndex; size_t previousLineEndingOffset = lineIndex >= 0 ? lineEndings.at(lineIndex-1) : 0; size_t column = offset - previousLineEndingOffset; return TextPosition(OrdinalNumber::fromZeroBasedInt(lineIndex), OrdinalNumber::fromZeroBasedInt(column)); Seems to be a shorter way to do the same. > Source/WebCore/inspector/ContentSearchUtils.cpp:128 > + while (start < text.length()) { I would do as follows instead to make sure we lways have at least one line (even for an empty file). That would be consistent with what we do in front-end (see utilities.js) while (start < text.length()) { size_t lineEnd = text.find('\n', start); if (lineEnd == notFound) break; result->append(lineEnd); start = lineEnd + 1; } result->append(text.length());
Comment on attachment 190212 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190212&action=review > Source/WebCore/inspector/InspectorStyleSheet.cpp:527 > + OwnPtr<Vector<size_t> > lineEndings(m_parentStyleSheet ? m_parentStyleSheet->lineEndings() : PassOwnPtr<Vector<size_t> >()); Could you please explain what is the expected value of SourceRange in case m_parentStyleSheet is null and hence lineEndings are not available? > Source/WebCore/inspector/front-end/CSSStyleModel.js:882 > + if (resource && resource.type === WebInspector.resourceTypes.Stylesheet) { Why is the resource type important here?
Comment on attachment 190212 [details] Patch r- per my comment about complexity of lineColumnFromOffset implementation.
Created attachment 190690 [details] Patch
Comment on attachment 190690 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190690&action=review > Source/WebCore/inspector/ContentSearchUtils.cpp:71 > +TextPosition lineColumnFromOffset(size_t offset, const Vector<size_t>& lineEndings) textPositionFromOffset?
Comment on attachment 190690 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190690&action=review >> Source/WebCore/inspector/ContentSearchUtils.cpp:71 >> +TextPosition lineColumnFromOffset(size_t offset, const Vector<size_t>& lineEndings) > > textPositionFromOffset? Makes sense, as long as it represents the line:column pair. Thanks.
Created attachment 190729 [details] Patch
Comment on attachment 190729 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190729&action=review > Source/WebCore/inspector/InspectorStyleSheet.cpp:64 > +#include <wtf/StdLibExtras.h> This include should go to ContentSearchUtils
Committed r144434: <http://trac.webkit.org/changeset/144434>