Refactored the logic for entering the debugger into one abstraction EnterDebu...

Refactored the logic for entering the debugger into one abstraction EnterDebugger.

Removed the static initializer for Top::break_access_.

Committed: http://code.google.com/p/v8/source/detail?r=421

[Kasper Lund, 2008-10-03 09:00:12]

LGTM with a few comments:

http://codereview.chromium.org/6234/diff/1/2
File src/debug.h (right):

http://codereview.chromium.org/6234/diff/1/2#newcode473
Line 473: // Enter the debugger.
Maybe put a better comment here? It's really a helper class that supports
entering the debugger, right?

http://codereview.chromium.org/6234/diff/1/2#newcode496
Line 496: }
This looks weird. Remove else?

http://codereview.chromium.org/6234/diff/1/2#newcode501
Line 501: // restore to the previous break state.
Capitalize comment.

http://codereview.chromium.org/6234/diff/1/2#newcode507
Line 507: inline bool Failed() { return load_failed; }
Rename to DidLoadFail?

http://codereview.chromium.org/6234/diff/1/2#newcode514
Line 514: bool load_failed;  // Did the debugger fail to load?
load_failed_

http://codereview.chromium.org/6234/diff/1/2#newcode515
Line 515: SaveContext save;  // Saves previous context.
save_

http://codereview.chromium.org/6234/diff/1/4
File src/runtime.cc (right):

http://codereview.chromium.org/6234/diff/1/4#newcode3114
Line 3114: EnterDebugger debugger;
Does entering the debugger using EnterDebugger require a HandleScope? It
probably does...

[Søren Thygesen Gjesse, 2008-10-03 09:53:14]

http://codereview.chromium.org/6234/diff/1/2
File src/debug.h (right):

http://codereview.chromium.org/6234/diff/1/2#newcode473
Line 473: // Enter the debugger.
On 2008/10/03 09:00:12, Kasper Lund wrote:
> Maybe put a better comment here? It's really a helper class that supports
> entering the debugger, right?

Added a comment describing what actually happens.

http://codereview.chromium.org/6234/diff/1/2#newcode496
Line 496: }
On 2008/10/03 09:00:12, Kasper Lund wrote:
> This looks weird. Remove else?

Removed.

http://codereview.chromium.org/6234/diff/1/2#newcode501
Line 501: // restore to the previous break state.
On 2008/10/03 09:00:12, Kasper Lund wrote:
> Capitalize comment.

Done.

http://codereview.chromium.org/6234/diff/1/2#newcode507
Line 507: inline bool Failed() { return load_failed; }
On 2008/10/03 09:00:12, Kasper Lund wrote:
> Rename to DidLoadFail?

Changed to FailedToEnter as it is supposed to detect all possible failures (even
though only the loading can currently fail).

http://codereview.chromium.org/6234/diff/1/2#newcode514
Line 514: bool load_failed;  // Did the debugger fail to load?
On 2008/10/03 09:00:12, Kasper Lund wrote:
> load_failed_

Renamed.

http://codereview.chromium.org/6234/diff/1/2#newcode515
Line 515: SaveContext save;  // Saves previous context.
On 2008/10/03 09:00:12, Kasper Lund wrote:
> save_

Renamed.

http://codereview.chromium.org/6234/diff/1/4
File src/runtime.cc (right):

http://codereview.chromium.org/6234/diff/1/4#newcode3114
Line 3114: EnterDebugger debugger;
On 2008/10/03 09:00:12, Kasper Lund wrote:
> Does entering the debugger using EnterDebugger require a HandleScope? It
> probably does...

It does, as EnterDebugger includes a SaveContext instance which uses Handles.