Oilpan: move PromiseTracker to the heap (fix build after r181646.)

Source/core/inspector/PromiseTracker.cpp

Index: Source/core/inspector/PromiseTracker.cpp
diff --git a/Source/core/inspector/PromiseTracker.cpp b/Source/core/inspector/PromiseTracker.cpp
index d549b95260e38c2e754f893a5076c1c7729f5646..309710b1ac5ec60bbc48722b41b84361adfb51a8 100644
--- a/Source/core/inspector/PromiseTracker.cpp
+++ b/Source/core/inspector/PromiseTracker.cpp
@@ -17,32 +17,55 @@ using blink::TypeBuilder::Debugger::PromiseDetails;
 
 namespace blink {
 
-class PromiseTracker::PromiseData : public RefCounted<PromiseData> {
+class PromiseTracker::PromiseData FINAL : public RefCountedWillBeGarbageCollectedFinalized<PromiseData> {
 public:
+    static PassRefPtrWillBeRawPtr<PromiseData> create(v8::Isolate* isolate, int promiseHash, int promiseId, v8::Handle<v8::Object> promise)
+    {
+        return adoptRefWillBeNoop(new PromiseData(isolate, promiseHash, promiseId, promise));
+    }
+
+    int promiseHash() const { return m_promiseHash; }
+    ScopedPersistent<v8::Object>& promise() { return m_promise; }
+
+    void trace(Visitor* visitor)
+    {
+        visitor->trace(m_callStack);
+    }
+
+    WeakPtrWillBeRawPtr<PromiseData> createWeakPtr()
+    {
+#if ENABLE(OILPAN)
+        return this;
+#else
+        return m_weakPtrFactory.createWeakPtr();
+#endif
+    }
+
+private:
+    friend class PromiseTracker;
+
     PromiseData(v8::Isolate* isolate, int promiseHash, int promiseId, v8::Handle<v8::Object> promise)
         : m_promiseHash(promiseHash)
         , m_promiseId(promiseId)
         , m_promise(isolate, promise)
         , m_parentPromiseId(0)
         , m_status(0)
+#if !ENABLE(OILPAN)
         , m_weakPtrFactory(this)
+#endif
     {
     }
 
-    int promiseHash() const { return m_promiseHash; }
-    ScopedPersistent<v8::Object>& promise() { return m_promise; }
-
-private:
-    friend class PromiseTracker;
-
     int m_promiseHash;
     int m_promiseId;
     ScopedPersistent<v8::Object> m_promise;
     int m_parentPromiseId;
     int m_status;
-    WeakPtrFactory<PromiseData> m_weakPtrFactory;
-    RefPtr<ScriptCallStack> m_callStack;
+    RefPtrWillBeMember<ScriptCallStack> m_callStack;
     ScopedPersistent<v8::Object> m_parentPromise;
+#if !ENABLE(OILPAN)
+    WeakPtrFactory<PromiseData> m_weakPtrFactory;
+#endif
 };
 
 static int indexOf(PromiseTracker::PromiseDataVector* vector, const ScopedPersistent<v8::Object>& promise)
@@ -56,18 +79,17 @@ static int indexOf(PromiseTracker::PromiseDataVector* vector, const ScopedPersis
 
 namespace {
 
-class PromiseDataWrapper {
+class PromiseDataWrapper FINAL : public NoBaseWillBeGarbageCollected<PromiseDataWrapper> {
 public:
-    PromiseDataWrapper(WeakPtr<PromiseTracker::PromiseData> data, PromiseTracker::PromiseDataMap* map)
-        : m_data(data)
-        , m_promiseDataMap(map)
+    static PassOwnPtrWillBeRawPtr<PromiseDataWrapper> create(PromiseTracker::PromiseData* data, PromiseTracker::PromiseDataMap* map)
     {
+        return adoptPtrWillBeNoop(new PromiseDataWrapper(data->createWeakPtr(), map));

[haraken, 2014/09/10 01:41:23]

I'd write:

#if ENABLE(OILPAN)
return adoptPtrWillBeNoop(new PromiseDataWrapper(data, map));
#else
return adoptPtrWillBeNoop(new PromiseDataWrapper(data->createWeakPtr(), map));
#endif

and completely remove createWeakPtr from oilpan builds.

[sof, 2014/09/10 13:40:34]

Let's do that.

     }
 
     static void didRemovePromise(const v8::WeakCallbackData<v8::Object, PromiseDataWrapper>& data)
     {
-        OwnPtr<PromiseDataWrapper> wrapper = adoptPtr(data.GetParameter());
-        WeakPtr<PromiseTracker::PromiseData> promiseData = wrapper->m_data;
+        OwnPtrWillBeRawPtr<PromiseDataWrapper> wrapper = adoptPtrWillBeNoop(data.GetParameter());
+        WeakPtrWillBeRawPtr<PromiseTracker::PromiseData> promiseData = wrapper->m_data;
         if (!promiseData)
             return;
         PromiseTracker::PromiseDataMap* map = wrapper->m_promiseDataMap;
@@ -80,9 +102,23 @@ public:
             map->remove(promiseHash);
     }
 
+    void trace(Visitor* visitor)
+    {
+#if ENABLE(OILPAN)
+        visitor->trace(m_data);
+        visitor->trace(m_promiseDataMap);
+#endif
+    }
+
 private:
-    WeakPtr<PromiseTracker::PromiseData> m_data;
-    PromiseTracker::PromiseDataMap* m_promiseDataMap;
+    PromiseDataWrapper(WeakPtrWillBeRawPtr<PromiseTracker::PromiseData> data, PromiseTracker::PromiseDataMap* map)
+        : m_data(data)
+        , m_promiseDataMap(map)
+    {
+    }
+
+    WeakPtrWillBeMember<PromiseTracker::PromiseData> m_data;

[haraken, 2014/09/10 01:41:23]

Just to confirm: Is this WeakPtr just used to break a reference cycle and it
doesn't need any weak semantics, right?

[sof, 2014/09/10 13:40:34]

As far as I understand the use, the unfulfilled promise shouldn't keep the
PromiseData alive, but allow it to be pre-emptively cleared.

This isn't provided in an Oilpan setting. Nor is this GCed wrapper object held
onto once created. Addressing both in an incoming follow-up CL.

+    RawPtrWillBeMember<PromiseTracker::PromiseDataMap> m_promiseDataMap;
 };
 
 }
@@ -97,6 +133,13 @@ PromiseTracker::~PromiseTracker()
 {
 }
 
+void PromiseTracker::trace(Visitor* visitor)
+{
+#if ENABLE(OILPAN)
+    visitor->trace(m_promiseDataMap);
+#endif
+}
+
 void PromiseTracker::setEnabled(bool enabled)
 {
     m_isEnabled = enabled;
@@ -117,21 +160,21 @@ int PromiseTracker::circularSequentialId()
     return m_circularSequentialId;
 }
 
-PassRefPtr<PromiseTracker::PromiseData> PromiseTracker::createPromiseDataIfNeeded(v8::Isolate* isolate, v8::Handle<v8::Object> promise)
+PassRefPtrWillBeRawPtr<PromiseTracker::PromiseData> PromiseTracker::createPromiseDataIfNeeded(v8::Isolate* isolate, v8::Handle<v8::Object> promise)
 {
     int promiseHash = promise->GetIdentityHash();
-    PromiseDataVector* vector;
+    RawPtr<PromiseDataVector> vector = nullptr;

[haraken, 2014/09/10 01:41:23]

PromiseDataVector* ?

It looks a bit strange that a "RawPtr<>" appears without any WillBe.

     PromiseDataMap::iterator it = m_promiseDataMap.find(promiseHash);
     if (it != m_promiseDataMap.end())
         vector = &it->value;
     else
         vector = &m_promiseDataMap.add(promiseHash, PromiseDataVector()).storedValue->value;
 
-    RefPtr<PromiseData> data;
+    RefPtrWillBeRawPtr<PromiseData> data = nullptr;
     int index = indexOf(vector, ScopedPersistent<v8::Object>(isolate, promise));
     if (index == -1) {
-        data = adoptRef(new PromiseData(isolate, promiseHash, circularSequentialId(), promise));
-        OwnPtr<PromiseDataWrapper> wrapper = adoptPtr(new PromiseDataWrapper(data->m_weakPtrFactory.createWeakPtr(), &m_promiseDataMap));
+        data = PromiseData::create(isolate, promiseHash, circularSequentialId(), promise);
+        OwnPtrWillBeRawPtr<PromiseDataWrapper> wrapper = PromiseDataWrapper::create(data.get(), &m_promiseDataMap);
         data->m_promise.setWeak(wrapper.leakPtr(), &PromiseDataWrapper::didRemovePromise);
         vector->append(data);
     } else {
@@ -146,10 +189,10 @@ void PromiseTracker::didReceiveV8PromiseEvent(ScriptState* scriptState, v8::Hand
     ASSERT(isEnabled());
 
     v8::Isolate* isolate = scriptState->isolate();
-    RefPtr<PromiseData> data = createPromiseDataIfNeeded(isolate, promise);
+    RefPtrWillBeRawPtr<PromiseData> data = createPromiseDataIfNeeded(isolate, promise);
     if (!parentPromise.IsEmpty() && parentPromise->IsObject()) {
         v8::Handle<v8::Object> handle = parentPromise->ToObject();
-        RefPtr<PromiseData> parentData = createPromiseDataIfNeeded(isolate, handle);
+        RefPtrWillBeRawPtr<PromiseData> parentData = createPromiseDataIfNeeded(isolate, handle);
         data->m_parentPromiseId = parentData->m_promiseId;
         data->m_parentPromise.set(isolate, handle);
     } else {
@@ -171,7 +214,7 @@ PassRefPtr<Array<PromiseDetails> > PromiseTracker::promises()
     for (PromiseDataMap::iterator it = m_promiseDataMap.begin(); it != m_promiseDataMap.end(); ++it) {
         PromiseDataVector* vector = &it->value;
         for (size_t index = 0; index < vector->size(); ++index) {
-            RefPtr<PromiseData> data = vector->at(index);
+            RefPtrWillBeRawPtr<PromiseData> data = vector->at(index);
             PromiseDetails::Status::Enum status;
             if (!data->m_status)
                 status = PromiseDetails::Status::Pending;