[inspector] entered into context before getPossibleBreakpoints call

src/inspector/v8-debugger-agent-impl.cc

Index: src/inspector/v8-debugger-agent-impl.cc
diff --git a/src/inspector/v8-debugger-agent-impl.cc b/src/inspector/v8-debugger-agent-impl.cc
index 431c297a744888b244eaec5a3ec4ac42c11693be..c059b0176c7f5a695064f2440a8e4c2b2348fb6b 100644
--- a/src/inspector/v8-debugger-agent-impl.cc
+++ b/src/inspector/v8-debugger-agent-impl.cc
@@ -457,10 +457,20 @@ Response V8DebuggerAgentImpl::getPossibleBreakpoints(
   auto it = m_scripts.find(scriptId);
   if (it == m_scripts.end()) return Response::Error("Script not found");
 
+  int contextId = it->second->executionContextId();
+  int contextGroupId = m_inspector->contextGroupId(contextId);
+  InspectedContext* inspected =
+      m_inspector->getContext(contextGroupId, contextId);
+  if (!inspected) return Response::InternalError();

[dgozman, 2017/04/28 20:57:23]

I don't like this return. Sometimes we don't have a context for a script - we
should handle it gracefully.

[kozy, 2017/04/28 21:17:10]

I think I can use debuggerContext to fix a crash and later think about this
within debugger context removing work - we currently enter debugger context when
setting a breakpoint for the same reason.

+  v8::HandleScope handleScope(m_isolate);
+  v8::Context::Scope contextScope(inspected->context());
+
   std::vector<v8::debug::BreakLocation> v8Locations;
-  if (!it->second->getPossibleBreakpoints(
-          v8Start, v8End, restrictToFunction.fromMaybe(false), &v8Locations)) {
-    return Response::InternalError();
+
+  {
+    v8::TryCatch tryCatch(m_isolate);

[dgozman, 2017/04/28 20:57:23]

Put all scopes inside of {} block?

[kozy, 2017/04/28 21:17:10]

Done.

+    it->second->getPossibleBreakpoints(
+        v8Start, v8End, restrictToFunction.fromMaybe(false), &v8Locations);
   }
 
   *locations = protocol::Array<protocol::Debugger::BreakLocation>::create();