[inspector] V8DebuggerAgent cleanup

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

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/inspector/v8-debugger-agent-impl.h"
 
 #include <algorithm>
 
 #include "src/debug/debug-interface.h"
 #include "src/inspector/injected-script.h"
@@ skipping 116 matching lines @@
     : m_inspector(session->inspector()),
       m_debugger(m_inspector->debugger()),
       m_session(session),
       m_enabled(false),
       m_state(state),
       m_frontend(frontendChannel),
       m_isolate(m_inspector->isolate()),
       m_breakReason(protocol::Debugger::Paused::ReasonEnum::Other),
       m_scheduledDebuggerStep(NoStep),
       m_javaScriptPauseScheduled(false),
-      m_recursionLevelForStepOut(0),
-      m_skipAllPauses(false) {
+      m_recursionLevelForStepOut(0) {
   clearBreakDetails();
 }
 
 V8DebuggerAgentImpl::~V8DebuggerAgentImpl() {}
 
 void V8DebuggerAgentImpl::enableImpl() {
-  // m_inspector->addListener may result in reporting all parsed scripts to
-  // the agent so it should already be in enabled state by then.
   m_enabled = true;
   m_state->setBoolean(DebuggerAgentState::debuggerEnabled, true);
   m_debugger->enable();
 
   std::vector<std::unique_ptr<V8DebuggerScript>> compiledScripts;
   m_debugger->getCompiledScripts(m_session->contextGroupId(), compiledScripts);
   for (size_t i = 0; i < compiledScripts.size(); i++)
     didParseSource(std::move(compiledScripts[i]), true);
 
   // FIXME(WK44513): breakpoints activated flag should be synchronized between
@@ skipping 15 matching lines @@
 
 Response V8DebuggerAgentImpl::disable() {
   if (!enabled()) return Response::OK();
 
   m_state->setObject(DebuggerAgentState::javaScriptBreakpoints,
                      protocol::DictionaryValue::create());
   m_state->setInteger(DebuggerAgentState::pauseOnExceptionsState,
                       v8::debug::NoBreakOnException);
   m_state->setInteger(DebuggerAgentState::asyncCallStackDepth, 0);
 
-  if (!m_pausedContext.IsEmpty()) m_debugger->continueProgram();
+  if (isPaused()) m_debugger->continueProgram();
   m_debugger->disable();
-  m_pausedContext.Reset();
   JavaScriptCallFrames emptyCallFrames;
   m_pausedCallFrames.swap(emptyCallFrames);
   m_blackboxedPositions.clear();
   m_blackboxPattern.reset();
   resetBlackboxedStateCache();
   m_scripts.clear();
   m_breakpointIdToDebuggerBreakpointIds.clear();
   m_debugger->setAsyncCallStackDepth(this, 0);
   m_continueToLocationBreakpointId = String16();
   clearBreakDetails();
   m_scheduledDebuggerStep = NoStep;
   m_javaScriptPauseScheduled = false;
-  m_skipAllPauses = false;
   m_state->remove(DebuggerAgentState::blackboxPattern);
   m_enabled = false;
   m_state->setBoolean(DebuggerAgentState::debuggerEnabled, false);
   return Response::OK();
 }
 
 void V8DebuggerAgentImpl::restore() {
   DCHECK(!m_enabled);
   if (!m_state->booleanProperty(DebuggerAgentState::debuggerEnabled, false))
     return;
   if (!m_inspector->client()->canExecuteScripts(m_session->contextGroupId()))
     return;
 
   enableImpl();
 
   int pauseState = v8::debug::NoBreakOnException;
   m_state->getInteger(DebuggerAgentState::pauseOnExceptionsState, &pauseState);
   setPauseOnExceptionsImpl(pauseState);
 
-  m_skipAllPauses =
-      m_state->booleanProperty(DebuggerAgentState::skipAllPauses, false);
-
   int asyncCallStackDepth = 0;
   m_state->getInteger(DebuggerAgentState::asyncCallStackDepth,
                       &asyncCallStackDepth);
   m_debugger->setAsyncCallStackDepth(this, asyncCallStackDepth);
 
   String16 blackboxPattern;
   if (m_state->getString(DebuggerAgentState::blackboxPattern,
                          &blackboxPattern)) {
     setBlackboxPattern(blackboxPattern);
   }
 }
 
 Response V8DebuggerAgentImpl::setBreakpointsActive(bool active) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
   m_debugger->setBreakpointsActivated(active);
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::setSkipAllPauses(bool skip) {
-  m_skipAllPauses = skip;
-  m_state->setBoolean(DebuggerAgentState::skipAllPauses, m_skipAllPauses);
+  m_state->setBoolean(DebuggerAgentState::skipAllPauses, skip);
   return Response::OK();
 }
 
+bool V8DebuggerAgentImpl::skipAllPauses() const {
+  bool value = false;
+  return m_state->getBoolean(DebuggerAgentState::skipAllPauses, &value) &&

[dgozman, 2017/02/02 23:40:36]

Let's have m_skipAllPauses, as this is slow but method is hot.

[kozy, 2017/02/03 01:04:08]

Done.

+         value;
+}
+
 static std::unique_ptr<protocol::DictionaryValue>
 buildObjectForBreakpointCookie(const String16& url, int lineNumber,
                                int columnNumber, const String16& condition,
                                bool isRegex) {
   std::unique_ptr<protocol::DictionaryValue> breakpointObject =
       protocol::DictionaryValue::create();
   breakpointObject->setString(DebuggerAgentState::url, url);
   breakpointObject->setInteger(DebuggerAgentState::lineNumber, lineNumber);
   breakpointObject->setInteger(DebuggerAgentState::columnNumber, columnNumber);
   breakpointObject->setString(DebuggerAgentState::condition, condition);
@@ skipping 287 matching lines @@
   if (!response.isSuccess()) return response;
   *newCallFrames = std::move(callFrames);
   *asyncStackTrace = currentAsyncStackTrace();
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::restartFrame(
     const String16& callFrameId,
     std::unique_ptr<Array<CallFrame>>* newCallFrames,
     Maybe<StackTrace>* asyncStackTrace) {
-  if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
   InjectedScript::CallFrameScope scope(m_inspector, m_session->contextGroupId(),
                                        callFrameId);
   Response response = scope.initialize();
   if (!response.isSuccess()) return response;
   if (scope.frameOrdinal() >= m_pausedCallFrames.size())
     return Response::Error("Could not find call frame with given id");
 
   v8::Local<v8::Value> resultValue;
   v8::Local<v8::Boolean> result;
   if (!m_pausedCallFrames[scope.frameOrdinal()]->restart().ToLocal(
@@ skipping 20 matching lines @@
     return Response::Error("No script for id: " + scriptId);
   v8::HandleScope handles(m_isolate);
   *scriptSource = it->second->source(m_isolate);
   return Response::OK();
 }
 
 void V8DebuggerAgentImpl::schedulePauseOnNextStatement(
     const String16& breakReason,
     std::unique_ptr<protocol::DictionaryValue> data) {
   if (!enabled() || m_scheduledDebuggerStep == StepInto ||
-      m_javaScriptPauseScheduled || m_debugger->isPaused() ||
+      m_javaScriptPauseScheduled || isPaused() ||
       !m_debugger->breakpointsActivated())
     return;
   m_breakReason = breakReason;
   m_breakAuxData = std::move(data);
   m_debugger->setPauseOnNextStatement(true);
 }
 
 void V8DebuggerAgentImpl::schedulePauseOnNextStatementIfSteppingInto() {
   DCHECK(enabled());
   if (m_scheduledDebuggerStep != StepInto || m_javaScriptPauseScheduled ||
-      m_debugger->isPaused())
+      isPaused())
     return;
   clearBreakDetails();
   m_debugger->setPauseOnNextStatement(true);
 }
 
 void V8DebuggerAgentImpl::cancelPauseOnNextStatement() {
-  if (m_javaScriptPauseScheduled || m_debugger->isPaused()) return;
+  if (m_javaScriptPauseScheduled || isPaused()) return;
   clearBreakDetails();
   m_debugger->setPauseOnNextStatement(false);
 }
 
 Response V8DebuggerAgentImpl::pause() {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
-  if (m_javaScriptPauseScheduled || m_debugger->isPaused())

[dgozman, 2017/02/02 23:40:36]

Let's merge V8Debugger::isPaused() with m_runningNestedMessageLoop, and make it
private.

[kozy, 2017/02/03 01:04:08]

Done.

-    return Response::OK();
+  if (m_javaScriptPauseScheduled || isPaused()) return Response::OK();
   clearBreakDetails();
   m_javaScriptPauseScheduled = true;
   m_scheduledDebuggerStep = NoStep;
   m_debugger->setPauseOnNextStatement(true);
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::resume() {
-  if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
   m_scheduledDebuggerStep = NoStep;
   m_session->releaseObjectGroup(kBacktraceObjectGroup);
   m_debugger->continueProgram();
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::stepOver() {
-  if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
   // StepOver at function return point should fallback to StepInto.
   JavaScriptCallFrame* frame =
       !m_pausedCallFrames.empty() ? m_pausedCallFrames[0].get() : nullptr;
-  if (frame && frame->isAtReturn()) return stepInto();
+  if (frame && frame->isAtReturn()) {
+    return stepInto();

[dgozman, 2017/02/02 23:40:36]

Please undo.

[kozy, 2017/02/03 01:04:08]

Done.

+  }
   m_scheduledDebuggerStep = StepOver;
   m_session->releaseObjectGroup(kBacktraceObjectGroup);
   m_debugger->stepOverStatement();
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::stepInto() {
-  if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
   m_scheduledDebuggerStep = StepInto;
   m_session->releaseObjectGroup(kBacktraceObjectGroup);
   m_debugger->stepIntoStatement();
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::stepOut() {
-  if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
   m_scheduledDebuggerStep = StepOut;
   m_recursionLevelForStepOut = 1;
   m_session->releaseObjectGroup(kBacktraceObjectGroup);
   m_debugger->stepOutOfFunction();
   return Response::OK();
 }
 
 Response V8DebuggerAgentImpl::setPauseOnExceptions(
     const String16& stringPauseState) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
@@ skipping 17 matching lines @@
       static_cast<v8::debug::ExceptionBreakState>(pauseState));
   m_state->setInteger(DebuggerAgentState::pauseOnExceptionsState, pauseState);
 }
 
 Response V8DebuggerAgentImpl::evaluateOnCallFrame(
     const String16& callFrameId, const String16& expression,
     Maybe<String16> objectGroup, Maybe<bool> includeCommandLineAPI,
     Maybe<bool> silent, Maybe<bool> returnByValue, Maybe<bool> generatePreview,
     std::unique_ptr<RemoteObject>* result,
     Maybe<protocol::Runtime::ExceptionDetails>* exceptionDetails) {
-  if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
   InjectedScript::CallFrameScope scope(m_inspector, m_session->contextGroupId(),
                                        callFrameId);
   Response response = scope.initialize();
   if (!response.isSuccess()) return response;
   if (scope.frameOrdinal() >= m_pausedCallFrames.size())
     return Response::Error("Could not find call frame with given id");
 
   if (includeCommandLineAPI.fromMaybe(false)) scope.installCommandLineAPI();
   if (silent.fromMaybe(false)) scope.ignoreExceptionsAndMuteConsole();
 
   v8::MaybeLocal<v8::Value> maybeResultValue =
       m_pausedCallFrames[scope.frameOrdinal()]->evaluate(
           toV8String(m_isolate, expression));
 
   // Re-initialize after running client's code, as it could have destroyed
   // context or session.
   response = scope.initialize();
   if (!response.isSuccess()) return response;
   return scope.injectedScript()->wrapEvaluateResult(
       maybeResultValue, scope.tryCatch(), objectGroup.fromMaybe(""),
       returnByValue.fromMaybe(false), generatePreview.fromMaybe(false), result,
       exceptionDetails);
 }
 
 Response V8DebuggerAgentImpl::setVariableValue(
     int scopeNumber, const String16& variableName,
     std::unique_ptr<protocol::Runtime::CallArgument> newValueArgument,
     const String16& callFrameId) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
-  if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
   InjectedScript::CallFrameScope scope(m_inspector, m_session->contextGroupId(),
                                        callFrameId);
   Response response = scope.initialize();
   if (!response.isSuccess()) return response;
   v8::Local<v8::Value> newValue;
   response = scope.injectedScript()->resolveCallArgument(newValueArgument.get(),
                                                          &newValue);
   if (!response.isSuccess()) return response;
 
   if (scope.frameOrdinal() >= m_pausedCallFrames.size())
@@ skipping 88 matching lines @@
         "Input positions array is not sorted or contains duplicate values.");
   }
 
   m_blackboxedPositions[scriptId] = positions;
   it->second->resetBlackboxedStateCache();
   return Response::OK();
 }
 
 void V8DebuggerAgentImpl::willExecuteScript(int scriptId) {
   changeJavaScriptRecursionLevel(+1);
-  // Fast return.
   if (m_scheduledDebuggerStep != StepInto) return;
   schedulePauseOnNextStatementIfSteppingInto();
 }
 
 void V8DebuggerAgentImpl::didExecuteScript() {
   changeJavaScriptRecursionLevel(-1);
 }
 
 void V8DebuggerAgentImpl::changeJavaScriptRecursionLevel(int step) {
-  if (m_javaScriptPauseScheduled && !m_skipAllPauses &&
-      !m_debugger->isPaused()) {
+  if (m_javaScriptPauseScheduled && !isPaused()) {
     // Do not ever loose user's pause request until we have actually paused.
     m_debugger->setPauseOnNextStatement(true);
   }
   if (m_scheduledDebuggerStep == StepOut) {
     m_recursionLevelForStepOut += step;
     if (!m_recursionLevelForStepOut) {
       // When StepOut crosses a task boundary (i.e. js -> c++) from where it was
       // requested,
       // switch stepping to step into a next JS task, as if we exited to a
       // blackboxed framework.
       m_scheduledDebuggerStep = StepInto;
     }
   }
 }
 
 Response V8DebuggerAgentImpl::currentCallFrames(
     std::unique_ptr<Array<CallFrame>>* result) {
-  if (m_pausedContext.IsEmpty() || !m_pausedCallFrames.size()) {
+  if (!isPaused()) {
     *result = Array<CallFrame>::create();
     return Response::OK();
   }
   v8::HandleScope handles(m_isolate);
   v8::Local<v8::Context> debuggerContext =
       v8::debug::GetDebugContext(m_isolate);
   v8::Context::Scope contextScope(debuggerContext);
 
   v8::Local<v8::Array> objects = v8::Array::New(m_isolate);
 
@@ skipping 88 matching lines @@
   if (!response.isSuccess()) return response;
   protocol::ErrorSupport errorSupport;
   *result = Array<CallFrame>::fromValue(protocolValue.get(), &errorSupport);
   if (!*result) return Response::Error(errorSupport.errors());
   TranslateWasmStackTraceLocations(result->get(),
                                    m_debugger->wasmTranslation());
   return Response::OK();
 }
 
 std::unique_ptr<StackTrace> V8DebuggerAgentImpl::currentAsyncStackTrace() {
-  if (m_pausedContext.IsEmpty()) return nullptr;
+  if (!isPaused()) return nullptr;
   V8StackTraceImpl* stackTrace = m_debugger->currentAsyncCallChain();
   return stackTrace ? stackTrace->buildInspectorObjectForTail(m_debugger)
                     : nullptr;
 }
 
 void V8DebuggerAgentImpl::didParseSource(
     std::unique_ptr<V8DebuggerScript> script, bool success) {
   v8::HandleScope handles(m_isolate);
   String16 scriptSource = script->source(m_isolate);
   if (!success) script->setSourceURL(findSourceURL(scriptSource, false));
@@ skipping 68 matching lines @@
                                  &breakpoint.column_number);
     breakpointObject->getString(DebuggerAgentState::condition,
                                 &breakpoint.condition);
     std::unique_ptr<protocol::Debugger::Location> location =
         resolveBreakpoint(cookie.first, breakpoint, UserBreakpointSource);
     if (location)
       m_frontend.breakpointResolved(cookie.first, std::move(location));
   }
 }
 
-bool V8DebuggerAgentImpl::didPause(v8::Local<v8::Context> context,
+void V8DebuggerAgentImpl::didPause(int contextId,
                                    v8::Local<v8::Value> exception,
                                    const std::vector<String16>& hitBreakpoints,
                                    bool isPromiseRejection, bool isUncaught,
                                    bool isOOMBreak) {
-  if (!isOOMBreak) {
-    if (m_skipAllPauses) return false;
-    JavaScriptCallFrames callFrames = m_debugger->currentCallFrames(1);
-    JavaScriptCallFrame* topCallFrame =
-        !callFrames.empty() ? callFrames.begin()->get() : nullptr;
-    // Skip pauses inside V8 internal scripts and on syntax errors.
-    if (!topCallFrame) return false;
-  }
-  DCHECK(m_pausedContext.IsEmpty());
+  DCHECK(!isPaused());
   JavaScriptCallFrames frames = m_debugger->currentCallFrames();
   m_pausedCallFrames.swap(frames);
-  m_pausedContext.Reset(m_isolate, context);
   v8::HandleScope handles(m_isolate);
 
   if (isOOMBreak) {
     m_breakReason = protocol::Debugger::Paused::ReasonEnum::OOM;
     m_breakAuxData = nullptr;
   } else if (!exception.IsEmpty()) {
     InjectedScript* injectedScript = nullptr;
-    m_session->findInjectedScript(InspectedContext::contextId(context),
-                                  injectedScript);
+    m_session->findInjectedScript(contextId, injectedScript);
     if (injectedScript) {
       m_breakReason =
           isPromiseRejection
               ? protocol::Debugger::Paused::ReasonEnum::PromiseRejection
               : protocol::Debugger::Paused::ReasonEnum::Exception;
       std::unique_ptr<protocol::Runtime::RemoteObject> obj;
       injectedScript->wrapObject(exception, kBacktraceObjectGroup, false, false,
                                  &obj);
       if (obj) {
         m_breakAuxData = obj->toValue();
@@ skipping 27 matching lines @@
   m_frontend.paused(std::move(protocolCallFrames), m_breakReason,
                     std::move(m_breakAuxData), std::move(hitBreakpointIds),
                     currentAsyncStackTrace());
   m_scheduledDebuggerStep = NoStep;
   m_javaScriptPauseScheduled = false;
 
   if (!m_continueToLocationBreakpointId.isEmpty()) {
     m_debugger->removeBreakpoint(m_continueToLocationBreakpointId);
     m_continueToLocationBreakpointId = "";
   }
-  return true;
 }
 
 void V8DebuggerAgentImpl::didContinue() {
-  m_pausedContext.Reset();
   JavaScriptCallFrames emptyCallFrames;
   m_pausedCallFrames.swap(emptyCallFrames);
   clearBreakDetails();
   m_frontend.resumed();
 }
 
 void V8DebuggerAgentImpl::breakProgram(
     const String16& breakReason,
     std::unique_ptr<protocol::DictionaryValue> data) {
-  if (!enabled() || m_skipAllPauses || !m_pausedContext.IsEmpty() ||
-      !m_debugger->canBreakProgram())
-    return;
+  if (!enabled() || !m_debugger->canBreakProgram()) return;

[dgozman, 2017/02/02 23:40:36]

Check for skipAllPauses.

[kozy, 2017/02/03 01:04:08]

Done.

   m_breakReason = breakReason;
   m_breakAuxData = std::move(data);
   m_scheduledDebuggerStep = NoStep;
   m_debugger->breakProgram();
 }
 
 void V8DebuggerAgentImpl::breakProgramOnException(
     const String16& breakReason,
     std::unique_ptr<protocol::DictionaryValue> data) {
   if (!enabled() ||
@@ skipping 27 matching lines @@
 void V8DebuggerAgentImpl::reset() {
   if (!enabled()) return;
   m_scheduledDebuggerStep = NoStep;
   m_blackboxedPositions.clear();
   resetBlackboxedStateCache();
   m_scripts.clear();
   m_breakpointIdToDebuggerBreakpointIds.clear();
 }
 
 }  // namespace v8_inspector