Oilpan: Prepare moving InspectorController and InspectorAgents to oilpan.

Source/core/inspector/WorkerDebuggerAgent.cpp

Index: Source/core/inspector/WorkerDebuggerAgent.cpp
diff --git a/Source/core/inspector/WorkerDebuggerAgent.cpp b/Source/core/inspector/WorkerDebuggerAgent.cpp
index 52d2b4d4f5c01e4badba956ba1125f152e94843d..2a4da722a09555c8320b55406c0366adb154c510 100644
--- a/Source/core/inspector/WorkerDebuggerAgent.cpp
+++ b/Source/core/inspector/WorkerDebuggerAgent.cpp
@@ -54,6 +54,31 @@ WorkerDebuggerAgents& workerDebuggerAgents()
     return agents;
 }
 
+#if ENABLE(OILPAN)
+class WorkerDebuggerAgentRemover {
+public:
+    WorkerDebuggerAgentRemover(WorkerThread* thread)
+    {
+        m_thread = thread;
+    }
+    ~WorkerDebuggerAgentRemover()
+    {
+        MutexLocker lock(workerDebuggerAgentsMutex());
+        workerDebuggerAgents().remove(m_thread);

[haraken, 2014/05/30 03:24:12]

If you insert printf to ~WorkerDebuggerAgentRemover(), will it be printed? I
want to confirm whether ~WorkerDebuggerAgentRemover() is never called or is
called sometime later.

+    }
+
+private:
+    WorkerThread* m_thread;
+};
+
+typedef WillBePersistentHeapHashMap<WeakMember<WorkerDebuggerAgent>, OwnPtr<WorkerDebuggerAgentRemover> > WorkerDebuggerAgentRemovers;

[haraken, 2014/06/02 04:51:14]

mads@, erik@: I'm guessing that this issue might be related to weak processing
where multiple threads get involved. The main thread creates the
PersistentHeapHashMap, and worker threads register items to the map and request
Oilpan's weak processing to clear the items. Does this work in oilpan?

[Mads Ager (chromium), 2014/06/02 09:00:59]

That does not work, no. If you need to do something like this you need to use
off-heap collections of raw pointers and keep the code that explicitly removes
the elements from the global set. This is also done for the DatabaseManager that
has the same kind of setup.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

One issue with using a persistent heap hash map is that it is not clear which
thread will own the backing store and therefore the backing store can get
deallocated if the owning worker thread goes away. The other threads will then
be left with a stale backing pointer. Therefore, we cannot allow that.

  

+
+WorkerDebuggerAgentRemovers& workerDebuggerAgentRemovers()
+{
+    DEFINE_STATIC_LOCAL(WorkerDebuggerAgentRemovers, removers, ());

[haraken, 2014/05/30 03:24:12]

I don't think it's related, but what happens if you use:

DEFINE_STATIC_LOCAL(OwnPtrWillBePersistent<WillBeHeapHashMap<WeakMember<WorkerDebuggerAgent>,
OwnPtr<WorkerDebuggerAgentRemover>>>)

?

+    return removers;
+}
+#endif
 
 class RunInspectorCommandsTask FINAL : public ScriptDebugServer::Task {
 public:
@@ -73,9 +98,9 @@ private:
 
 } // namespace
 
-PassOwnPtr<WorkerDebuggerAgent> WorkerDebuggerAgent::create(WorkerScriptDebugServer* scriptDebugServer, WorkerGlobalScope* inspectedWorkerGlobalScope, InjectedScriptManager* injectedScriptManager)
+PassOwnPtrWillBeRawPtr<WorkerDebuggerAgent> WorkerDebuggerAgent::create(WorkerScriptDebugServer* scriptDebugServer, WorkerGlobalScope* inspectedWorkerGlobalScope, InjectedScriptManager* injectedScriptManager)
 {
-    return adoptPtr(new WorkerDebuggerAgent(scriptDebugServer, inspectedWorkerGlobalScope, injectedScriptManager));
+    return adoptPtrWillBeNoop(new WorkerDebuggerAgent(scriptDebugServer, inspectedWorkerGlobalScope, injectedScriptManager));
 }
 
 WorkerDebuggerAgent::WorkerDebuggerAgent(WorkerScriptDebugServer* scriptDebugServer, WorkerGlobalScope* inspectedWorkerGlobalScope, InjectedScriptManager* injectedScriptManager)
@@ -84,14 +109,26 @@ WorkerDebuggerAgent::WorkerDebuggerAgent(WorkerScriptDebugServer* scriptDebugSer
     , m_inspectedWorkerGlobalScope(inspectedWorkerGlobalScope)
 {
     MutexLocker lock(workerDebuggerAgentsMutex());
-    workerDebuggerAgents().set(inspectedWorkerGlobalScope->thread(), this);
+    WorkerThread* thread = inspectedWorkerGlobalScope->thread();
+    ASSERT(!workerDebuggerAgents().contains(thread));
+    ASSERT(!workerDebuggerAgentRemovers().contains(this));
+    workerDebuggerAgents().set(thread, this);
+    workerDebuggerAgentRemovers().set(this, adoptPtr(new WorkerDebuggerAgentRemover(thread)));

[haraken, 2014/05/30 01:13:20]

The line 113 and 115 need to be removed in oilpan builds, because you removed
the line 124 in oilpan builds, right? Hope it fixes your issue.

[keishi, 2014/05/30 02:01:07]

Won't line 79 in ~WorkerDebuggerAgentRemover work as a replacement for line 124?

[haraken, 2014/05/30 03:24:12]

oh, line 67 should work.

 }
 
 WorkerDebuggerAgent::~WorkerDebuggerAgent()
 {
+#if !ENABLE(OILPAN)
     MutexLocker lock(workerDebuggerAgentsMutex());
     ASSERT(workerDebuggerAgents().contains(m_inspectedWorkerGlobalScope->thread()));
     workerDebuggerAgents().remove(m_inspectedWorkerGlobalScope->thread());
+#endif
+}
+
+void WorkerDebuggerAgent::trace(Visitor* visitor)
+{
+    visitor->trace(m_inspectedWorkerGlobalScope);
+    InspectorDebuggerAgent::trace(visitor);
 }
 
 void WorkerDebuggerAgent::interruptAndDispatchInspectorCommands(WorkerThread* thread)