Skip to content

Commit ede9d2e

Browse files
targosMylesBorins
authored andcommitted
deps: cherry-pick f19b889 from upstream V8
Original commit message: [inspector] support for cases when embedder doesn't call contextDestroyed Node.js doesn't have good place to call contextDestroyed. We need to cleanup everything on our side to allow clients to not call contextDestroyed method. [email protected],[email protected] Bug: none Change-Id: Ibe3f01fd18afbfa579e5db66ab6f174d5fad7c82 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng Reviewed-on: https://chromium-review.googlesource.com/575519 Reviewed-by: Dmitry Gozman <[email protected]> Commit-Queue: Aleksey Kozyatinskiy <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#46849} Reviewed-on: https://chromium-review.googlesource.com/596549 Cr-Commit-Position: refs/heads/master@{#47060} Backport-PR-URL: #15393 PR-URL: #14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 63ebad5 commit ede9d2e

9 files changed

+87
-0
lines changed

deps/v8/src/inspector/inspected-context.cc

+41
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,39 @@
1515

1616
namespace v8_inspector {
1717

18+
class InspectedContext::WeakCallbackData {
19+
public:
20+
WeakCallbackData(InspectedContext* context, V8InspectorImpl* inspector,
21+
int groupId, int contextId)
22+
: m_context(context),
23+
m_inspector(inspector),
24+
m_groupId(groupId),
25+
m_contextId(contextId) {}
26+
27+
static void resetContext(const v8::WeakCallbackInfo<WeakCallbackData>& data) {
28+
// InspectedContext is alive here because weak handler is still alive.
29+
data.GetParameter()->m_context->m_weakCallbackData = nullptr;
30+
data.GetParameter()->m_context->m_context.Reset();
31+
data.SetSecondPassCallback(&callContextCollected);
32+
}
33+
34+
static void callContextCollected(
35+
const v8::WeakCallbackInfo<WeakCallbackData>& data) {
36+
// InspectedContext can be dead here since anything can happen between first
37+
// and second pass callback.
38+
WeakCallbackData* callbackData = data.GetParameter();
39+
callbackData->m_inspector->contextCollected(callbackData->m_groupId,
40+
callbackData->m_contextId);
41+
delete callbackData;
42+
}
43+
44+
private:
45+
InspectedContext* m_context;
46+
V8InspectorImpl* m_inspector;
47+
int m_groupId;
48+
int m_contextId;
49+
};
50+
1851
InspectedContext::InspectedContext(V8InspectorImpl* inspector,
1952
const V8ContextInfo& info, int contextId)
2053
: m_inspector(inspector),
@@ -25,6 +58,11 @@ InspectedContext::InspectedContext(V8InspectorImpl* inspector,
2558
m_humanReadableName(toString16(info.humanReadableName)),
2659
m_auxData(toString16(info.auxData)) {
2760
v8::debug::SetContextId(info.context, contextId);
61+
m_weakCallbackData =
62+
new WeakCallbackData(this, m_inspector, m_contextGroupId, m_contextId);
63+
m_context.SetWeak(m_weakCallbackData,
64+
&InspectedContext::WeakCallbackData::resetContext,
65+
v8::WeakCallbackType::kParameter);
2866
if (!info.hasMemoryOnConsole) return;
2967
v8::Context::Scope contextScope(info.context);
3068
v8::Local<v8::Object> global = info.context->Global();
@@ -38,6 +76,9 @@ InspectedContext::InspectedContext(V8InspectorImpl* inspector,
3876
}
3977

4078
InspectedContext::~InspectedContext() {
79+
// If we destory InspectedContext before weak callback is invoked then we need
80+
// to delete data here.
81+
if (!m_context.IsEmpty()) delete m_weakCallbackData;
4182
}
4283

4384
// static

deps/v8/src/inspector/inspected-context.h

+3
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ class InspectedContext {
4747
friend class V8InspectorImpl;
4848
InspectedContext(V8InspectorImpl*, const V8ContextInfo&, int contextId);
4949

50+
class WeakCallbackData;
51+
5052
V8InspectorImpl* m_inspector;
5153
v8::Global<v8::Context> m_context;
5254
int m_contextId;
@@ -56,6 +58,7 @@ class InspectedContext {
5658
const String16 m_auxData;
5759
std::unordered_set<int> m_reportedSessionIds;
5860
std::unordered_map<int, std::unique_ptr<InjectedScript>> m_injectedScripts;
61+
WeakCallbackData* m_weakCallbackData;
5962

6063
DISALLOW_COPY_AND_ASSIGN(InspectedContext);
6164
};

deps/v8/src/inspector/v8-inspector-impl.cc

+4
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,10 @@ void V8InspectorImpl::contextCreated(const V8ContextInfo& info) {
203203
void V8InspectorImpl::contextDestroyed(v8::Local<v8::Context> context) {
204204
int contextId = InspectedContext::contextId(context);
205205
int groupId = contextGroupId(context);
206+
contextCollected(groupId, contextId);
207+
}
208+
209+
void V8InspectorImpl::contextCollected(int groupId, int contextId) {
206210
m_contextIdToGroupIdMap.erase(contextId);
207211

208212
ConsoleStorageMap::iterator storageIt = m_consoleStorageMap.find(groupId);

deps/v8/src/inspector/v8-inspector-impl.h

+1
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ class V8InspectorImpl : public V8Inspector {
7474
const StringView& state) override;
7575
void contextCreated(const V8ContextInfo&) override;
7676
void contextDestroyed(v8::Local<v8::Context>) override;
77+
void contextCollected(int contextGroupId, int contextId);
7778
void resetContextGroup(int contextGroupId) override;
7879
void idleStarted() override;
7980
void idleFinished() override;

deps/v8/test/inspector/inspector-test.cc

+9
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,9 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
642642
inspector->Set(ToV8String(isolate, "fireContextDestroyed"),
643643
v8::FunctionTemplate::New(
644644
isolate, &InspectorExtension::FireContextDestroyed));
645+
inspector->Set(
646+
ToV8String(isolate, "freeContext"),
647+
v8::FunctionTemplate::New(isolate, &InspectorExtension::FreeContext));
645648
inspector->Set(ToV8String(isolate, "addInspectedObject"),
646649
v8::FunctionTemplate::New(
647650
isolate, &InspectorExtension::AddInspectedObject));
@@ -683,6 +686,12 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
683686
data->FireContextDestroyed(context);
684687
}
685688

689+
static void FreeContext(const v8::FunctionCallbackInfo<v8::Value>& args) {
690+
v8::Local<v8::Context> context = args.GetIsolate()->GetCurrentContext();
691+
IsolateData* data = IsolateData::FromContext(context);
692+
data->FreeContext(context);
693+
}
694+
686695
static void AddInspectedObject(
687696
const v8::FunctionCallbackInfo<v8::Value>& args) {
688697
if (args.Length() != 2 || !args[0]->IsInt32()) {

deps/v8/test/inspector/isolate-data.cc

+7
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,13 @@ void IsolateData::FireContextDestroyed(v8::Local<v8::Context> context) {
303303
inspector_->contextDestroyed(context);
304304
}
305305

306+
void IsolateData::FreeContext(v8::Local<v8::Context> context) {
307+
int context_group_id = GetContextGroupId(context);
308+
auto it = contexts_.find(context_group_id);
309+
if (it == contexts_.end()) return;
310+
contexts_.erase(it);
311+
}
312+
306313
std::vector<int> IsolateData::GetSessionIds(int context_group_id) {
307314
std::vector<int> result;
308315
for (auto& it : sessions_) {

deps/v8/test/inspector/isolate-data.h

+1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ class IsolateData : public v8_inspector::V8InspectorClient {
6868
void DumpAsyncTaskStacksStateForTest();
6969
void FireContextCreated(v8::Local<v8::Context> context, int context_group_id);
7070
void FireContextDestroyed(v8::Local<v8::Context> context);
71+
void FreeContext(v8::Local<v8::Context> context);
7172

7273
private:
7374
struct VectorCompare {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Tests that contextDesrtoyed nofitication is fired when context is collected.
2+
{
3+
method : Runtime.executionContextDestroyed
4+
params : {
5+
executionContextId : <executionContextId>
6+
}
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2017 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
let {session, contextGroup, Protocol} =
6+
InspectorTest.start('Tests that contextDesrtoyed nofitication is fired when context is collected.');
7+
8+
(async function test() {
9+
await Protocol.Runtime.enable();
10+
Protocol.Runtime.onExecutionContextDestroyed(InspectorTest.logMessage);
11+
contextGroup.addScript('inspector.freeContext()');
12+
await Protocol.HeapProfiler.collectGarbage();
13+
InspectorTest.completeTest();
14+
})();

0 commit comments

Comments
 (0)