Reviewers: Søren Gjesse, Description: For ScriptCollected events current context may be null. Message.GetEventContext will return an empty handle in such cases.
Please review this at http://codereview.chromium.org/113698 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/debug.cc M test/cctest/test-debug.cc Index: test/cctest/test-debug.cc =================================================================== --- test/cctest/test-debug.cc (revision 2022) +++ test/cctest/test-debug.cc (working copy) @@ -4817,7 +4817,7 @@ v8::HandleScope scope; DebugLocalContext env; - // Request the loaded scripte to initialize the debugger script cache. + // Request the loaded scripts to initialize the debugger script cache. Debug::GetLoadedScripts(); // Do garbage collection to ensure that only the script in this test will be @@ -4841,3 +4841,58 @@ v8::Debug::SetDebugEventListener(NULL); CheckDebuggerUnloaded(); } + + +// Debug event listener which counts the script collected events. +int script_collected_message_count = 0; +static void ScriptCollectedMessageHandler(const v8::Debug::Message& message) { + // Count the number of breaks. + if (message.IsEvent() && message.GetEvent() == v8::ScriptCollected) { + script_collected_message_count++; + v8::Handle<v8::Context> context = message.GetEventContext(); + CHECK(context.IsEmpty()); + } +} + + +// Test that GetEventContext doesn't fail and return empty handle for +// ScriptCollected events. +TEST(ScriptCollectedEventContext) { + break_point_hit_count = 0; + v8::HandleScope scope; + + { // Scope for the DebugLocalContext. + DebugLocalContext env; + + // Request the loaded scripts to initialize the debugger script cache. + Debug::GetLoadedScripts(); + + // Do garbage collection to ensure that only the script in this test will be + // collected afterwards. + Heap::CollectAllGarbage(); + + script_collected_count = 0; + v8::Debug::SetMessageHandler2(ScriptCollectedMessageHandler); + { + v8::Script::Compile(v8::String::New("eval('a=1')"))->Run(); + v8::Script::Compile(v8::String::New("eval('a=2')"))->Run(); + } + } + + //Handle<i::Context> context = Debug::debugger_entry()->GetContext(); + //bool isEmpty = context.is_null(); + //bool isNull = (*context == NULL); + + // Do garbage collection to collect the script above which is no longer + // referenced. +// Heap::CollectAllGarbageIfContextDisposed(); + i::Context* cc = i::Top::context(); + Heap::CollectAllGarbage(); + + CHECK_EQ(2, script_collected_message_count); + + v8::Debug::SetMessageHandler2(NULL); + // Should SetMessageHandler2 cause execution of Debugger::UnloadDebugger like + // Debugger::SetEventListener does? + // CheckDebuggerUnloaded(); +} Index: src/debug.cc =================================================================== --- src/debug.cc (revision 2022) +++ src/debug.cc (working copy) @@ -2400,6 +2400,11 @@ v8::Handle<v8::Context> MessageImpl::GetEventContext() const { Handle<Context> context = Debug::debugger_entry()->GetContext(); + // Top::context() may have been NULL when the event occured. + if (*context == NULL) { + ASSERT(event_ == v8::ScriptCollected); + return v8::Local<v8::Context>(); + } Handle<Context> global_context(context->global_context()); return v8::Utils::ToLocal(global_context); } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
