From 12bc4034e0fc0b9bed34dd74a5d3099f26ccd2ba Mon Sep 17 00:00:00 2001
From: "sgjesse@chromium.org"
 <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Fri, 3 Oct 2008 09:57:18 +0000
Subject: [PATCH] Refactored the logic for entering the debugger into one
 abstraction EnterDebugger.

Removed the static initializer for Top::break_access_.
Review URL: http://codereview.chromium.org/6234

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@421 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/debug.cc        | 35 +++++++++++++++++------------------
 src/debug.h         | 46 +++++++++++++++++++++++++---------------------
 src/mirror-delay.js |  2 +-
 src/runtime.cc      | 11 +++++++----
 src/top.cc          |  3 ++-
 5 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/src/debug.cc b/src/debug.cc
index b6925aa..e3fcb7b 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -631,8 +631,11 @@ Object* Debug::Break(Arguments args) {
     return Heap::undefined_value();
   }
 
-  SaveBreakFrame save;
-  EnterDebuggerContext enter;
+  // Enter the debugger.
+  EnterDebugger debugger;
+  if (debugger.FailedToEnter()) {
+    return Heap::undefined_value();
+  }
 
   // Postpone interrupt during breakpoint processing.
   PostponeInterruptsScope postpone;
@@ -1415,10 +1418,9 @@ void Debugger::OnException(Handle<Object> exception, bool uncaught) {
     if (!Debug::break_on_exception()) return;
   }
 
-  // Enter the debugger.  Bail out if the debugger cannot be loaded.
-  if (!Debug::Load()) return;
-  SaveBreakFrame save;
-  EnterDebuggerContext enter;
+  // Enter the debugger.
+  EnterDebugger debugger;
+  if (debugger.FailedToEnter()) return;
 
   // Clear all current stepping setup.
   Debug::ClearStepping();
@@ -1479,10 +1481,9 @@ void Debugger::OnBeforeCompile(Handle<Script> script) {
   if (compiling_natives()) return;
   if (!EventActive(v8::BeforeCompile)) return;
 
-  // Enter the debugger.  Bail out if the debugger cannot be loaded.
-  if (!Debug::Load()) return;
-  SaveBreakFrame save;
-  EnterDebuggerContext enter;
+  // Enter the debugger.
+  EnterDebugger debugger;
+  if (debugger.FailedToEnter()) return;
 
   // Create the event data object.
   bool caught_exception = false;
@@ -1509,10 +1510,9 @@ void Debugger::OnAfterCompile(Handle<Script> script, Handle<JSFunction> fun) {
   // No more to do if not debugging.
   if (!debugger_active()) return;
 
-  // Enter the debugger.  Bail out if the debugger cannot be loaded.
-  if (!Debug::Load()) return;
-  SaveBreakFrame save;
-  EnterDebuggerContext enter;
+  // Enter the debugger.
+  EnterDebugger debugger;
+  if (debugger.FailedToEnter()) return;
 
   // If debugging there might be script break points registered for this
   // script. Make sure that these break points are set.
@@ -1567,10 +1567,9 @@ void Debugger::OnNewFunction(Handle<JSFunction> function) {
   if (compiling_natives()) return;
   if (!Debugger::EventActive(v8::NewFunction)) return;
 
-  // Enter the debugger.  Bail out if the debugger cannot be loaded.
-  if (!Debug::Load()) return;
-  SaveBreakFrame save;
-  EnterDebuggerContext enter;
+  // Enter the debugger.
+  EnterDebugger debugger;
+  if (debugger.FailedToEnter()) return;
 
   // Create the event object.
   bool caught_exception = false;
diff --git a/src/debug.h b/src/debug.h
index 52e8a98..94cd806 100644
--- a/src/debug.h
+++ b/src/debug.h
@@ -31,6 +31,7 @@
 #include "../include/v8-debug.h"
 #include "assembler.h"
 #include "code-stubs.h"
+#include "execution.h"
 #include "factory.h"
 #include "platform.h"
 #include "string-stream.h"
@@ -469,10 +470,15 @@ class DebugMessageThread: public Thread {
 };
 
 
-// Helper class to support saving/restoring the top break frame id.
-class SaveBreakFrame {
+// This class is used for entering the debugger. Create an instance in the stack
+// to enter the debugger. This will set the current break state, make sure the
+// debugger is loaded and switch to the debugger context. If the debugger for
+// some reason could not be entered FailedToEnter will return true.
+class EnterDebugger BASE_EMBEDDED {
  public:
-  SaveBreakFrame() : set_(!it_.done()) {
+  EnterDebugger() : set_(!it_.done()) {
+    // If there is no JavaScript frames on the stack don't switch to new break
+    // and break frame.
     if (set_) {
       // Store the previous break is and frame id.
       break_id_ = Top::break_id();
@@ -481,36 +487,34 @@ class SaveBreakFrame {
       // Create the new break info.
       Top::new_break(it_.frame()->id());
     }
+
+    // Make sure that debugger is loaded and enter the debugger context.
+    load_failed_ = !Debug::Load();
+    if (!load_failed_) {
+      // NOTE the member variable save which saves the previous context before
+      // this change.
+      Top::set_context(*Debug::debug_context());
+      Top::set_security_context(*Debug::debug_context());
+    }
   }
 
-  ~SaveBreakFrame() {
+  ~EnterDebugger() {
     if (set_) {
-      // restore to the previous break state.
+      // Restore to the previous break state.
       Top::set_break(break_frame_id_, break_id_);
     }
   }
 
+  // Check whether the debugger could be entered.
+  inline bool FailedToEnter() { return load_failed_; }
+
  private:
   JavaScriptFrameIterator it_;
   const bool set_;  // Was the break actually set?
   StackFrame::Id break_frame_id_;  // Previous break frame id.
   int break_id_;  // Previous break id.
-};
-
-
-class EnterDebuggerContext BASE_EMBEDDED {
- public:
-  // Enter the debugger by storing the previous top context and setting the
-  // current top context to the debugger context.
-  EnterDebuggerContext()  {
-    // NOTE the member variable save which saves the previous context before
-    // this change.
-    Top::set_context(*Debug::debug_context());
-    Top::set_security_context(*Debug::debug_context());
-  }
-
- private:
-  SaveContext save;
+  bool load_failed_;  // Did the debugger fail to load?
+  SaveContext save_;  // Saves previous context.
 };
 
 
diff --git a/src/mirror-delay.js b/src/mirror-delay.js
index 2e78e90..2a297b5 100644
--- a/src/mirror-delay.js
+++ b/src/mirror-delay.js
@@ -1608,7 +1608,7 @@ FrameMirror.prototype.invocationText = function() {
     // For constructor frames display new followed by the function name.
     result += 'new ';
     result += func.name() ? func.name() : '[anonymous]';
-  } else   if (this.isDebuggerFrame()) {
+  } else if (this.isDebuggerFrame()) {
     result += '[debugger]';
   } else {
     // If the receiver has a className which is 'global' don't display it.
diff --git a/src/runtime.cc b/src/runtime.cc
index 507fba1..1ac07c5 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -3096,8 +3096,8 @@ static Object* RuntimePreempt(Arguments args) {
 
 
 static Object* Runtime_DebugBreak(Arguments args) {
-  // Just continue if breaks are disabled or if we fail to load the debugger.
-  if (Debug::disable_break() || !Debug::Load()) {
+  // Just continue if breaks are disabled.
+  if (Debug::disable_break()) {
     return args[0];
   }
 
@@ -3118,8 +3118,11 @@ static Object* Runtime_DebugBreak(Arguments args) {
   StackGuard::Continue(DEBUGBREAK);
 
   HandleScope scope;
-  SaveBreakFrame save;
-  EnterDebuggerContext enter;
+  // Enter the debugger. Just continue if we fail to enter the debugger.
+  EnterDebugger debugger;
+  if (debugger.FailedToEnter()) {
+    return args[0];
+  }
 
   // Notify the debug event listeners.
   Debugger::OnDebugBreak(Factory::undefined_value());
diff --git a/src/top.cc b/src/top.cc
index 2320895..87db165 100644
--- a/src/top.cc
+++ b/src/top.cc
@@ -37,7 +37,7 @@
 namespace v8 { namespace internal {
 
 ThreadLocalTop Top::thread_local_;
-Mutex* Top::break_access_ = OS::CreateMutex();
+Mutex* Top::break_access_;
 StackFrame::Id Top::break_frame_id_;
 int Top::break_count_;
 int Top::break_id_;
@@ -222,6 +222,7 @@ void Top::Initialize() {
 
   InitializeThreadLocal();
 
+  break_access_ = OS::CreateMutex();
   break_frame_id_ = StackFrame::NO_ID;
   break_count_ = 0;
   break_id_ = 0;
-- 
2.7.4