Web Inspector: InspectorCounters mechanism should be thread-safe
authoryurys@chromium.org <yurys@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 16:20:30 +0000 (16:20 +0000)
committeryurys@chromium.org <yurys@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 16:20:30 +0000 (16:20 +0000)
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):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@109922 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSEventListener.cpp
Source/WebCore/bindings/v8/V8AbstractEventListener.cpp
Source/WebCore/inspector/InspectorController.cpp
Source/WebCore/inspector/InspectorCounters.cpp
Source/WebCore/inspector/InspectorCounters.h
Source/WebCore/inspector/InspectorTimelineAgent.cpp
Source/WebCore/inspector/InspectorTimelineAgent.h
Source/WebCore/inspector/WorkerInspectorController.cpp
Source/WebCore/platform/ThreadGlobalData.cpp
Source/WebCore/platform/ThreadGlobalData.h

index 55e8942..537541a 100644 (file)
@@ -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
index d6fdf77..5836900 100644 (file)
@@ -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
index 9c56440..86f81d0 100644 (file)
@@ -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)
index c08721d..89c1c09 100644 (file)
@@ -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()));
index 7069707..d17e289 100644 (file)
@@ -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)
index 03dc110..3becf76 100644 (file)
 #ifndef InspectorCounters_h
 #define InspectorCounters_h
 
+#if !ASSERT_DISABLED
+#include <wtf/MainThread.h>
+#endif
+
 namespace WebCore {
 
 class InspectorCounters {
@@ -45,6 +49,7 @@ public:
     static inline void incrementCounter(CounterType type)
     {
 #if ENABLE(INSPECTOR)
+        ASSERT(isMainThread());
         ++s_counters[type];
 #endif
     }
@@ -52,6 +57,7 @@ public:
     static inline void decrementCounter(CounterType type)
     {
 #if ENABLE(INSPECTOR)
+        ASSERT(isMainThread());
         --s_counters[type];
 #endif
     }
@@ -68,6 +74,35 @@ private:
 #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)
index fd0f78c..8089728 100644 (file)
@@ -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)
 {
 }
 
index 42c5811..10464b0 100644 (file)
@@ -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 @@ private:
         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 @@ private:
     typedef Vector<GCEvent> GCEvents;
     GCEvents m_gcEvents;
     int m_maxCallStackDepth;
+    InspectorType m_inspectorType;
 };
 
 } // namespace WebCore
index f214699..8f0ed0b 100644 (file)
@@ -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
index 166eb47..da846f8 100644 (file)
@@ -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();
index bb9f1a8..42f05af 100644 (file)
@@ -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