Oilpan: fix PromiseTracker's handling of its persistent promise wrappers.
The handling of the PromiseDataWrapper in r181662 wasn't complete, as it
failed to keep an Oilpan-reachable persistent to the GCed wrapper object,
risking premature finalization. For Oilpan also, this wrapper object should
only keep its references weakly alive.
To make the tracing of these relationships work out, PromiseTracker is
no longer a part object, but heap allocated.
R=haraken
BUG=340522
Review URL: https://codereview.chromium.org/561753002
git-svn-id: svn://svn.chromium.org/blink/trunk@181845 bbb929c8-8fbe-4397-9dbb-9b2b20218538
diff --git a/Source/core/inspector/InspectorDebuggerAgent.cpp b/Source/core/inspector/InspectorDebuggerAgent.cpp
index f13d0f5..0f1a9c2 100644
--- a/Source/core/inspector/InspectorDebuggerAgent.cpp
+++ b/Source/core/inspector/InspectorDebuggerAgent.cpp
@@ -130,6 +130,7 @@
, m_skipAllPauses(false)
, m_skipContentScripts(false)
, m_asyncCallStackTracker(adoptPtrWillBeNoop(new AsyncCallStackTracker()))
+ , m_promiseTracker(PromiseTracker::create())
{
}
@@ -232,7 +233,7 @@
m_state->setBoolean(DebuggerAgentState::skipAllPauses, false);
}
asyncCallStackTracker().setAsyncCallStackDepth(m_state->getLong(DebuggerAgentState::asyncCallStackDepth));
- m_promiseTracker.setEnabled(m_state->getBoolean(DebuggerAgentState::promiseTrackerEnabled));
+ promiseTracker().setEnabled(m_state->getBoolean(DebuggerAgentState::promiseTrackerEnabled));
}
}
@@ -922,9 +923,9 @@
void InspectorDebuggerAgent::didReceiveV8PromiseEvent(ScriptState* scriptState, v8::Handle<v8::Object> promise, v8::Handle<v8::Value> parentPromise, int status)
{
- if (!m_promiseTracker.isEnabled())
+ if (!promiseTracker().isEnabled())
return;
- m_promiseTracker.didReceiveV8PromiseEvent(scriptState, promise, parentPromise, status);
+ promiseTracker().didReceiveV8PromiseEvent(scriptState, promise, parentPromise, status);
}
void InspectorDebuggerAgent::pause(ErrorString*)
@@ -1173,20 +1174,20 @@
void InspectorDebuggerAgent::enablePromiseTracker(ErrorString*)
{
m_state->setBoolean(DebuggerAgentState::promiseTrackerEnabled, true);
- m_promiseTracker.setEnabled(true);
+ promiseTracker().setEnabled(true);
}
void InspectorDebuggerAgent::disablePromiseTracker(ErrorString*)
{
m_state->setBoolean(DebuggerAgentState::promiseTrackerEnabled, false);
- m_promiseTracker.setEnabled(false);
+ promiseTracker().setEnabled(false);
}
void InspectorDebuggerAgent::getPromises(ErrorString*, RefPtr<Array<PromiseDetails> >& promises)
{
- if (!m_promiseTracker.isEnabled())
+ if (!promiseTracker().isEnabled())
return;
- promises = m_promiseTracker.promises();
+ promises = promiseTracker().promises();
}
void InspectorDebuggerAgent::scriptExecutionBlockedByCSP(const String& directiveText)
@@ -1451,7 +1452,7 @@
m_scripts.clear();
m_breakpointIdToDebugServerBreakpointIds.clear();
asyncCallStackTracker().clear();
- m_promiseTracker.clear();
+ promiseTracker().clear();
m_continueToLocationBreakpointId = String();
clearBreakDetails();
m_javaScriptPauseScheduled = false;
@@ -1494,7 +1495,7 @@
m_scripts.clear();
m_breakpointIdToDebugServerBreakpointIds.clear();
asyncCallStackTracker().clear();
- m_promiseTracker.clear();
+ promiseTracker().clear();
if (m_frontend)
m_frontend->globalObjectCleared();
}
diff --git a/Source/core/inspector/InspectorDebuggerAgent.h b/Source/core/inspector/InspectorDebuggerAgent.h
index 1358a84..5a7bc71 100644
--- a/Source/core/inspector/InspectorDebuggerAgent.h
+++ b/Source/core/inspector/InspectorDebuggerAgent.h
@@ -244,7 +244,8 @@
String sourceMapURLForScript(const Script&, CompileResult);
PassRefPtrWillBeRawPtr<JavaScriptCallFrame> topCallFrameSkipUnknownSources(String* scriptURL, bool* isBlackboxed);
- AsyncCallStackTracker& asyncCallStackTracker() { return *m_asyncCallStackTracker; };
+ AsyncCallStackTracker& asyncCallStackTracker() const { return *m_asyncCallStackTracker; };
+ PromiseTracker& promiseTracker() const { return *m_promiseTracker; }
typedef HashMap<String, Script> ScriptsMap;
typedef HashMap<String, Vector<String> > BreakpointIdToDebugServerBreakpointIdsMap;
@@ -272,7 +273,7 @@
bool m_skipContentScripts;
OwnPtr<ScriptRegexp> m_cachedSkipStackRegExp;
OwnPtrWillBeMember<AsyncCallStackTracker> m_asyncCallStackTracker;
- PromiseTracker m_promiseTracker;
+ OwnPtrWillBeMember<PromiseTracker> m_promiseTracker;
};
} // namespace blink
diff --git a/Source/core/inspector/PromiseTracker.cpp b/Source/core/inspector/PromiseTracker.cpp
index 309710b..b7d9a1f 100644
--- a/Source/core/inspector/PromiseTracker.cpp
+++ b/Source/core/inspector/PromiseTracker.cpp
@@ -27,20 +27,18 @@
int promiseHash() const { return m_promiseHash; }
ScopedPersistent<v8::Object>& promise() { return m_promise; }
+#if !ENABLE(OILPAN)
+ WeakPtr<PromiseData> createWeakPtr()
+ {
+ return m_weakPtrFactory.createWeakPtr();
+ }
+#endif
+
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;
@@ -81,57 +79,68 @@
class PromiseDataWrapper FINAL : public NoBaseWillBeGarbageCollected<PromiseDataWrapper> {
public:
- static PassOwnPtrWillBeRawPtr<PromiseDataWrapper> create(PromiseTracker::PromiseData* data, PromiseTracker::PromiseDataMap* map)
+ static PassOwnPtrWillBeRawPtr<PromiseDataWrapper> create(PromiseTracker::PromiseData* data, PromiseTracker* tracker)
{
- return adoptPtrWillBeNoop(new PromiseDataWrapper(data->createWeakPtr(), map));
+#if ENABLE(OILPAN)
+ return new PromiseDataWrapper(data, tracker);
+#else
+ return adoptPtr(new PromiseDataWrapper(data->createWeakPtr(), tracker));
+#endif
}
+#if ENABLE(OILPAN)
+ static void didRemovePromise(const v8::WeakCallbackData<v8::Object, Persistent<PromiseDataWrapper> >& data)
+#else
static void didRemovePromise(const v8::WeakCallbackData<v8::Object, PromiseDataWrapper>& data)
+#endif
{
- OwnPtrWillBeRawPtr<PromiseDataWrapper> wrapper = adoptPtrWillBeNoop(data.GetParameter());
+#if ENABLE(OILPAN)
+ OwnPtr<Persistent<PromiseDataWrapper> > persistentWrapper = adoptPtr(data.GetParameter());
+ RawPtr<PromiseDataWrapper> wrapper = *persistentWrapper;
+#else
+ OwnPtr<PromiseDataWrapper> wrapper = adoptPtr(data.GetParameter());
+#endif
WeakPtrWillBeRawPtr<PromiseTracker::PromiseData> promiseData = wrapper->m_data;
- if (!promiseData)
+ if (!promiseData || !wrapper->m_tracker)
return;
- PromiseTracker::PromiseDataMap* map = wrapper->m_promiseDataMap;
+ PromiseTracker::PromiseDataMap& map = wrapper->m_tracker->promiseDataMap();
int promiseHash = promiseData->promiseHash();
- PromiseTracker::PromiseDataVector* vector = &map->find(promiseHash)->value;
+ PromiseTracker::PromiseDataVector* vector = &map.find(promiseHash)->value;
int index = indexOf(vector, promiseData->promise());
ASSERT(index >= 0);
vector->remove(index);
- if (vector->size() == 0)
- map->remove(promiseHash);
+ if (vector->isEmpty())
+ map.remove(promiseHash);
}
void trace(Visitor* visitor)
{
#if ENABLE(OILPAN)
visitor->trace(m_data);
- visitor->trace(m_promiseDataMap);
+ visitor->trace(m_tracker);
#endif
}
private:
- PromiseDataWrapper(WeakPtrWillBeRawPtr<PromiseTracker::PromiseData> data, PromiseTracker::PromiseDataMap* map)
+ PromiseDataWrapper(WeakPtrWillBeRawPtr<PromiseTracker::PromiseData> data, PromiseTracker* tracker)
: m_data(data)
- , m_promiseDataMap(map)
+ , m_tracker(tracker)
{
}
- WeakPtrWillBeMember<PromiseTracker::PromiseData> m_data;
- RawPtrWillBeMember<PromiseTracker::PromiseDataMap> m_promiseDataMap;
+ WeakPtrWillBeWeakMember<PromiseTracker::PromiseData> m_data;
+ RawPtrWillBeWeakMember<PromiseTracker> m_tracker;
};
}
PromiseTracker::PromiseTracker()
- : m_isEnabled(false)
- , m_circularSequentialId(0)
+ : m_circularSequentialId(0)
+ , m_isEnabled(false)
{
}
-PromiseTracker::~PromiseTracker()
-{
-}
+DEFINE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(PromiseTracker);
void PromiseTracker::trace(Visitor* visitor)
{
@@ -170,16 +179,22 @@
else
vector = &m_promiseDataMap.add(promiseHash, PromiseDataVector()).storedValue->value;
- RefPtrWillBeRawPtr<PromiseData> data = nullptr;
int index = indexOf(vector, ScopedPersistent<v8::Object>(isolate, promise));
- if (index == -1) {
- 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 {
- data = vector->at(index);
- }
+ if (index != -1)
+ return vector->at(index);
+
+ // FIXME: Consider using the ScriptState's DOMWrapperWorld instead
+ // to handle the lifetime of PromiseDataWrapper, avoiding all this
+ // manual labor to achieve the same, with and without Oilpan.
+ RefPtrWillBeRawPtr<PromiseData> data = PromiseData::create(isolate, promiseHash, circularSequentialId(), promise);
+ OwnPtrWillBeRawPtr<PromiseDataWrapper> dataWrapper = PromiseDataWrapper::create(data.get(), this);
+#if ENABLE(OILPAN)
+ OwnPtr<Persistent<PromiseDataWrapper> > wrapper = adoptPtr(new Persistent<PromiseDataWrapper>(dataWrapper));
+#else
+ OwnPtr<PromiseDataWrapper> wrapper = dataWrapper.release();
+#endif
+ data->m_promise.setWeak(wrapper.leakPtr(), &PromiseDataWrapper::didRemovePromise);
+ vector->append(data);
return data.release();
}
diff --git a/Source/core/inspector/PromiseTracker.h b/Source/core/inspector/PromiseTracker.h
index 97bc2d4..9bbdce9 100644
--- a/Source/core/inspector/PromiseTracker.h
+++ b/Source/core/inspector/PromiseTracker.h
@@ -17,12 +17,14 @@
class ScriptState;
-class PromiseTracker FINAL {
+class PromiseTracker FINAL : public NoBaseWillBeGarbageCollected<PromiseTracker> {
WTF_MAKE_NONCOPYABLE(PromiseTracker);
- DISALLOW_ALLOCATION();
+ DECLARE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(PromiseTracker);
public:
- PromiseTracker();
- ~PromiseTracker();
+ static PassOwnPtrWillBeRawPtr<PromiseTracker> create()
+ {
+ return adoptPtrWillBeNoop(new PromiseTracker());
+ }
bool isEnabled() const { return m_isEnabled; }
void setEnabled(bool);
@@ -40,13 +42,17 @@
void trace(Visitor*);
+ PromiseDataMap& promiseDataMap() { return m_promiseDataMap; }
+
private:
+ PromiseTracker();
+
int circularSequentialId();
PassRefPtrWillBeRawPtr<PromiseData> createPromiseDataIfNeeded(v8::Isolate*, v8::Handle<v8::Object> promise);
- bool m_isEnabled;
int m_circularSequentialId;
PromiseDataMap m_promiseDataMap;
+ bool m_isEnabled;
};
} // namespace blink
