Refactor script position calculation

src/js/messages.js

Index: src/js/messages.js
diff --git a/src/js/messages.js b/src/js/messages.js
index be33ff759b79dd0945f529636d881958f14924bc..470ddca27dc425b0b2c11f68eac084f2dfe966aa 100644
--- a/src/js/messages.js
+++ b/src/js/messages.js
@@ -225,34 +225,8 @@ function GetSourceLine(message) {
        else the line number.
  */
 function ScriptLineFromPosition(position) {
-  var lower = 0;
-  var upper = this.lineCount() - 1;
-  var line_ends = this.line_ends;
-
-  // We'll never find invalid positions so bail right away.
-  if (position > line_ends[upper]) {
-    return -1;
-  }
-
-  // This means we don't have to safe-guard indexing line_ends[i - 1].
-  if (position <= line_ends[0]) {
-    return 0;
-  }
-
-  // Binary search to find line # from position range.
-  while (upper >= 1) {
-    var i = (lower + upper) >> 1;
-
-    if (position > line_ends[i]) {
-      lower = i + 1;
-    } else if (position <= line_ends[i - 1]) {
-      upper = i - 1;
-    } else {
-      return i;
-    }
-  }
-
-  return -1;
+  var info = %ScriptPositionInfo(this, position, false);
+  return (info == null) ? -1 : info.line;
 }
 
 
@@ -266,27 +240,11 @@ function ScriptLineFromPosition(position) {
  */
 function ScriptLocationFromPosition(position,
                                     include_resource_offset) {
-  var line = this.lineFromPosition(position);
-  if (line == -1) return null;
-
-  // Determine start, end and column.
-  var line_ends = this.line_ends;
-  var start = line == 0 ? 0 : line_ends[line - 1] + 1;
-  var end = line_ends[line];
-  if (end > 0 && %_StringCharAt(this.source, end - 1) === '\r') {
-    end--;
-  }
-  var column = position - start;
+  var info = %ScriptPositionInfo(this, position, !!include_resource_offset);
+  if (info == null) return null;
 
-  // Adjust according to the offset within the resource.
-  if (include_resource_offset) {
-    line += this.line_offset;
-    if (line == this.line_offset) {
-      column += this.column_offset;
-    }
-  }
-
-  return new SourceLocation(this, position, line, column, start, end);
+  return new SourceLocation(this, position, info.line, info.column,

[Yang, 2016/05/18 13:23:08]

Can we simply return 'info'? Do we need line_start and line_end for anything
other than formatting the string in SourceLocationSourceText? Is
SourceLocation.start and SourceLocation.end used anywhere? If not, we can simply
omit line_start/line_end and simply have %ScriptPositionInfo store the string
slice for the source text on 'info'. Substrings are fairly cheap to create
anyways.

[jgruber1, 2016/05/19 08:05:24]

Done. I'll remove ScripLocationFromPosition itself in an upcoming CL.

+          info.line_start, info.line_end);
 }
 
 
@@ -333,7 +291,7 @@ function ScriptLocationFromLine(opt_line, opt_column, opt_offset_position) {
     }
 
     return this.locationFromPosition(
-        this.line_ends[offset_line + line - 1] + 1 + column);  // line > 0 here.
+        %ScriptLineStartPosition(this, offset_line + line) + column);
   }
 }
 
@@ -367,15 +325,14 @@ function ScriptSourceSlice(opt_from_line, opt_to_line) {
     return null;
   }
 
-  var line_ends = this.line_ends;
-  var from_position = from_line == 0 ? 0 : line_ends[from_line - 1] + 1;
-  var to_position = to_line == 0 ? 0 : line_ends[to_line - 1] + 1;
+  var from_position = %ScriptLineStartPosition(this, from_line);
+  var to_position = %ScriptLineStartPosition(this, to_line);
 
   // Return a source slice with line numbers re-adjusted to the resource.
   return new SourceSlice(this,

[Yang, 2016/05/18 13:23:08]

The SourceSlice object seems not to be used anywhere in Chrome, and in V8 only
in a test case. Let's remove that test case and this whole SourceSlice thing.

[jgruber1, 2016/05/19 08:05:24]

What about DebugCommandProcessor.prototype.sourceRequest_?

                          from_line + this.line_offset,
                          to_line + this.line_offset,
-                          from_position, to_position);
+                         from_position, to_position);
 }
 
 
@@ -393,9 +350,8 @@ function ScriptSourceLine(opt_line) {
   }
 
   // Return the source line.
-  var line_ends = this.line_ends;
-  var start = line == 0 ? 0 : line_ends[line - 1] + 1;
-  var end = line_ends[line];
+  var start = %ScriptLineStartPosition(this, line);
+  var end = %ScriptLineEndPosition(this, line);
   return %_Call(StringSubstring, this.source, start, end);
 }
 
@@ -407,17 +363,7 @@ function ScriptSourceLine(opt_line) {
  */
 function ScriptLineCount() {
   // Return number of source lines.
-  return this.line_ends.length;
-}
-
-
-/**
- * Returns the position of the nth line end.
- * @return {number}
- *     Zero-based position of the nth line end in the script.
- */
-function ScriptLineEnd(n) {
-  return this.line_ends[n];
+  return %ScriptLineCount(this);

[Yang, 2016/05/18 13:23:09]

This also doesn't seem to be used outside of V8. Let's not expose this anymore.

[jgruber1, 2016/05/19 08:05:24]

Ditto.

 }
 
 
@@ -442,7 +388,6 @@ utils.SetUpLockedPrototype(Script, [
     "name",
     "source_url",
     "source_mapping_url",
-    "line_ends",
     "line_offset",
     "column_offset"
   ], [
@@ -453,7 +398,6 @@ utils.SetUpLockedPrototype(Script, [
     "sourceLine", ScriptSourceLine,
     "lineCount", ScriptLineCount,
     "nameOrSourceURL", ScriptNameOrSourceURL,
-    "lineEnd", ScriptLineEnd
   ]
 );