Switch v8 inspector to stl collections

third_party/WebKit/Source/platform/v8_inspector/V8DebuggerImpl.cpp

 /*
  * Copyright (c) 2010-2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 139 matching lines @@
 void V8DebuggerImpl::debuggerAgentDisabled()
 {
     if (!--m_enabledAgentsCount)
         disable();
 }
 
 V8DebuggerAgentImpl* V8DebuggerImpl::findEnabledDebuggerAgent(int contextGroupId)
 {
     if (!contextGroupId)
         return nullptr;
-    V8InspectorSessionImpl* session = m_sessions.get(contextGroupId);
-    if (session && session->debuggerAgent()->enabled())
-        return session->debuggerAgent();
+    SessionMap::iterator iter = m_sessions.find(contextGroupId);
+    if (iter != m_sessions.end() && iter->second) {

[dgozman, 2016/06/24 17:01:13]

Prefer early bail-out:
if (iter == m_sessions.end())
    return nullptr;

[dgozman, 2016/06/24 17:01:13]

iter->second is always non-null

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+        V8DebuggerAgentImpl* agent = iter->second->debuggerAgent();
+        if (agent->enabled()) {

[dgozman, 2016/06/24 17:01:13]

style: extra {}

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+            return agent;
+        }
+    }
     return nullptr;
 }
 
 V8DebuggerAgentImpl* V8DebuggerImpl::findEnabledDebuggerAgent(v8::Local<v8::Context> context)
 {
     return findEnabledDebuggerAgent(getGroupId(context));
 }
 
-void V8DebuggerImpl::getCompiledScripts(int contextGroupId, protocol::Vector<V8DebuggerParsedScript>& result)
+void V8DebuggerImpl::getCompiledScripts(int contextGroupId, std::vector<V8DebuggerParsedScript>& result)
 {
     v8::HandleScope scope(m_isolate);
     v8::MicrotasksScope microtasks(m_isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
     v8::Local<v8::Object> debuggerScript = m_debuggerScript.Get(m_isolate);
     DCHECK(!debuggerScript->IsUndefined());
     v8::Local<v8::Function> getScriptsFunction = v8::Local<v8::Function>::Cast(debuggerScript->Get(v8InternalizedString("getScripts")));
     v8::Local<v8::Value> argv[] = { v8::Integer::New(m_isolate, contextGroupId) };
     v8::Local<v8::Value> value;
     if (!getScriptsFunction->Call(debuggerContext(), debuggerScript, PROTOCOL_ARRAY_LENGTH(argv), argv).ToLocal(&value))
         return;
@@ skipping 213 matching lines @@
         v8result = maybeResult.ToLocalChecked();
     }
     DCHECK(!v8result.IsEmpty());
     v8::Local<v8::Object> resultTuple = v8result->ToObject(m_isolate);
     int code = static_cast<int>(resultTuple->Get(0)->ToInteger(m_isolate)->Value());
     switch (code) {
     case 0:
         {
             *stackChanged = resultTuple->Get(1)->BooleanValue();
             // Call stack may have changed after if the edited function was on the stack.
-            if (!preview && isPaused())
-                newCallFrames->swap(currentCallFrames());
+            if (!preview && isPaused()) {
+                JavaScriptCallFrames frames = currentCallFrames();
+                newCallFrames->swap(frames);
+            }
             return true;
         }
     // Compile error.
     case 1:
         {
             *errorData = protocol::Debugger::SetScriptSourceError::create()
                 .setMessage(toProtocolStringWithTypeCheck(resultTuple->Get(2)))
                 .setLineNumber(resultTuple->Get(3)->ToInteger(m_isolate)->Value())
                 .setColumnNumber(resultTuple->Get(4)->ToInteger(m_isolate)->Value()).build();
             return false;
@@ skipping 20 matching lines @@
         return JavaScriptCallFrames();
     v8::Local<v8::Array> callFramesArray = currentCallFramesV8.As<v8::Array>();
     JavaScriptCallFrames callFrames;
     for (size_t i = 0; i < callFramesArray->Length(); ++i) {
         v8::Local<v8::Value> callFrameValue;
         if (!callFramesArray->Get(debuggerContext(), i).ToLocal(&callFrameValue))
             return JavaScriptCallFrames();
         if (!callFrameValue->IsObject())
             return JavaScriptCallFrames();
         v8::Local<v8::Object> callFrameObject = callFrameValue.As<v8::Object>();
-        callFrames.append(JavaScriptCallFrame::create(debuggerContext(), v8::Local<v8::Object>::Cast(callFrameObject)));
+        callFrames.push_back(JavaScriptCallFrame::create(debuggerContext(), v8::Local<v8::Object>::Cast(callFrameObject)));
     }
     return callFrames;
 }
 
 static V8DebuggerImpl* toV8DebuggerImpl(v8::Local<v8::Value> data)
 {
     void* p = v8::Local<v8::External>::Cast(data)->Value();
     return static_cast<V8DebuggerImpl*>(p);
 }
 
@@ skipping 10 matching lines @@
 void V8DebuggerImpl::handleProgramBreak(v8::Local<v8::Context> pausedContext, v8::Local<v8::Object> executionState, v8::Local<v8::Value> exception, v8::Local<v8::Array> hitBreakpointNumbers, bool isPromiseRejection)
 {
     // Don't allow nested breaks.
     if (m_runningNestedMessageLoop)
         return;
 
     V8DebuggerAgentImpl* agent = findEnabledDebuggerAgent(pausedContext);
     if (!agent)
         return;
 
-    protocol::Vector<String16> breakpointIds;
+    std::vector<String16> breakpointIds;
     if (!hitBreakpointNumbers.IsEmpty()) {
-        breakpointIds.resize(hitBreakpointNumbers->Length());
+        breakpointIds.reserve(hitBreakpointNumbers->Length());
         for (size_t i = 0; i < hitBreakpointNumbers->Length(); i++) {
             v8::Local<v8::Value> hitBreakpointNumber = hitBreakpointNumbers->Get(i);
             DCHECK(!hitBreakpointNumber.IsEmpty() && hitBreakpointNumber->IsInt32());
-            breakpointIds[i] = String16::number(hitBreakpointNumber->Int32Value());
+            breakpointIds.push_back(String16::number(hitBreakpointNumber->Int32Value()));
         }
     }
 
     m_pausedContext = pausedContext;
     m_executionState = executionState;
     V8DebuggerAgentImpl::SkipPauseRequest result = agent->didPause(pausedContext, exception, breakpointIds, isPromiseRejection);
     if (result == V8DebuggerAgentImpl::RequestNoSkip) {
         m_runningNestedMessageLoop = true;
         int groupId = getGroupId(pausedContext);
         DCHECK(groupId);
@@ skipping 231 matching lines @@
 }
 
 std::unique_ptr<V8StackTrace> V8DebuggerImpl::createStackTrace(v8::Local<v8::StackTrace> stackTrace)
 {
     V8DebuggerAgentImpl* agent = findEnabledDebuggerAgent(m_isolate->GetCurrentContext());
     return V8StackTraceImpl::create(agent, stackTrace, V8StackTrace::maxCallStackSizeToCapture);
 }
 
 std::unique_ptr<V8InspectorSession> V8DebuggerImpl::connect(int contextGroupId, protocol::FrontendChannel* channel, V8InspectorSessionClient* client, const String16* state)
 {
-    DCHECK(!m_sessions.contains(contextGroupId));
-    std::unique_ptr<V8InspectorSessionImpl> session = V8InspectorSessionImpl::create(this, contextGroupId, channel, client, state);
-    m_sessions.set(contextGroupId, session.get());
+    DCHECK(m_sessions.find(contextGroupId) == m_sessions.cend());
+    std::unique_ptr<V8InspectorSessionImpl> session =
+        V8InspectorSessionImpl::create(this, contextGroupId, channel, client, state);
+    m_sessions[contextGroupId] = session.get();
     return std::move(session);
 }
 
 void V8DebuggerImpl::disconnect(V8InspectorSessionImpl* session)
 {
-    DCHECK(m_sessions.contains(session->contextGroupId()));
-    m_sessions.remove(session->contextGroupId());
+    DCHECK(m_sessions.find(session->contextGroupId()) != m_sessions.end());
+    m_sessions.erase(session->contextGroupId());
 }
 
 void V8DebuggerImpl::contextCreated(const V8ContextInfo& info)
 {
     DCHECK(info.context->GetIsolate() == m_isolate);
     // TODO(dgozman): make s_lastContextId non-static.
     int contextId = atomicIncrement(&s_lastContextId);
     String16 debugData = String16::number(info.contextGroupId) + "," + String16::number(contextId) + "," + (info.isDefault ? "default" : "nondefault");
     v8::HandleScope scope(m_isolate);
     v8::Context::Scope contextScope(info.context);
     info.context->SetEmbedderData(static_cast<int>(v8::Context::kDebugIdIndex), toV8String(m_isolate, debugData));
 
-    if (!m_contexts.contains(info.contextGroupId))
-        m_contexts.set(info.contextGroupId, wrapUnique(new ContextByIdMap()));
-    DCHECK(!m_contexts.get(info.contextGroupId)->contains(contextId));
+    const auto& contextById =
+        m_contexts.insert(std::make_pair(info.contextGroupId, wrapUnique(new ContextByIdMap()))).first->second;

[dgozman, 2016/06/24 17:01:13]

I think this creates an extra ContextByIdMap most of the times just to delete
it.

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

 
-    std::unique_ptr<InspectedContext> contextOwner(new InspectedContext(this, info, contextId));
-    InspectedContext* inspectedContext = contextOwner.get();
-    m_contexts.get(info.contextGroupId)->set(contextId, std::move(contextOwner));
-
-    if (V8InspectorSessionImpl* session = m_sessions.get(info.contextGroupId))
-        session->runtimeAgent()->reportExecutionContextCreated(inspectedContext);
+    DCHECK(contextById->find(contextId) == contextById->end());

[dgozman, 2016/06/24 17:01:13]

cend?

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+    InspectedContext* context = new InspectedContext(this, info, contextId);
+    bool inserted = contextById->insert(std::make_pair(contextId, wrapUnique(context))).second;

[dgozman, 2016/06/24 17:01:13]

Why not just
contextById[contextId] = wrapUnique(context);

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+    DCHECK(inserted);
+    SessionMap::iterator session = m_sessions.find(info.contextGroupId);

[dgozman, 2016/06/24 17:01:13]

nit: sessionIt

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+    if (session != m_sessions.end() && session->second) {

[dgozman, 2016/06/24 17:01:13]

No need to check for session->second

[dgozman, 2016/06/24 17:01:13]

style: extra {}

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+        session->second->runtimeAgent()->reportExecutionContextCreated(context);
+    }
 }
 
 void V8DebuggerImpl::contextDestroyed(v8::Local<v8::Context> context)
 {
     int contextId = V8Debugger::contextId(context);
     int contextGroupId = getGroupId(context);
-    if (!m_contexts.contains(contextGroupId) || !m_contexts.get(contextGroupId)->contains(contextId))
+    if (!isContextInGroup(contextGroupId, contextId))

[dgozman, 2016/06/24 17:01:13]

Let's make this method return InspectedContext* and just check it for null.

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

         return;
 
-    InspectedContext* inspectedContext = m_contexts.get(contextGroupId)->get(contextId);
-    if (V8InspectorSessionImpl* session = m_sessions.get(contextGroupId))
-        session->runtimeAgent()->reportExecutionContextDestroyed(inspectedContext);
-
-    m_contexts.get(contextGroupId)->remove(contextId);
-    if (m_contexts.get(contextGroupId)->isEmpty())
-        m_contexts.remove(contextGroupId);
+    SessionMap::iterator iter = m_sessions.find(contextGroupId);
+    if (iter != m_sessions.end()) {
+        const std::unique_ptr<InspectedContext>& inspectedContext =
+            (*m_contexts[contextGroupId])[contextId];
+        iter->second->runtimeAgent()
+            ->reportExecutionContextDestroyed(inspectedContext.get());
+    }
+    discardInspectedContext(contextGroupId, contextId);
 }
 
 void V8DebuggerImpl::resetContextGroup(int contextGroupId)
 {
-    if (V8InspectorSessionImpl* session = m_sessions.get(contextGroupId))
-        session->reset();
-    m_contexts.remove(contextGroupId);
+    SessionMap::iterator session = m_sessions.find(contextGroupId);
+    if (session != m_sessions.end() && session->second)

[dgozman, 2016/06/24 17:01:13]

No need to check for session->second

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+        session->second->reset();
+    m_contexts.erase(contextGroupId);
 }
 
 void V8DebuggerImpl::willExecuteScript(v8::Local<v8::Context> context, int scriptId)
 {
     if (V8DebuggerAgentImpl* agent = findEnabledDebuggerAgent(context))
         agent->willExecuteScript(scriptId);
 }
 
 void V8DebuggerImpl::didExecuteScript(v8::Local<v8::Context> context)
 {
@@ skipping 19 matching lines @@
 
 v8::Local<v8::Context> V8DebuggerImpl::regexContext()
 {
     if (m_regexContext.IsEmpty())
         m_regexContext.Reset(m_isolate, v8::Context::New(m_isolate));
     return m_regexContext.Get(m_isolate);
 }
 
 void V8DebuggerImpl::discardInspectedContext(int contextGroupId, int contextId)
 {
-    if (!m_contexts.contains(contextGroupId) || !m_contexts.get(contextGroupId)->contains(contextId))
+    if (!isContextInGroup(contextGroupId, contextId))
         return;
-    m_contexts.get(contextGroupId)->remove(contextId);
-    if (m_contexts.get(contextGroupId)->isEmpty())
-        m_contexts.remove(contextGroupId);
+    m_contexts[contextGroupId]->erase(contextId);
+    if (m_contexts[contextGroupId]->empty())
+        m_contexts.erase(contextGroupId);
 }
 
 const V8DebuggerImpl::ContextByIdMap* V8DebuggerImpl::contextGroup(int contextGroupId)
 {
-    if (!m_contexts.contains(contextGroupId))
-        return nullptr;
-    return m_contexts.get(contextGroupId);
+    ContextsByGroupMap::iterator iter = m_contexts.find(contextGroupId);
+    return iter == m_contexts.end() ? nullptr : iter->second.get();
 }
 
 V8InspectorSessionImpl* V8DebuggerImpl::sessionForContextGroup(int contextGroupId)
 {
-    return contextGroupId ? m_sessions.get(contextGroupId) : nullptr;
+    if (contextGroupId) {

[dgozman, 2016/06/24 17:01:13]

Prefer early return instead.

[eostroukhov-old, 2016/06/24 22:24:25]

Done.

+        SessionMap::iterator iter = m_sessions.find(contextGroupId);
+        return iter == m_sessions.end() ? nullptr : iter->second;
+    }
+    return nullptr;
 }
 
 } // namespace blink