Revert of Refactor script position calculation (patchset #6 id:100001…

… of https://codereview.chromium.org/1986173002/ )

Reason for revert:
Breaks layout tests:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/6896

Original issue's description:
> Refactor script position calculation
>
> Script position calculation logic (i.e. line & column numbers for a
> given code position) is now based on a single method
> Script::GetPositionInfo(). Refactored related code in isolate.cc and
> js/messages.js to use the new method and removed the line_ends JS
> accessor.
>
> R=yangguo@chromium.org
> BUG=
>
> Committed: https://crrev.com/c04d547298ce4fd425ef1eaa9b02ad1e177918dc
> Cr-Commit-Position: refs/heads/master@{#36359}

TBR=yangguo@chromium.org,jgruber@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review-Url: https://codereview.chromium.org/1994973002
Cr-Commit-Position: refs/heads/master@{#36368}

src/accessors.cc

@@ -437,6 +437,40 @@ Handle<AccessorInfo> Accessors::ScriptCompilationTypeInfo(
 }
 
 
+//
+// Accessors::ScriptGetLineEnds
+//
+
+
+void Accessors::ScriptLineEndsGetter(
+    v8::Local<v8::Name> name,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
+  HandleScope scope(isolate);
+  Handle<Object> object = Utils::OpenHandle(*info.Holder());
+  Handle<Script> script(
+      Script::cast(Handle<JSValue>::cast(object)->value()), isolate);
+  Script::InitLineEnds(script);
+  DCHECK(script->line_ends()->IsFixedArray());
+  Handle<FixedArray> line_ends(FixedArray::cast(script->line_ends()));
+  // We do not want anyone to modify this array from JS.
+  DCHECK(*line_ends == isolate->heap()->empty_fixed_array() ||
+         line_ends->map() == isolate->heap()->fixed_cow_array_map());
+  Handle<JSArray> js_array =
+      isolate->factory()->NewJSArrayWithElements(line_ends);
+  info.GetReturnValue().Set(Utils::ToLocal(js_array));
+}
+
+
+Handle<AccessorInfo> Accessors::ScriptLineEndsInfo(
+      Isolate* isolate, PropertyAttributes attributes) {
+  Handle<String> name(isolate->factory()->InternalizeOneByteString(
+      STATIC_CHAR_VECTOR("line_ends")));
+  return MakeAccessor(isolate, name, &ScriptLineEndsGetter, nullptr,
+                      attributes);
+}
+
+
 //
 // Accessors::ScriptSourceUrl
 //

src/accessors.h

@@ -36,6 +36,7 @@ class AccessorInfo;
   V(ScriptEvalFromScriptPosition) \
   V(ScriptEvalFromFunctionName)   \
   V(ScriptId)                     \
+  V(ScriptLineEnds)               \
   V(ScriptLineOffset)             \
   V(ScriptName)                   \
   V(ScriptSource)                 \

src/bootstrapper.cc

@@ -2364,6 +2364,15 @@ void Bootstrapper::ExportFromRuntime(Isolate* isolate,
       script_map->AppendDescriptor(&d);
     }
 
+    Handle<AccessorInfo> script_line_ends =
+        Accessors::ScriptLineEndsInfo(isolate, attribs);
+    {
+      AccessorConstantDescriptor d(
+          Handle<Name>(Name::cast(script_line_ends->name())), script_line_ends,
+          attribs);
+      script_map->AppendDescriptor(&d);
+    }
+
     Handle<AccessorInfo> script_context_data =
         Accessors::ScriptContextDataInfo(isolate, attribs);
     {

src/debug/mirrors.js

@@ -1950,7 +1950,7 @@ FrameMirror.prototype.sourceColumn = function() {
 FrameMirror.prototype.sourceLineText = function() {
   var location = this.sourceLocation();
   if (location) {
-    return location.sourceText;
+    return location.sourceText();
   }
 };
 

src/heap-symbols.h

@@ -34,7 +34,6 @@
   V(cell_value_string, "%cell_value")                              \
   V(char_at_string, "CharAt")                                      \
   V(closure_string, "(closure)")                                   \
-  V(column_string, "column")                                       \
   V(compare_ic_string, "==")                                       \
   V(configurable_string, "configurable")                           \
   V(constructor_string, "constructor")                             \
@@ -83,7 +82,6 @@
   V(KeyedStoreMonomorphic_string, "KeyedStoreMonomorphic")         \
   V(last_index_string, "lastIndex")                                \
   V(length_string, "length")                                       \
-  V(line_string, "line")                                           \
   V(Map_string, "Map")                                             \
   V(minus_infinity_string, "-Infinity")                            \
   V(minus_zero_string, "-0")                                       \
@@ -97,7 +95,6 @@
   V(object_string, "object")                                       \
   V(Object_string, "Object")                                       \
   V(ownKeys_string, "ownKeys")                                     \
-  V(position_string, "position")                                   \
   V(preventExtensions_string, "preventExtensions")                 \
   V(private_api_string, "private_api")                             \
   V(Promise_string, "Promise")                                     \
@@ -106,13 +103,11 @@
   V(Proxy_string, "Proxy")                                         \
   V(query_colon_string, "(?:)")                                    \
   V(RegExp_string, "RegExp")                                       \
-  V(script_string, "script")                                       \
   V(setPrototypeOf_string, "setPrototypeOf")                       \
   V(set_string, "set")                                             \
   V(Set_string, "Set")                                             \
   V(source_mapping_url_string, "source_mapping_url")               \
   V(source_string, "source")                                       \
-  V(sourceText_string, "sourceText")                               \
   V(source_url_string, "source_url")                               \
   V(stack_string, "stack")                                         \
   V(strict_compare_ic_string, "===")                               \

src/isolate.cc

@@ -551,17 +551,28 @@ class CaptureStackTraceHelper {
     Handle<Script> script(Script::cast(fun->shared()->script()));
 
     if (!line_key_.is_null()) {
-      Script::PositionInfo info;
-      bool valid_pos =
-          script->GetPositionInfo(position, &info, Script::kWithOffset);
-
-      if (!column_key_.is_null() && valid_pos) {
+      int script_line_offset = script->line_offset();
+      int line_number = Script::GetLineNumber(script, position);
+      // line_number is already shifted by the script_line_offset.
+      int relative_line_number = line_number - script_line_offset;
+      if (!column_key_.is_null() && relative_line_number >= 0) {
+        Handle<FixedArray> line_ends(FixedArray::cast(script->line_ends()));
+        int start =
+            (relative_line_number == 0)
+                ? 0
+                : Smi::cast(line_ends->get(relative_line_number - 1))->value() +
+                      1;
+        int column_offset = position - start;
+        if (relative_line_number == 0) {
+          // For the case where the code is on the same line as the script tag.
+          column_offset += script->column_offset();
+        }
         JSObject::AddProperty(stack_frame, column_key_,
-                              handle(Smi::FromInt(info.column + 1), isolate_),
+                              handle(Smi::FromInt(column_offset + 1), isolate_),
                               NONE);
       }
       JSObject::AddProperty(stack_frame, line_key_,
-                            handle(Smi::FromInt(info.line + 1), isolate_),
+                            handle(Smi::FromInt(line_number + 1), isolate_),
                             NONE);
     }
 

src/js/messages.js

@@ -214,7 +214,7 @@ function GetSourceLine(message) {
   var start_position = %MessageGetStartPosition(message);
   var location = script.locationFromPosition(start_position, true);
   if (location == null) return "";
-  return location.sourceText;
+  return location.sourceText();
 }
 
 
@@ -225,27 +225,68 @@ function GetSourceLine(message) {
        else the line number.
  */
 function ScriptLineFromPosition(position) {
-  var info = %ScriptPositionInfo(this, position, false);
-  return (info == null) ? -1 : info.line;
+  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;
 }
 
 
 /**
  * Get information on a specific source position.
- * Returns an object with the following following properties:
- *   script     : script object for the source
- *   line       : source line number
- *   column     : source column within the line
- *   position   : position within the source
- *   sourceText : a string containing the current line
  * @param {number} position The source position
  * @param {boolean} include_resource_offset Set to true to have the resource
  *     offset added to the location
- * @return If line is negative or not in the source null is returned.
+ * @return {SourceLocation}
+ *     If line is negative or not in the source null is returned.
  */
 function ScriptLocationFromPosition(position,
                                     include_resource_offset) {
-  return %ScriptPositionInfo(this, 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;
+
+  // 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);
 }
 
 
@@ -261,7 +302,8 @@ function ScriptLocationFromPosition(position,
  * @param {number} opt_offset_position The offset from the begining of the
  *     source from where the line and column calculation starts.
  *     Default value is 0
- * @return If line is negative or not in the source null is returned.
+ * @return {SourceLocation}
+ *     If line is negative or not in the source null is returned.
  */
 function ScriptLocationFromLine(opt_line, opt_column, opt_offset_position) {
   // Default is the first line in the script. Lines in the script is relative
@@ -291,7 +333,7 @@ function ScriptLocationFromLine(opt_line, opt_column, opt_offset_position) {
     }
 
     return this.locationFromPosition(
-        %ScriptLineStartPosition(this, offset_line + line) + column);
+        this.line_ends[offset_line + line - 1] + 1 + column);  // line > 0 here.
   }
 }
 
@@ -325,14 +367,15 @@ function ScriptSourceSlice(opt_from_line, opt_to_line) {
     return null;
   }
 
-  var from_position = %ScriptLineStartPosition(this, from_line);
-  var to_position = %ScriptLineStartPosition(this, to_line);
+  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;
 
   // Return a source slice with line numbers re-adjusted to the resource.
   return new SourceSlice(this,
                          from_line + this.line_offset,
                          to_line + this.line_offset,
-                         from_position, to_position);
+                          from_position, to_position);
 }
 
 
@@ -350,8 +393,9 @@ function ScriptSourceLine(opt_line) {
   }
 
   // Return the source line.
-  var start = %ScriptLineStartPosition(this, line);
-  var end = %ScriptLineEndPosition(this, line);
+  var line_ends = this.line_ends;
+  var start = line == 0 ? 0 : line_ends[line - 1] + 1;
+  var end = line_ends[line];
   return %_Call(StringSubstring, this.source, start, end);
 }
 
@@ -363,7 +407,17 @@ function ScriptSourceLine(opt_line) {
  */
 function ScriptLineCount() {
   // Return number of source lines.
-  return %ScriptLineCount(this);
+  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];
 }
 
 
@@ -388,6 +442,7 @@ utils.SetUpLockedPrototype(Script, [
     "name",
     "source_url",
     "source_mapping_url",
+    "line_ends",
     "line_offset",
     "column_offset"
   ], [
@@ -398,10 +453,58 @@ utils.SetUpLockedPrototype(Script, [
     "sourceLine", ScriptSourceLine,
     "lineCount", ScriptLineCount,
     "nameOrSourceURL", ScriptNameOrSourceURL,
+    "lineEnd", ScriptLineEnd
   ]
 );
 
 
+/**
+ * Class for source location. A source location is a position within some
+ * source with the following properties:
+ *   script   : script object for the source
+ *   line     : source line number
+ *   column   : source column within the line
+ *   position : position within the source
+ *   start    : position of start of source context (inclusive)
+ *   end      : position of end of source context (not inclusive)
+ * Source text for the source context is the character interval
+ * [start, end[. In most cases end will point to a newline character.
+ * It might point just past the final position of the source if the last
+ * source line does not end with a newline character.
+ * @param {Script} script The Script object for which this is a location
+ * @param {number} position Source position for the location
+ * @param {number} line The line number for the location
+ * @param {number} column The column within the line for the location
+ * @param {number} start Source position for start of source context
+ * @param {number} end Source position for end of source context
+ * @constructor
+ */
+function SourceLocation(script, position, line, column, start, end) {
+  this.script = script;
+  this.position = position;
+  this.line = line;
+  this.column = column;
+  this.start = start;
+  this.end = end;
+}
+
+
+/**
+ * Get the source text for a SourceLocation
+ * @return {String}
+ *     Source text for this location.
+ */
+function SourceLocationSourceText() {
+  return %_Call(StringSubstring, this.script.source, this.start, this.end);
+}
+
+
+utils.SetUpLockedPrototype(SourceLocation,
+  ["script", "position", "line", "column", "start", "end"],
+  ["sourceText", SourceLocationSourceText]
+);
+
+
 /**
  * Class for a source slice. A source slice is a part of a script source with
  * the following properties: