[wasm][debug] Simplify stepping interface

This is a minor refactoring before fixing actual issues.

1) The update of the {per_isolate_data_} is moved into
   {FloodWithBreakpoints}, which is already taking the mutex.
2) The {PrepareStep} method takes a {WasmFrame*} directly instead of its
   ID. In most cases, this prevents the creation of an additional stack
   frame iterator.

R=thibaudm@chromium.org

Bug: chromium:1145176
Change-Id: I1a6cd15550bbb4ef78ba522427bed1c23185569e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2558318Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71399}

src/debug/debug.cc

@@ -1102,8 +1102,8 @@ void Debug::PrepareStep(StepAction step_action) {
     wasm::WasmCodeRefScope code_ref_scope;
     wasm::WasmCode* code = wasm_frame->wasm_code();
     if (code->is_liftoff()) {
-      wasm_frame->native_module()->GetDebugInfo()->PrepareStep(isolate_,
-                                                               frame_id);
+      auto* debug_info = wasm_frame->native_module()->GetDebugInfo();
+      debug_info->PrepareStep(wasm_frame);
     }
     // In case the wasm code returns, prepare the next frame (if JS) to break.
     step_action = StepOut;

src/wasm/wasm-debug.cc

@@ -495,32 +495,26 @@ class DebugInfoImpl {
     WasmCode* new_code = RecompileLiftoffWithBreakpoints(
         frame->function_index(), VectorOf(&offset, 1), 0);
     UpdateReturnAddress(frame, new_code, return_location);
+
+    per_isolate_data_[frame->isolate()].stepping_frame = frame->id();
   }
 
-  void PrepareStep(Isolate* isolate, StackFrameId break_frame_id) {
-    StackTraceFrameIterator it(isolate, break_frame_id);
-    DCHECK(!it.done());
-    DCHECK(it.frame()->is_wasm());
-    WasmFrame* frame = WasmFrame::cast(it.frame());
-    StepAction step_action = isolate->debug()->last_step_action();
-
-    // If we are flooding the top frame, the return location is after a
-    // breakpoints. Otherwise, it's after a call.
-    ReturnLocation return_location = kAfterBreakpoint;
+  void PrepareStep(WasmFrame* frame) {
+    Isolate* isolate = frame->isolate();
 
     // If we are at a return instruction, then any stepping action is equivalent
     // to StepOut, and we need to flood the parent function.
-    if (IsAtReturn(frame) || step_action == StepOut) {
+    if (IsAtReturn(frame) || isolate->debug()->last_step_action() == StepOut) {
+      StackTraceFrameIterator it(isolate, isolate->debug()->break_frame_id());
+      DCHECK(!it.done());
+      DCHECK(it.frame()->is_wasm());
       it.Advance();
       if (it.done() || !it.frame()->is_wasm()) return;
       frame = WasmFrame::cast(it.frame());
-      return_location = kAfterWasmCall;
+      FloodWithBreakpoints(frame, kAfterWasmCall);
+    } else {
+      FloodWithBreakpoints(frame, kAfterBreakpoint);
     }
-
-    FloodWithBreakpoints(frame, return_location);
-
-    base::MutexGuard guard(&mutex_);
-    per_isolate_data_[isolate].stepping_frame = frame->id();
   }
 
   void ClearStepping(Isolate* isolate) {
@@ -880,9 +874,7 @@ void DebugInfo::SetBreakpoint(int func_index, int offset,
   impl_->SetBreakpoint(func_index, offset, current_isolate);
 }
 
-void DebugInfo::PrepareStep(Isolate* isolate, StackFrameId break_frame_id) {
-  impl_->PrepareStep(isolate, break_frame_id);
-}
+void DebugInfo::PrepareStep(WasmFrame* frame) { impl_->PrepareStep(frame); }
 
 void DebugInfo::ClearStepping(Isolate* isolate) {
   impl_->ClearStepping(isolate);

src/wasm/wasm-debug.h

@@ -170,7 +170,7 @@ class V8_EXPORT_PRIVATE DebugInfo {
 
   void SetBreakpoint(int func_index, int offset, Isolate* current_isolate);
 
-  void PrepareStep(Isolate*, StackFrameId);
+  void PrepareStep(WasmFrame*);
 
   void ClearStepping(Isolate*);