Web Inspector: InspectorCounters mechanism should be thread-safe

https://bugs.webkit.org/show_bug.cgi?id=80166 Make InspectorCounters instance thread local so that it can be safely accessed in workers. Reviewed by Pavel Feldman. * bindings/js/JSEventListener.cpp: (WebCore::JSEventListener::JSEventListener): (WebCore::JSEventListener::~JSEventListener): * bindings/v8/V8AbstractEventListener.cpp: (WebCore::V8AbstractEventListener::V8AbstractEventListener): (WebCore::V8AbstractEventListener::~V8AbstractEventListener): * dom/Document.cpp: (WebCore::Document::Document): (WebCore::Document::~Document): * dom/Document.h: (WebCore::Node::Node): * dom/Node.cpp: (WebCore::Node::~Node): * inspector/InspectorCounters.cpp: (WebCore::InspectorCounters::InspectorCounters): (WebCore::InspectorCounters::counterValue): (WebCore): (WebCore::InspectorCounters::current): * inspector/InspectorCounters.h: (WebCore::InspectorCounters::incrementCounter): (WebCore::InspectorCounters::decrementCounter): (InspectorCounters): * inspector/InspectorTimelineAgent.cpp: (WebCore::InspectorTimelineAgent::setHeapSizeStatistic): * platform/ThreadGlobalData.cpp: (WebCore::ThreadGlobalData::ThreadGlobalData): (WebCore::ThreadGlobalData::destroy): * platform/ThreadGlobalData.h: (WebCore): (ThreadGlobalData): (WebCore::ThreadGlobalData::inspectorCounters): Canonical link: https://commits.webkit.org/97559@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@109922 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Mar 6, 2012 · 0e29e27 · 0e29e27
1 parent 61e323d
commit 0e29e27
Show file tree

Hide file tree

Showing 11 changed files with 135 additions and 13 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,45 @@
+2012-03-02  Yury Semikhatsky  <yurys@chromium.org>
+
+        Web Inspector: InspectorCounters mechanism should be thread-safe
+        https://bugs.webkit.org/show_bug.cgi?id=80166
+
+        Make InspectorCounters instance thread local so that it can be safely accessed
+        in workers.
+
+        Reviewed by Pavel Feldman.
+
+        * bindings/js/JSEventListener.cpp:
+        (WebCore::JSEventListener::JSEventListener):
+        (WebCore::JSEventListener::~JSEventListener):
+        * bindings/v8/V8AbstractEventListener.cpp:
+        (WebCore::V8AbstractEventListener::V8AbstractEventListener):
+        (WebCore::V8AbstractEventListener::~V8AbstractEventListener):
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::~Document):
+        * dom/Document.h:
+        (WebCore::Node::Node):
+        * dom/Node.cpp:
+        (WebCore::Node::~Node):
+        * inspector/InspectorCounters.cpp:
+        (WebCore::InspectorCounters::InspectorCounters):
+        (WebCore::InspectorCounters::counterValue):
+        (WebCore):
+        (WebCore::InspectorCounters::current):
+        * inspector/InspectorCounters.h:
+        (WebCore::InspectorCounters::incrementCounter):
+        (WebCore::InspectorCounters::decrementCounter):
+        (InspectorCounters):
+        * inspector/InspectorTimelineAgent.cpp:
+        (WebCore::InspectorTimelineAgent::setHeapSizeStatistic):
+        * platform/ThreadGlobalData.cpp:
+        (WebCore::ThreadGlobalData::ThreadGlobalData):
+        (WebCore::ThreadGlobalData::destroy):
+        * platform/ThreadGlobalData.h:
+        (WebCore):
+        (ThreadGlobalData):
+        (WebCore::ThreadGlobalData::inspectorCounters):
+
 2012-03-06  Leo Yang  <leo.yang@torchmobile.com.cn>
 
         [BlackBerry] Upstream typedef of NativeImageSourcePtr and NativeImagePtr

diff --git a/Source/WebCore/bindings/js/JSEventListener.cpp b/Source/WebCore/bindings/js/JSEventListener.cpp
@@ -45,12 +45,16 @@ JSEventListener::JSEventListener(JSObject* function, JSObject* wrapper, bool isA
         m_jsFunction.setMayBeNull(*m_isolatedWorld->globalData(), wrapper, function);
     else
         ASSERT(!function);
-    InspectorCounters::incrementCounter(InspectorCounters::JSEventListenerCounter);
+#if ENABLE(INSPECTOR)
+    ThreadLocalInspectorCounters::current().incrementCounter(ThreadLocalInspectorCounters::JSEventListenerCounter);
+#endif
 }
 
 JSEventListener::~JSEventListener()
 {
-    InspectorCounters::decrementCounter(InspectorCounters::JSEventListenerCounter);
+#if ENABLE(INSPECTOR)
+    ThreadLocalInspectorCounters::current().decrementCounter(ThreadLocalInspectorCounters::JSEventListenerCounter);
+#endif
 }
 
 JSObject* JSEventListener::initializeJSFunction(ScriptExecutionContext*) const

diff --git a/Source/WebCore/bindings/v8/V8AbstractEventListener.cpp b/Source/WebCore/bindings/v8/V8AbstractEventListener.cpp
@@ -58,7 +58,9 @@ V8AbstractEventListener::V8AbstractEventListener(bool isAttribute, const WorldCo
     , m_isAttribute(isAttribute)
     , m_worldContext(worldContext)
 {
-    InspectorCounters::incrementCounter(InspectorCounters::JSEventListenerCounter);
+#if ENABLE(INSPECTOR)
+    ThreadLocalInspectorCounters::current().incrementCounter(ThreadLocalInspectorCounters::JSEventListenerCounter);
+#endif
 }
 
 V8AbstractEventListener::~V8AbstractEventListener()
@@ -69,7 +71,9 @@ V8AbstractEventListener::~V8AbstractEventListener()
         V8EventListenerList::clearWrapper(listener, m_isAttribute);
     }
     disposeListenerObject();
-    InspectorCounters::decrementCounter(InspectorCounters::JSEventListenerCounter);
+#if ENABLE(INSPECTOR)
+    ThreadLocalInspectorCounters::current().decrementCounter(ThreadLocalInspectorCounters::JSEventListenerCounter);
+#endif
 }
 
 void V8AbstractEventListener::handleEvent(ScriptExecutionContext* context, Event* event)

diff --git a/Source/WebCore/inspector/InspectorController.cpp b/Source/WebCore/inspector/InspectorController.cpp
@@ -111,7 +111,7 @@ InspectorController::InspectorController(Page* page, InspectorClient* inspectorC
     InspectorDOMStorageAgent* domStorageAgent = domStorageAgentPtr.get();
     m_agents.append(domStorageAgentPtr.release());
     m_agents.append(InspectorMemoryAgent::create(m_instrumentingAgents.get(), m_state.get(), m_page, m_domAgent));
-    m_agents.append(InspectorTimelineAgent::create(m_instrumentingAgents.get(), m_state.get()));
+    m_agents.append(InspectorTimelineAgent::create(m_instrumentingAgents.get(), m_state.get(), InspectorTimelineAgent::PageInspector));
     m_agents.append(InspectorApplicationCacheAgent::create(m_instrumentingAgents.get(), m_state.get(), pageAgent));
 
     OwnPtr<InspectorResourceAgent> resourceAgentPtr(InspectorResourceAgent::create(m_instrumentingAgents.get(), pageAgent, inspectorClient, m_state.get()));

diff --git a/Source/WebCore/inspector/InspectorCounters.cpp b/Source/WebCore/inspector/InspectorCounters.cpp
@@ -30,6 +30,7 @@
 
 #include "config.h"
 #include "InspectorCounters.h"
+#include "ThreadGlobalData.h"
 
 #if ENABLE(INSPECTOR)
 
@@ -42,6 +43,22 @@ int InspectorCounters::counterValue(CounterType type)
     return s_counters[type];
 }
 
+ThreadLocalInspectorCounters::ThreadLocalInspectorCounters()
+{
+    for (size_t i = 0; i < CounterTypeLength; i++)
+        m_counters[i] = 0;
+}
+
+int ThreadLocalInspectorCounters::counterValue(CounterType type)
+{
+    return m_counters[type];
+}
+
+ThreadLocalInspectorCounters& ThreadLocalInspectorCounters::current()
+{
+    return threadGlobalData().inspectorCounters();
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(INSPECTOR)
diff --git a/Source/WebCore/inspector/InspectorCounters.h b/Source/WebCore/inspector/InspectorCounters.h
@@ -31,6 +31,10 @@
 #ifndef InspectorCounters_h
 #define InspectorCounters_h
 
+#if !ASSERT_DISABLED
+#include <wtf/MainThread.h>
+#endif
+
 namespace WebCore {
 
 class InspectorCounters {
@@ -45,13 +49,15 @@ class InspectorCounters {
     static inline void incrementCounter(CounterType type)
     {
 #if ENABLE(INSPECTOR)
+        ASSERT(isMainThread());
         ++s_counters[type];
 #endif
     }
 
     static inline void decrementCounter(CounterType type)
     {
 #if ENABLE(INSPECTOR)
+        ASSERT(isMainThread());
         --s_counters[type];
 #endif
     }
@@ -68,6 +74,35 @@ class InspectorCounters {
 #endif
 };
 
+
+#if ENABLE(INSPECTOR)
+class ThreadLocalInspectorCounters {
+public:
+    enum CounterType {
+        JSEventListenerCounter,
+        CounterTypeLength
+    };
+    ThreadLocalInspectorCounters();
+
+    inline void incrementCounter(CounterType type)
+    {
+        ++m_counters[type];
+    }
+
+    inline void decrementCounter(CounterType type)
+    {
+        --m_counters[type];
+    }
+
+    int counterValue(CounterType);
+
+    static ThreadLocalInspectorCounters& current();
+
+private:
+    int m_counters[CounterTypeLength];
+};
+#endif
+
 } // namespace WebCore
 
 #endif // !defined(InspectorCounters_h)
diff --git a/Source/WebCore/inspector/InspectorTimelineAgent.cpp b/Source/WebCore/inspector/InspectorTimelineAgent.cpp
@@ -398,9 +398,9 @@ void InspectorTimelineAgent::setHeapSizeStatistic(InspectorObject* record)
 
     if (m_state->getBoolean(TimelineAgentState::includeMemoryDetails)) {
         RefPtr<InspectorObject> counters = InspectorObject::create();
-        counters->setNumber("nodes", InspectorCounters::counterValue(InspectorCounters::NodeCounter));
-        counters->setNumber("documents", InspectorCounters::counterValue(InspectorCounters::DocumentCounter));
-        counters->setNumber("jsEventListeners", InspectorCounters::counterValue(InspectorCounters::JSEventListenerCounter));
+        counters->setNumber("nodes", (m_inspectorType == PageInspector) ? InspectorCounters::counterValue(InspectorCounters::NodeCounter) : 0);
+        counters->setNumber("documents", (m_inspectorType == PageInspector) ? InspectorCounters::counterValue(InspectorCounters::DocumentCounter) : 0);
+        counters->setNumber("jsEventListeners", ThreadLocalInspectorCounters::current().counterValue(ThreadLocalInspectorCounters::JSEventListenerCounter));
         record->setObject("counters", counters.release());
     }
 }
@@ -421,11 +421,12 @@ void InspectorTimelineAgent::didCompleteCurrentRecord(const String& type)
     }
 }
 
-InspectorTimelineAgent::InspectorTimelineAgent(InstrumentingAgents* instrumentingAgents, InspectorState* state)
+InspectorTimelineAgent::InspectorTimelineAgent(InstrumentingAgents* instrumentingAgents, InspectorState* state, InspectorType type)
     : InspectorBaseAgent<InspectorTimelineAgent>("Timeline", instrumentingAgents, state)
     , m_frontend(0)
     , m_id(1)
     , m_maxCallStackDepth(5)
+    , m_inspectorType(type)
 {
 }
 

diff --git a/Source/WebCore/inspector/InspectorTimelineAgent.h b/Source/WebCore/inspector/InspectorTimelineAgent.h
@@ -56,9 +56,10 @@ typedef String ErrorString;
 class InspectorTimelineAgent : public InspectorBaseAgent<InspectorTimelineAgent>, ScriptGCEventListener, public InspectorBackendDispatcher::TimelineCommandHandler {
     WTF_MAKE_NONCOPYABLE(InspectorTimelineAgent);
 public:
-    static PassOwnPtr<InspectorTimelineAgent> create(InstrumentingAgents* instrumentingAgents, InspectorState* state)
+    enum InspectorType { PageInspector, WorkerInspector };
+    static PassOwnPtr<InspectorTimelineAgent> create(InstrumentingAgents* instrumentingAgents, InspectorState* state, InspectorType type)
     {
-        return adoptPtr(new InspectorTimelineAgent(instrumentingAgents, state));
+        return adoptPtr(new InspectorTimelineAgent(instrumentingAgents, state, type));
     }
 
     ~InspectorTimelineAgent();
@@ -142,7 +143,7 @@ class InspectorTimelineAgent : public InspectorBaseAgent<InspectorTimelineAgent>
         String type;
     };
 
-    InspectorTimelineAgent(InstrumentingAgents*, InspectorState*);
+    InspectorTimelineAgent(InstrumentingAgents*, InspectorState*, InspectorType);
 
     void pushCurrentRecord(PassRefPtr<InspectorObject>, const String& type, bool captureCallStack);
     void setHeapSizeStatistic(InspectorObject* record);
@@ -172,6 +173,7 @@ class InspectorTimelineAgent : public InspectorBaseAgent<InspectorTimelineAgent>
     typedef Vector<GCEvent> GCEvents;
     GCEvents m_gcEvents;
     int m_maxCallStackDepth;
+    InspectorType m_inspectorType;
 };
 
 } // namespace WebCore

diff --git a/Source/WebCore/inspector/WorkerInspectorController.cpp b/Source/WebCore/inspector/WorkerInspectorController.cpp
@@ -102,7 +102,7 @@ WorkerInspectorController::WorkerInspectorController(WorkerContext* workerContex
     m_debuggerAgent = WorkerDebuggerAgent::create(m_instrumentingAgents.get(), m_state.get(), workerContext, m_injectedScriptManager.get());
     m_profilerAgent = InspectorProfilerAgent::create(m_instrumentingAgents.get(), m_consoleAgent.get(), workerContext, m_state.get(), m_injectedScriptManager.get());
 #endif
-    m_timelineAgent = InspectorTimelineAgent::create(m_instrumentingAgents.get(), m_state.get());
+    m_timelineAgent = InspectorTimelineAgent::create(m_instrumentingAgents.get(), m_state.get(), InspectorTimelineAgent::WorkerInspector);
 
     m_injectedScriptManager->injectedScriptHost()->init(0
         , 0

diff --git a/Source/WebCore/platform/ThreadGlobalData.cpp b/Source/WebCore/platform/ThreadGlobalData.cpp
@@ -29,6 +29,7 @@
 
 #include "DOMImplementation.h"
 #include "EventNames.h"
+#include "InspectorCounters.h"
 #include "ThreadTimers.h"
 #include <wtf/MainThread.h>
 #include <wtf/UnusedParam.h>
@@ -70,6 +71,9 @@ ThreadGlobalData::ThreadGlobalData()
 #if PLATFORM(MAC)
     , m_cachedConverterTEC(adoptPtr(new TECConverterWrapper))
 #endif
+#if ENABLE(INSPECTOR)
+    , m_inspectorCounters(adoptPtr(new ThreadLocalInspectorCounters()))
+#endif
 {
     // This constructor will have been called on the main thread before being called on
     // any other thread, and is only called once per thread - this makes this a convenient
@@ -93,6 +97,10 @@ void ThreadGlobalData::destroy()
     m_cachedConverterICU.clear();
 #endif
 
+#if ENABLE(INSPECTOR)
+    m_inspectorCounters.clear();
+#endif
+
     m_eventNames.clear();
     m_threadTimers.clear();
     m_xmlTypeRegExp.clear();

diff --git a/Source/WebCore/platform/ThreadGlobalData.h b/Source/WebCore/platform/ThreadGlobalData.h
@@ -42,6 +42,7 @@ using WTF::ThreadSpecific;
 namespace WebCore {
 
     class EventNames;
+    class ThreadLocalInspectorCounters;
     class ThreadTimers;
     class XMLMIMETypeRegExp;
 
@@ -67,6 +68,10 @@ namespace WebCore {
         TECConverterWrapper& cachedConverterTEC() { return *m_cachedConverterTEC; }
 #endif
 
+#if ENABLE(INSPECTOR)
+        ThreadLocalInspectorCounters& inspectorCounters() { return *m_inspectorCounters; }
+#endif
+
     private:
         OwnPtr<EventNames> m_eventNames;
         OwnPtr<ThreadTimers> m_threadTimers;
@@ -84,6 +89,10 @@ namespace WebCore {
         OwnPtr<TECConverterWrapper> m_cachedConverterTEC;
 #endif
 
+#if ENABLE(INSPECTOR)
+        OwnPtr<ThreadLocalInspectorCounters> m_inspectorCounters;
+#endif
+
 #if ENABLE(WORKERS)
         static ThreadSpecific<ThreadGlobalData>* staticData;
 #else