Switch v8 inspector to stl collections

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

Index: third_party/WebKit/Source/platform/v8_inspector/V8DebuggerImpl.cpp
diff --git a/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerImpl.cpp b/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerImpl.cpp
index 387bc4b86693a1bf760088886c40ecfb7fc43dd3..d56dddd31cbed7459eb0f906e4389340fee7afa2 100644
--- a/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerImpl.cpp
+++ b/third_party/WebKit/Source/platform/v8_inspector/V8DebuggerImpl.cpp
@@ -157,9 +157,13 @@ 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;
 }
 
@@ -168,7 +172,7 @@ V8DebuggerAgentImpl* V8DebuggerImpl::findEnabledDebuggerAgent(v8::Local<v8::Cont
     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);
@@ -402,8 +406,10 @@ bool V8DebuggerImpl::setScriptSource(const String16& sourceID, const String16& n
         {
             *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.
@@ -444,7 +450,7 @@ JavaScriptCallFrames V8DebuggerImpl::currentCallFrames(int limit)
         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;
 }
@@ -475,13 +481,13 @@ void V8DebuggerImpl::handleProgramBreak(v8::Local<v8::Context> pausedContext, v8
     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()));
         }
     }
 
@@ -733,16 +739,17 @@ std::unique_ptr<V8StackTrace> V8DebuggerImpl::createStackTrace(v8::Local<v8::Sta
 
 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)
@@ -755,39 +762,42 @@ void V8DebuggerImpl::contextCreated(const V8ContextInfo& info)
     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));
-
-    std::unique_ptr<InspectedContext> contextOwner(new InspectedContext(this, info, contextId));
-    InspectedContext* inspectedContext = contextOwner.get();
-    m_contexts.get(info.contextGroupId)->set(contextId, std::move(contextOwner));
+    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.

 
-    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)
@@ -827,23 +837,26 @@ v8::Local<v8::Context> V8DebuggerImpl::regexContext()
 
 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