[inspector] Fix handling of whitespace in breakpoint hinting

The patch fixes two bugs in hinting:
- trimmed whitespace in hints was not taken into account.
- range check for out-of-bound hints did not include the offset.

Bug: chromium:1409286
Change-Id: I5838cd6b697ed13a19c30f158963c0d9fac2f045
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4187224
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85448}

src/inspector/string-16.cc

@@ -118,8 +118,8 @@ int String16::toInteger(bool* ok) const {
   return static_cast<int>(result);
 }
 
-String16 String16::stripWhiteSpace() const {
-  if (!length()) return String16();
+std::pair<size_t, size_t> String16::getTrimmedOffsetAndLength() const {
+  if (!length()) return std::make_pair(0, 0);
 
   size_t start = 0;
   size_t end = length() - 1;
@@ -128,13 +128,21 @@ String16 String16::stripWhiteSpace() const {
   while (start <= end && isSpaceOrNewLine(characters16()[start])) ++start;
 
   // only white space
-  if (start > end) return String16();
+  if (start > end) return std::make_pair(0, 0);
 
   // skip white space from end
   while (end && isSpaceOrNewLine(characters16()[end])) --end;
 
-  if (!start && end == length() - 1) return *this;
-  return String16(characters16() + start, end + 1 - start);
+  return std::make_pair(start, end + 1 - start);
+}
+
+String16 String16::stripWhiteSpace() const {
+  std::pair<size_t, size_t> offsetAndLength = getTrimmedOffsetAndLength();
+  if (offsetAndLength.second == 0) return String16();
+  if (offsetAndLength.first == 0 && offsetAndLength.second == length() - 1) {
+    return *this;
+  }
+  return substring(offsetAndLength.first, offsetAndLength.second);
 }
 
 String16Builder::String16Builder() = default;

src/inspector/string-16.h

@@ -46,6 +46,7 @@ class String16 {
   int64_t toInteger64(bool* ok = nullptr) const;
   uint64_t toUInt64(bool* ok = nullptr) const;
   int toInteger(bool* ok = nullptr) const;
+  std::pair<size_t, size_t> getTrimmedOffsetAndLength() const;
   String16 stripWhiteSpace() const;
   const UChar* characters16() const { return m_impl.c_str(); }
   size_t length() const { return m_impl.length(); }

src/inspector/v8-debugger-agent-impl.cc

@@ -195,8 +195,11 @@ std::unique_ptr<protocol::DictionaryValue> breakpointHint(
   }
 
   auto hintObject = protocol::DictionaryValue::create();
+  String16 rawHint = script.source(actualOffset, kBreakpointHintMaxLength);
+  std::pair<size_t, size_t> offsetAndLength =
+      rawHint.getTrimmedOffsetAndLength();
   String16 hint =
-      script.source(actualOffset, kBreakpointHintMaxLength).stripWhiteSpace();
+      rawHint.substring(offsetAndLength.first, offsetAndLength.second);
   for (size_t i = 0; i < hint.length(); ++i) {
     if (hint[i] == '\r' || hint[i] == '\n' || hint[i] == ';') {
       hint = hint.substring(0, i);
@@ -210,7 +213,7 @@ std::unique_ptr<protocol::DictionaryValue> breakpointHint(
   // time, we will keep the breakpoint at the same location (so that
   // breakpoints do not slide around on reloads without any edits).
   if (breakpointOffset <= actualOffset) {
-    size_t length = actualOffset - breakpointOffset;
+    size_t length = actualOffset - breakpointOffset + offsetAndLength.first;
     String16 prefix = script.source(breakpointOffset, length);
     int crc32 = computeCrc32(prefix);
     hintObject->setInteger(DebuggerAgentState::breakpointHintPrefixHash, crc32);
@@ -276,7 +279,7 @@ void adjustBreakpointLocation(const V8DebuggerScript& script,
   }
   size_t bestMatch;
   if (nextMatch == String16::kNotFound ||
-      nextMatch > kBreakpointHintMaxSearchOffset) {
+      nextMatch > offset + kBreakpointHintMaxSearchOffset) {
     bestMatch = prevMatch;
   } else if (prevMatch == String16::kNotFound) {
     bestMatch = nextMatch;

test/inspector/debugger/restore-breakpoint-expected.txt

@@ -44,6 +44,31 @@ bad();
 #boo();
 }
 
+Running test: testInsertNewLineWithLongCommentBefore
+/////////////////////////////////////////////////////////////////////////////...
+function foo() {
+boo();
+#boo();
+}
+/////////////////////////////////////////////////////////////////////////////...
+function foo() {
+boo();
+
+#boo();
+}
+
+Running test: testSameSourceBreakAfterReturnWithWhitespace
+function baz() {
+}
+
+function foo() {
+return 1;#                              }
+function baz() {
+}
+
+function foo() {
+return 1;#                              }
+
 Running test: testSameSourceDuplicateLinesDifferentPrefix
 function foo() {
 boo();

test/inspector/debugger/restore-breakpoint.js

@@ -29,6 +29,20 @@ InspectorTest.runTestSuite([
     test(source, source, {lineNumber: 3, columnNumber: 0}, next);
   },
 
+  function testInsertNewLineWithLongCommentBefore(next) {
+    var longComment = '/'.repeat(1e3);
+    var source = `${longComment}\nfunction foo() {\nboo();\nboo();\n}`;
+    var newSource = `${longComment}\nfunction foo() {\nboo();\n\nboo();\n}`;
+    test(source, newSource, {lineNumber: 3, columnNumber: 0}, next);
+  },
+
+  function testSameSourceBreakAfterReturnWithWhitespace(next) {
+    var whitespace = ' '.repeat(30);
+    var source =
+        `function baz() {\n}\n\nfunction foo() {\nreturn 1;${whitespace}}`;
+    test(source, source, {lineNumber: 4, columnNumber: 9}, next);
+  },
+
   function testSameSourceDuplicateLinesDifferentPrefix(next) {
     var source = 'function foo() {\nboo();\n// something\nboo();\n}';
     var newSource = 'function foo() {\nboo();\n// somethinX\nboo();\n}';