Skip to content

Commit f3dcdf8

Browse files
alexkozyCommit bot
alexkozy
authored and
Commit bot
committed
[inspector] introduced debug::SetBreakEventListener,SetExceptionEventListener
Inspector is moved to per-event-type callbacks instead of general v8::debug::SetDebugEventListener. It allows to: - remove any usage of v8::Debug::EventDetails in debug-interface, - avoid redundant JS call on each event to get properties of event objects, - introduce better pure C++ API for these events later. BUG=v8:5510 R=yangguo@chromium.org,jgruber@chromium.org,dgozman@chromium.org Review-Url: https://codereview.chromium.org/2622253004 Cr-Commit-Position: refs/heads/master@{#42483}
1 parent ea4f834 commit f3dcdf8

8 files changed

Lines changed: 134 additions & 232 deletions

File tree

‎src/api.cc‎

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8987,19 +8987,6 @@ MaybeLocal<Array> Debug::GetInternalProperties(Isolate* v8_isolate,
89878987
return Utils::ToLocal(result);
89888988
}
89898989

8990-
bool debug::SetDebugEventListener(Isolate* isolate, debug::EventCallback that,
8991-
Local<Value> data) {
8992-
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
8993-
ENTER_V8(i_isolate);
8994-
i::HandleScope scope(i_isolate);
8995-
i::Handle<i::Object> foreign = i_isolate->factory()->undefined_value();
8996-
if (that != NULL) {
8997-
foreign = i_isolate->factory()->NewForeign(FUNCTION_ADDR(that));
8998-
}
8999-
i_isolate->debug()->SetEventListener(foreign, Utils::OpenHandle(*data, true));
9000-
return true;
9001-
}
9002-
90038990
Local<Context> debug::GetDebugContext(Isolate* isolate) {
90048991
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
90058992
ENTER_V8(i_isolate);
@@ -9331,20 +9318,11 @@ MaybeLocal<UnboundScript> debug::CompileInspectorScript(Isolate* v8_isolate,
93319318
RETURN_ESCAPED(ToApiHandle<UnboundScript>(result));
93329319
}
93339320

9334-
void debug::SetAsyncTaskListener(Isolate* v8_isolate,
9335-
debug::AsyncTaskListener listener,
9336-
void* data) {
9337-
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
9338-
ENTER_V8(isolate);
9339-
isolate->debug()->SetAsyncTaskListener(listener, data);
9340-
}
9341-
9342-
void debug::SetCompileEventListener(Isolate* v8_isolate,
9343-
debug::CompileEventListener listener,
9344-
void* data) {
9321+
void debug::SetDebugEventListener(Isolate* v8_isolate,
9322+
debug::DebugEventListener* listener) {
93459323
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
93469324
ENTER_V8(isolate);
9347-
isolate->debug()->SetCompileEventListener(listener, data);
9325+
isolate->debug()->SetDebugEventListener(listener);
93489326
}
93499327

93509328
Local<String> CpuProfileNode::GetFunctionName() const {

‎src/debug/debug-interface.h‎

Lines changed: 17 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -16,52 +16,6 @@
1616
namespace v8 {
1717
namespace debug {
1818

19-
/**
20-
* An event details object passed to the debug event listener.
21-
*/
22-
class EventDetails : public v8::Debug::EventDetails {
23-
public:
24-
/**
25-
* Event type.
26-
*/
27-
virtual v8::DebugEvent GetEvent() const = 0;
28-
29-
/**
30-
* Access to execution state and event data of the debug event. Don't store
31-
* these cross callbacks as their content becomes invalid.
32-
*/
33-
virtual Local<Object> GetExecutionState() const = 0;
34-
virtual Local<Object> GetEventData() const = 0;
35-
36-
/**
37-
* Get the context active when the debug event happened. Note this is not
38-
* the current active context as the JavaScript part of the debugger is
39-
* running in its own context which is entered at this point.
40-
*/
41-
virtual Local<Context> GetEventContext() const = 0;
42-
43-
/**
44-
* Client data passed with the corresponding callback when it was
45-
* registered.
46-
*/
47-
virtual Local<Value> GetCallbackData() const = 0;
48-
49-
virtual ~EventDetails() {}
50-
};
51-
52-
/**
53-
* Debug event callback function.
54-
*
55-
* \param event_details object providing information about the debug event
56-
*
57-
* A EventCallback does not take possession of the event data,
58-
* and must not rely on the data persisting after the handler returns.
59-
*/
60-
typedef void (*EventCallback)(const EventDetails& event_details);
61-
62-
bool SetDebugEventListener(Isolate* isolate, EventCallback that,
63-
Local<Value> data = Local<Value>());
64-
6519
/**
6620
* Debugger is running in its own context which is entered while debugger
6721
* messages are being dispatched. This is an explicit getter for this
@@ -193,17 +147,23 @@ void GetLoadedScripts(Isolate* isolate, PersistentValueVector<Script>& scripts);
193147
MaybeLocal<UnboundScript> CompileInspectorScript(Isolate* isolate,
194148
Local<String> source);
195149

196-
typedef std::function<void(debug::PromiseDebugActionType type, int id,
197-
void* data)>
198-
AsyncTaskListener;
199-
void SetAsyncTaskListener(Isolate* isolate, AsyncTaskListener listener,
200-
void* data);
201-
202-
typedef std::function<void(v8::Local<Script> script, bool has_compile_error,
203-
void* data)>
204-
CompileEventListener;
205-
void SetCompileEventListener(Isolate* isolate, CompileEventListener listener,
206-
void* data);
150+
class DebugEventListener {
151+
public:
152+
virtual ~DebugEventListener() {}
153+
virtual void PromiseEventOccurred(debug::PromiseDebugActionType type,
154+
int id) {}
155+
virtual void ScriptCompiled(v8::Local<Script> script,
156+
bool has_compile_error) {}
157+
virtual void BreakProgramRequested(v8::Local<v8::Context> paused_context,
158+
v8::Local<v8::Object> exec_state,
159+
v8::Local<v8::Value> break_points_hit) {}
160+
virtual void ExceptionThrown(v8::Local<v8::Context> paused_context,
161+
v8::Local<v8::Object> exec_state,
162+
v8::Local<v8::Value> exception,
163+
bool is_promise_rejection, bool is_uncaught) {}
164+
};
165+
166+
void SetDebugEventListener(Isolate* isolate, DebugEventListener* listener);
207167

208168
} // namespace debug
209169
} // namespace v8

‎src/debug/debug.cc‎

Lines changed: 55 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,16 @@ void Debug::OnPromiseReject(Handle<Object> promise, Handle<Object> value) {
17151715
}
17161716
}
17171717

1718+
namespace {
1719+
v8::Local<v8::Context> GetDebugEventContext(Isolate* isolate) {
1720+
Handle<Context> context = isolate->debug()->debugger_entry()->GetContext();
1721+
// Isolate::context() may have been NULL when "script collected" event
1722+
// occured.
1723+
if (context.is_null()) return v8::Local<v8::Context>();
1724+
Handle<Context> native_context(context->native_context());
1725+
return v8::Utils::ToLocal(native_context);
1726+
}
1727+
} // anonymous namespace
17181728

17191729
void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
17201730
// We cannot generate debug events when JS execution is disallowed.
@@ -1754,6 +1764,21 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
17541764
DebugScope debug_scope(this);
17551765
if (debug_scope.failed()) return;
17561766

1767+
if (debug_event_listener_) {
1768+
HandleScope scope(isolate_);
1769+
1770+
// Create the execution state.
1771+
Handle<Object> exec_state;
1772+
// Bail out and don't call debugger if exception.
1773+
if (!MakeExecutionState().ToHandle(&exec_state)) return;
1774+
1775+
debug_event_listener_->ExceptionThrown(
1776+
GetDebugEventContext(isolate_),
1777+
v8::Utils::ToLocal(Handle<JSObject>::cast(exec_state)),
1778+
v8::Utils::ToLocal(exception), promise->IsJSObject(), uncaught);
1779+
if (!non_inspector_listener_exists()) return;
1780+
}
1781+
17571782
// Create the event data object.
17581783
Handle<Object> event_data;
17591784
// Bail out and don't call debugger if exception.
@@ -1777,6 +1802,24 @@ void Debug::OnDebugBreak(Handle<Object> break_points_hit, bool auto_continue) {
17771802
PrintBreakLocation();
17781803
#endif // DEBUG
17791804

1805+
if (debug_event_listener_) {
1806+
HandleScope scope(isolate_);
1807+
1808+
// Create the execution state.
1809+
Handle<Object> exec_state;
1810+
// Bail out and don't call debugger if exception.
1811+
if (!MakeExecutionState().ToHandle(&exec_state)) return;
1812+
1813+
bool previous = in_debug_event_listener_;
1814+
in_debug_event_listener_ = true;
1815+
debug_event_listener_->BreakProgramRequested(
1816+
GetDebugEventContext(isolate_),
1817+
v8::Utils::ToLocal(Handle<JSObject>::cast(exec_state)),
1818+
v8::Utils::ToLocal(break_points_hit));
1819+
in_debug_event_listener_ = previous;
1820+
if (!non_inspector_listener_exists()) return;
1821+
}
1822+
17801823
HandleScope scope(isolate_);
17811824
// Create the event data object.
17821825
Handle<Object> event_data;
@@ -1854,18 +1897,11 @@ int Debug::NextAsyncTaskId(Handle<JSObject> promise) {
18541897
return async_id->value();
18551898
}
18561899

1857-
void Debug::SetAsyncTaskListener(debug::AsyncTaskListener listener,
1858-
void* data) {
1859-
async_task_listener_ = listener;
1860-
async_task_listener_data_ = data;
1861-
UpdateState();
1862-
}
1863-
18641900
void Debug::OnAsyncTaskEvent(debug::PromiseDebugActionType type, int id) {
18651901
if (in_debug_scope() || ignore_events()) return;
18661902

1867-
if (async_task_listener_) {
1868-
async_task_listener_(type, id, async_task_listener_data_);
1903+
if (debug_event_listener_) {
1904+
debug_event_listener_->PromiseEventOccurred(type, id);
18691905
if (!non_inspector_listener_exists()) return;
18701906
}
18711907

@@ -1920,7 +1956,7 @@ void Debug::CallEventCallback(v8::DebugEvent event,
19201956
in_debug_event_listener_ = true;
19211957
if (event_listener_->IsForeign()) {
19221958
// Invoke the C debug event listener.
1923-
debug::EventCallback callback = FUNCTION_CAST<debug::EventCallback>(
1959+
v8::Debug::EventCallback callback = FUNCTION_CAST<v8::Debug::EventCallback>(
19241960
Handle<Foreign>::cast(event_listener_)->foreign_address());
19251961
EventDetailsImpl event_details(event,
19261962
Handle<JSObject>::cast(exec_state),
@@ -1945,13 +1981,6 @@ void Debug::CallEventCallback(v8::DebugEvent event,
19451981
in_debug_event_listener_ = previous;
19461982
}
19471983

1948-
void Debug::SetCompileEventListener(debug::CompileEventListener listener,
1949-
void* data) {
1950-
compile_event_listener_ = listener;
1951-
compile_event_listener_data_ = data;
1952-
UpdateState();
1953-
}
1954-
19551984
void Debug::ProcessCompileEvent(v8::DebugEvent event, Handle<Script> script) {
19561985
if (ignore_events()) return;
19571986
if (script->type() != i::Script::TYPE_NORMAL &&
@@ -1963,10 +1992,9 @@ void Debug::ProcessCompileEvent(v8::DebugEvent event, Handle<Script> script) {
19631992
DebugScope debug_scope(this);
19641993
if (debug_scope.failed()) return;
19651994

1966-
if (compile_event_listener_) {
1967-
compile_event_listener_(ToApiHandle<debug::Script>(script),
1968-
event != v8::AfterCompile,
1969-
compile_event_listener_data_);
1995+
if (debug_event_listener_) {
1996+
debug_event_listener_->ScriptCompiled(ToApiHandle<debug::Script>(script),
1997+
event != v8::AfterCompile);
19701998
if (!non_inspector_listener_exists()) return;
19711999
}
19722000

@@ -2162,10 +2190,14 @@ void Debug::SetMessageHandler(v8::Debug::MessageHandler handler) {
21622190
}
21632191
}
21642192

2193+
void Debug::SetDebugEventListener(debug::DebugEventListener* listener) {
2194+
debug_event_listener_ = listener;
2195+
UpdateState();
2196+
}
2197+
21652198
void Debug::UpdateState() {
21662199
bool is_active = message_handler_ != nullptr || !event_listener_.is_null() ||
2167-
async_task_listener_ != nullptr ||
2168-
compile_event_listener_ != nullptr;
2200+
debug_event_listener_ != nullptr;
21692201
if (is_active || in_debug_scope()) {
21702202
// Note that the debug context could have already been loaded to
21712203
// bootstrap test cases.
@@ -2503,17 +2535,6 @@ v8::Local<v8::String> MessageImpl::GetJSON() const {
25032535
}
25042536
}
25052537

2506-
namespace {
2507-
v8::Local<v8::Context> GetDebugEventContext(Isolate* isolate) {
2508-
Handle<Context> context = isolate->debug()->debugger_entry()->GetContext();
2509-
// Isolate::context() may have been NULL when "script collected" event
2510-
// occured.
2511-
if (context.is_null()) return v8::Local<v8::Context>();
2512-
Handle<Context> native_context(context->native_context());
2513-
return v8::Utils::ToLocal(native_context);
2514-
}
2515-
} // anonymous namespace
2516-
25172538
v8::Local<v8::Context> MessageImpl::GetEventContext() const {
25182539
Isolate* isolate = event_data_->GetIsolate();
25192540
v8::Local<v8::Context> context = GetDebugEventContext(isolate);

‎src/debug/debug.h‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ class MessageImpl : public v8::Debug::Message {
286286
};
287287

288288
// Details of the debug event delivered to the debug event listener.
289-
class EventDetailsImpl : public debug::EventDetails {
289+
class EventDetailsImpl : public v8::Debug::EventDetails {
290290
public:
291291
EventDetailsImpl(DebugEvent event,
292292
Handle<JSObject> exec_state,
@@ -466,9 +466,7 @@ class Debug {
466466

467467
int NextAsyncTaskId(Handle<JSObject> promise);
468468

469-
void SetAsyncTaskListener(debug::AsyncTaskListener listener, void* data);
470-
void SetCompileEventListener(debug::CompileEventListener listener,
471-
void* data);
469+
void SetDebugEventListener(debug::DebugEventListener* listener);
472470

473471
// Returns whether the operation succeeded. Compilation can only be triggered
474472
// if a valid closure is passed as the second argument, otherwise the shared
@@ -679,10 +677,7 @@ class Debug {
679677

680678
v8::Debug::MessageHandler message_handler_;
681679

682-
debug::AsyncTaskListener async_task_listener_ = nullptr;
683-
void* async_task_listener_data_ = nullptr;
684-
debug::CompileEventListener compile_event_listener_ = nullptr;
685-
void* compile_event_listener_data_ = nullptr;
680+
debug::DebugEventListener* debug_event_listener_ = nullptr;
686681

687682
static const int kQueueInitialSize = 4;
688683
base::Semaphore command_received_; // Signaled for each command received.

‎src/inspector/debugger-script.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,10 @@ DebuggerScript.setBreakpointsActivated = function(execState, info)
239239
}
240240

241241
/**
242-
* @param {!BreakEvent} eventData
242+
* @param {!Array<!BreakPoint>|undefined} breakpoints
243243
*/
244-
DebuggerScript.getBreakpointNumbers = function(eventData)
244+
DebuggerScript.getBreakpointNumbers = function(breakpoints)
245245
{
246-
var breakpoints = eventData.breakPointsHit();
247246
var numbers = [];
248247
if (!breakpoints)
249248
return numbers;

‎src/inspector/debugger_script_externs.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -159,13 +159,6 @@ BreakPoint.prototype.script_break_point = function() {}
159159
BreakPoint.prototype.number = function() {}
160160

161161

162-
/** @interface */
163-
function BreakEvent() {}
164-
165-
/** @return {!Array<!BreakPoint>|undefined} */
166-
BreakEvent.prototype.breakPointsHit = function() {}
167-
168-
169162
/** @interface */
170163
function ExecutionState() {}
171164

0 commit comments

Comments
 (0)