[debug] removed live_edit flag and reimplemented LiveEdit::RestartFrame

It looks like we do not need live_edit flag.

R=dgozman@chromium.org,yangguo@chromium.org

Bug: v8:7862
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I2b635f7d24138894b7a0f94fc90293d50e40f22c
Reviewed-on: https://chromium-review.googlesource.com/1108386
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53980}

src/api.cc

@@ -9139,11 +9139,6 @@ v8_inspector::V8Inspector* debug::GetInspector(Isolate* isolate) {
   return reinterpret_cast<i::Isolate*>(isolate)->inspector();
 }
 
-void debug::SetLiveEditEnabled(Isolate* isolate, bool enable) {
-  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
-  internal_isolate->debug()->set_live_edit_enabled(enable);
-}
-
 void debug::SetBreakOnNextFunctionCall(Isolate* isolate) {
   reinterpret_cast<i::Isolate*>(isolate)->debug()->SetBreakOnNextFunctionCall();
 }

src/d8.cc

@@ -2350,8 +2350,6 @@ class InspectorClient : public v8_inspector::V8InspectorClient {
             .ToLocalChecked();
     CHECK(context->Global()->Set(context, function_name, function).FromJust());
 
-    v8::debug::SetLiveEditEnabled(isolate_, true);
-
     context_.Reset(isolate_, context);
   }
 

src/debug/debug-interface.h

@@ -34,13 +34,6 @@ int GetContextId(Local<Context> context);
 void SetInspector(Isolate* isolate, v8_inspector::V8Inspector*);
 v8_inspector::V8Inspector* GetInspector(Isolate* isolate);
 
-/**
- * Enable/disable LiveEdit functionality for the given Isolate
- * (default Isolate if not provided). V8 will abort if LiveEdit is
- * unexpectedly used. LiveEdit is enabled by default.
- */
-V8_EXPORT_PRIVATE void SetLiveEditEnabled(Isolate* isolate, bool enable);
-
 // Schedule a debugger break to happen when function is called inside given
 // isolate.
 void SetBreakOnNextFunctionCall(Isolate* isolate);

src/debug/debug.cc

@@ -83,7 +83,6 @@ Debug::Debug(Isolate* isolate)
       is_active_(false),
       hook_on_function_call_(false),
       is_suppressed_(false),
-      live_edit_enabled_(false),
       break_disabled_(false),
       break_points_active_(true),
       break_on_exception_(false),
@@ -1912,17 +1911,16 @@ bool Debug::SetScriptSource(Handle<Script> script, Handle<String> source,
     thread_local_.break_frame_id_ = frame_id;
   }
 
-  set_live_edit_enabled(true);
   Handle<Object> script_wrapper = Script::GetWrapper(script);
   Handle<Object> argv[] = {script_wrapper, source,
                            isolate_->factory()->ToBoolean(preview),
                            isolate_->factory()->NewJSArray(0)};
   Handle<Object> result;
   MaybeHandle<Object> maybe_exception;
+  running_live_edit_ = true;
   if (!CallFunction("SetScriptSource", arraysize(argv), argv, &maybe_exception)
            .ToHandle(&result)) {
     Handle<Object> pending_exception = maybe_exception.ToHandleChecked();
-    set_live_edit_enabled(false);
     if (pending_exception->IsJSObject()) {
       Handle<JSObject> exception = Handle<JSObject>::cast(pending_exception);
       Handle<String> message = Handle<String>::cast(
@@ -1947,15 +1945,16 @@ bool Debug::SetScriptSource(Handle<Script> script, Handle<String> source,
         output->message = Utils::ToLocal(error);
       }
     }
+    running_live_edit_ = false;
     return false;
   }
-  set_live_edit_enabled(false);
   Handle<Object> stack_changed_value =
       JSReceiver::GetProperty(isolate_, Handle<JSObject>::cast(result),
                               "stack_modified")
           .ToHandleChecked();
   output->stack_changed = stack_changed_value->IsTrue(isolate_);
   output->status = debug::LiveEditResult::OK;
+  running_live_edit_ = false;
   return true;
 }
 
@@ -1984,7 +1983,7 @@ void Debug::ProcessCompileEvent(bool has_compile_error, Handle<Script> script) {
   DisableBreak no_recursive_break(this);
   AllowJavascriptExecution allow_script(isolate_);
   debug_delegate_->ScriptCompiled(ToApiHandle<debug::Script>(script),
-                                  live_edit_enabled(), has_compile_error);
+                                  running_live_edit_, has_compile_error);
 }
 
 

src/debug/debug.h

@@ -348,11 +348,6 @@ class Debug {
   }
   inline Handle<Context> debug_context() { return debug_context_; }
 
-  void set_live_edit_enabled(bool v) { live_edit_enabled_ = v; }
-  bool live_edit_enabled() const {
-    return FLAG_enable_liveedit && live_edit_enabled_;
-  }
-
   inline bool is_active() const { return is_active_; }
   inline bool is_loaded() const { return !debug_context_.is_null(); }
   inline bool in_debug_scope() const {
@@ -499,8 +494,8 @@ class Debug {
   bool hook_on_function_call_;
   // Suppress debug events.
   bool is_suppressed_;
-  // LiveEdit is enabled.
-  bool live_edit_enabled_;
+  // Running liveedit.
+  bool running_live_edit_ = false;
   // Do not trigger debug break events.
   bool break_disabled_;
   // Do not break on break points.

src/debug/liveedit.cc

@@ -1419,63 +1419,37 @@ Handle<JSArray> LiveEdit::CheckAndDropActivations(
   return result;
 }
 
-
-// Describes a single callframe a target. Not finding this frame
-// means an error.
-class SingleFrameTarget {
- public:
-  explicit SingleFrameTarget(JavaScriptFrame* frame)
-      : m_frame(frame),
-        m_saved_status(LiveEdit::FUNCTION_AVAILABLE_FOR_PATCH) {}
-
-  bool MatchActivation(StackFrame* frame,
-      LiveEdit::FunctionPatchabilityStatus status) {
-    if (frame->fp() == m_frame->fp()) {
-      m_saved_status = status;
-      return true;
+bool LiveEdit::RestartFrame(JavaScriptFrame* frame) {
+  if (!LiveEdit::kFrameDropperSupported) return false;
+  Isolate* isolate = frame->isolate();
+  Zone zone(isolate->allocator(), ZONE_NAME);
+  Vector<StackFrame*> frames = CreateStackMap(isolate, &zone);
+  StackFrame::Id break_frame_id = isolate->debug()->break_frame_id();
+  bool break_frame_found = break_frame_id == StackFrame::NO_ID;
+  for (StackFrame* current : frames) {
+    break_frame_found = break_frame_found || break_frame_id == current->id();
+    if (current->fp() == frame->fp()) {
+      if (break_frame_found) {
+        isolate->debug()->ScheduleFrameRestart(current);
+        return true;
+      } else {
+        return false;
+      }
+    }
+    if (!break_frame_found) continue;
+    if (current->is_exit() || current->is_builtin_exit()) {
+      return false;
+    }
+    if (!current->is_java_script()) continue;
+    std::vector<Handle<SharedFunctionInfo>> shareds;
+    JavaScriptFrame::cast(current)->GetFunctions(&shareds);
+    for (auto& shared : shareds) {
+      if (IsResumableFunction(shared->kind())) {
+        return false;
+      }
     }
-    return false;
-  }
-  const char* GetNotFoundMessage() const {
-    return "Failed to found requested frame";
-  }
-  LiveEdit::FunctionPatchabilityStatus saved_status() {
-    return m_saved_status;
-  }
-  void set_status(LiveEdit::FunctionPatchabilityStatus status) {
-    m_saved_status = status;
-  }
-
-  bool FrameUsesNewTarget(StackFrame* frame) {
-    if (!frame->is_java_script()) return false;
-    JavaScriptFrame* jsframe = JavaScriptFrame::cast(frame);
-    Handle<SharedFunctionInfo> shared(jsframe->function()->shared(),
-                                      jsframe->isolate());
-    return shared->scope_info()->HasNewTarget();
-  }
-
- private:
-  JavaScriptFrame* m_frame;
-  LiveEdit::FunctionPatchabilityStatus m_saved_status;
-};
-
-// Finds a drops required frame and all frames above.
-// Returns error message or nullptr.
-const char* LiveEdit::RestartFrame(JavaScriptFrame* frame) {
-  SingleFrameTarget target(frame);
-
-  const char* result =
-      DropActivationsInActiveThreadImpl(frame->isolate(), target, true);
-  if (result != nullptr) {
-    return result;
-  }
-  if (target.saved_status() == LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE) {
-    return "Function is blocked under native code";
-  }
-  if (target.saved_status() == LiveEdit::FUNCTION_BLOCKED_UNDER_GENERATOR) {
-    return "Function is blocked under a generator activation";
   }
-  return nullptr;
+  return false;
 }
 
 Handle<JSArray> LiveEditFunctionTracker::Collect(FunctionLiteral* node,

src/debug/liveedit.h

@@ -135,8 +135,7 @@ class LiveEdit : AllStatic {
       bool do_drop);
 
   // Restarts the call frame and completely drops all frames above it.
-  // Return error message or nullptr.
-  static const char* RestartFrame(JavaScriptFrame* frame);
+  static bool RestartFrame(JavaScriptFrame* frame);
 
   static void CompareStrings(Isolate* isolate, Handle<String> a,
                              Handle<String> b,

src/flag-definitions.h

@@ -911,7 +911,6 @@ DEFINE_IMPLICATION(trace_array_abuse, trace_js_array_abuse)
 DEFINE_IMPLICATION(trace_array_abuse, trace_external_array_abuse)
 
 // debugger
-DEFINE_BOOL(enable_liveedit, true, "enable liveedit experimental feature")
 DEFINE_BOOL(
     trace_side_effect_free_debug_evaluate, false,
     "print debug messages for side-effect-free debug-evaluate for testing")

src/runtime/runtime-liveedit.cc

@@ -20,7 +20,6 @@ namespace internal {
 // in OpaqueReferences.
 RUNTIME_FUNCTION(Runtime_LiveEditFindSharedFunctionInfosForScript) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_CHECKED(JSValue, script_value, 0);
 
@@ -63,7 +62,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditFindSharedFunctionInfosForScript) {
 // with the function itself going first. The root function is a script function.
 RUNTIME_FUNCTION(Runtime_LiveEditGatherCompileInfo) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_CHECKED(JSValue, script, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, source, 1);
@@ -81,7 +79,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditGatherCompileInfo) {
 // the script with its original source and sends notification to debugger.
 RUNTIME_FUNCTION(Runtime_LiveEditReplaceScript) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(3, args.length());
   CONVERT_ARG_CHECKED(JSValue, original_script_value, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, new_source, 1);
@@ -105,7 +102,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditReplaceScript) {
 // SharedFunctionInfos.
 RUNTIME_FUNCTION(Runtime_LiveEditFixupScript) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(args.length(), 2);
   CONVERT_ARG_CHECKED(JSValue, script_value, 0);
   CONVERT_INT32_ARG_CHECKED(max_function_literal_id, 1);
@@ -119,7 +115,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditFixupScript) {
 
 RUNTIME_FUNCTION(Runtime_LiveEditFunctionSourceUpdated) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(args.length(), 2);
   CONVERT_ARG_HANDLE_CHECKED(JSArray, shared_info, 0);
   CONVERT_INT32_ARG_CHECKED(new_function_literal_id, 1);
@@ -133,7 +128,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditFunctionSourceUpdated) {
 // Replaces code of SharedFunctionInfo with a new one.
 RUNTIME_FUNCTION(Runtime_LiveEditReplaceFunctionCode) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSArray, new_compile_info, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSArray, shared_info, 1);
@@ -147,7 +141,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditReplaceFunctionCode) {
 // Connects SharedFunctionInfo to another script.
 RUNTIME_FUNCTION(Runtime_LiveEditFunctionSetScript) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(Object, function_object, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, script_object, 1);
@@ -174,7 +167,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditFunctionSetScript) {
 // with a substitution one.
 RUNTIME_FUNCTION(Runtime_LiveEditReplaceRefToNestedFunction) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(3, args.length());
 
   CONVERT_ARG_HANDLE_CHECKED(JSValue, parent_wrapper, 0);
@@ -197,7 +189,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditReplaceRefToNestedFunction) {
 // Each group describes a change in text; groups are sorted by change_begin.
 RUNTIME_FUNCTION(Runtime_LiveEditPatchFunctionPositions) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSArray, shared_array, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSArray, position_change_array, 1);
@@ -214,7 +205,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditPatchFunctionPositions) {
 // LiveEdit::FunctionPatchabilityStatus type.
 RUNTIME_FUNCTION(Runtime_LiveEditCheckAndDropActivations) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(3, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSArray, old_shared_array, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSArray, new_shared_array, 1);
@@ -252,7 +242,6 @@ RUNTIME_FUNCTION(Runtime_LiveEditCheckAndDropActivations) {
 // of diff chunks.
 RUNTIME_FUNCTION(Runtime_LiveEditCompareStrings) {
   HandleScope scope(isolate);
-  CHECK(isolate->debug()->live_edit_enabled());
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(String, s1, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, s2, 1);

test/cctest/test-debug.cc

@@ -3660,24 +3660,6 @@ TEST(DebuggerCreatesContextIffActive) {
   v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr);
 }
 
-
-TEST(LiveEditEnabled) {
-  v8::internal::FLAG_allow_natives_syntax = true;
-  LocalContext env;
-  v8::HandleScope scope(env->GetIsolate());
-  v8::debug::SetLiveEditEnabled(env->GetIsolate(), true);
-  CompileRun("%LiveEditCompareStrings('', '')");
-}
-
-
-TEST(LiveEditDisabled) {
-  v8::internal::FLAG_allow_natives_syntax = true;
-  LocalContext env;
-  v8::HandleScope scope(env->GetIsolate());
-  v8::debug::SetLiveEditEnabled(env->GetIsolate(), false);
-  CompileRun("%LiveEditCompareStrings('', '')");
-}
-
 class DebugBreakStackTraceListener : public v8::debug::DebugDelegate {
  public:
   void BreakProgramRequested(v8::Local<v8::Context> paused_context,