From 6c7a746c5dfe9865399ea114d74412751061a222 Mon Sep 17 00:00:00 2001
From: "sgjesse@chromium.org"
 <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 18 Dec 2008 14:32:49 +0000
Subject: [PATCH] Refactored the code for handling debug step in in the runtime
 system into one function. For constructors this also means that step in will
 no longer step into the code for the builtins context. Review URL:
 http://codereview.chromium.org/15035

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1001 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/debug.cc                             | 27 ++++++++++++++++++++++++
 src/debug.h                              |  3 +++
 src/ic.cc                                | 16 +++++---------
 src/runtime.cc                           | 12 +++--------
 test/mjsunit/debug-stepin-constructor.js |  2 +-
 5 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/src/debug.cc b/src/debug.cc
index 027c89b64..ff32023b2 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -1095,6 +1095,33 @@ Handle<Object> Debug::GetSourceBreakLocations(
 }
 
 
+// Handle stepping into a function.
+void Debug::HandleStepIn(Handle<JSFunction> function,
+                         Address fp,
+                         bool is_constructor) {
+  // If the frame pointer is not supplied by the caller find it.
+  if (fp == 0) {
+    StackFrameIterator it;
+    it.Advance();
+    // For constructor functions skip another frame.
+    if (is_constructor) {
+      ASSERT(it.frame()->is_construct());
+      it.Advance();
+    }
+    fp = it.frame()->fp();
+  }
+
+  // Flood the function with one-shot break points if it is called from where
+  // step into was requested.
+  if (fp == Debug::step_in_fp()) {
+    // Don't allow step into functions in the native context.
+    if (function->context()->global() != Top::context()->builtins()) {
+      Debug::FloodWithOneShot(Handle<SharedFunctionInfo>(function->shared()));
+    }
+  }  
+}
+
+
 void Debug::ClearStepping() {
   // Clear the various stepping setup.
   ClearOneShot();
diff --git a/src/debug.h b/src/debug.h
index fdb80bd20..56b802622 100644
--- a/src/debug.h
+++ b/src/debug.h
@@ -205,6 +205,9 @@ class Debug {
   inline static bool has_break_points() { return has_break_points_; }
 
   static bool StepInActive() { return thread_local_.step_into_fp_ != 0; }
+  static void HandleStepIn(Handle<JSFunction> function,
+                           Address fp,
+                           bool is_constructor);
   static Address step_in_fp() { return thread_local_.step_into_fp_; }
   static Address* step_in_fp_addr() { return &thread_local_.step_into_fp_; }
 
diff --git a/src/ic.cc b/src/ic.cc
index 260a0fb7a..4b2e084f4 100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -352,20 +352,14 @@ Object* CallIC::LoadFunction(State state,
       if (opt->IsJSFunction()) return opt;
     }
 
-    // If performing debug step into then flood this function with one-shot
-    // break points if it is called from where step into was requested.
-    if (Debug::StepInActive() && fp() == Debug::step_in_fp()) {
+    // Handle stepping into a function if step into is active.
+    if (Debug::StepInActive()) {
       // Protect the result in a handle as the debugger can allocate and might
       // cause GC.
       HandleScope scope;
-      Handle<Object> result_handle(result);
-      // Don't allow step into functions in the native context.
-      if (JSFunction::cast(result)->context()->global() !=
-          Top::context()->builtins()) {
-        Handle<SharedFunctionInfo> shared(JSFunction::cast(result)->shared());
-        Debug::FloodWithOneShot(shared);
-      }
-      return *result_handle;
+      Handle<JSFunction> function(JSFunction::cast(result));
+      Debug::HandleStepIn(function, fp(), false);
+      return *function;
     }
 
     return result;
diff --git a/src/runtime.cc b/src/runtime.cc
index 69e2b6fd3..e79608100 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -3323,16 +3323,10 @@ static Object* Runtime_NewObject(Arguments args) {
   if (constructor->IsJSFunction()) {
     JSFunction* function = JSFunction::cast(constructor);
 
-    // Handle steping into constructors.
+    // Handle steping into constructors if step into is active.
     if (Debug::StepInActive()) {
-      StackFrameIterator it;
-      it.Advance();
-      ASSERT(it.frame()->is_construct());
-      it.Advance();
-      if (it.frame()->fp() == Debug::step_in_fp()) {
-        HandleScope scope;
-        Debug::FloodWithOneShot(Handle<SharedFunctionInfo>(function->shared()));
-      }
+      HandleScope scope;
+      Debug::HandleStepIn(Handle<JSFunction>(function), 0, true);
     }
 
     if (function->has_initial_map() &&
diff --git a/test/mjsunit/debug-stepin-constructor.js b/test/mjsunit/debug-stepin-constructor.js
index ecd128363..ec35ce163 100644
--- a/test/mjsunit/debug-stepin-constructor.js
+++ b/test/mjsunit/debug-stepin-constructor.js
@@ -68,7 +68,7 @@ function g() {
 
 break_break_point_hit_count = 0;
 g();
-assertEquals(5, break_break_point_hit_count);
+assertEquals(4, break_break_point_hit_count);
 
 // Get rid of the debug event listener.
 Debug.removeListener(listener);
-- 
2.34.1