Skip to content

Commit 73c59bb

Browse files
alexkozyEugene Ostroukhov
alexkozy
authored and
Eugene Ostroukhov
committed
deps: cherry-pick f19b889 from V8 upstream
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} Ref: https://chromium.googlesource.com/v8/v8.git/+/f19b889be801bdebc04c49090e37c787f7ba8805 PR-URL: #14465 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Aleksey Kozyatinskiy <[email protected]>
1 parent 7307839 commit 73c59bb

7 files changed

Lines changed: 79 additions & 0 deletions

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

Lines changed: 41 additions & 0 deletions
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),
@@ -26,6 +59,11 @@ InspectedContext::InspectedContext(V8InspectorImpl* inspector,
2659
m_auxData(toString16(info.auxData)),
2760
m_reported(false) {
2861
v8::debug::SetContextId(info.context, contextId);
62+
m_weakCallbackData =
63+
new WeakCallbackData(this, m_inspector, m_contextGroupId, m_contextId);
64+
m_context.SetWeak(m_weakCallbackData,
65+
&InspectedContext::WeakCallbackData::resetContext,
66+
v8::WeakCallbackType::kParameter);
2967
if (!info.hasMemoryOnConsole) return;
3068
v8::Context::Scope contextScope(info.context);
3169
v8::Local<v8::Object> global = info.context->Global();
@@ -39,6 +77,9 @@ InspectedContext::InspectedContext(V8InspectorImpl* inspector,
3977
}
4078

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

4485
// static

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ class InspectedContext {
4444
friend class V8InspectorImpl;
4545
InspectedContext(V8InspectorImpl*, const V8ContextInfo&, int contextId);
4646

47+
class WeakCallbackData;
48+
4749
V8InspectorImpl* m_inspector;
4850
v8::Global<v8::Context> m_context;
4951
int m_contextId;
@@ -53,6 +55,7 @@ class InspectedContext {
5355
const String16 m_auxData;
5456
bool m_reported;
5557
std::unique_ptr<InjectedScript> m_injectedScript;
58+
WeakCallbackData* m_weakCallbackData;
5659

5760
DISALLOW_COPY_AND_ASSIGN(InspectedContext);
5861
};

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,10 @@ void V8InspectorImpl::contextCreated(const V8ContextInfo& info) {
219219
void V8InspectorImpl::contextDestroyed(v8::Local<v8::Context> context) {
220220
int contextId = InspectedContext::contextId(context);
221221
int groupId = contextGroupId(context);
222+
contextCollected(groupId, contextId);
223+
}
224+
225+
void V8InspectorImpl::contextCollected(int groupId, int contextId) {
222226
m_contextIdToGroupIdMap.erase(contextId);
223227

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

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

Lines changed: 1 addition & 0 deletions
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‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,9 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
619619
inspector->Set(ToV8String(isolate, "fireContextDestroyed"),
620620
v8::FunctionTemplate::New(
621621
isolate, &InspectorExtension::FireContextDestroyed));
622+
inspector->Set(
623+
ToV8String(isolate, "freeContext"),
624+
v8::FunctionTemplate::New(isolate, &InspectorExtension::FreeContext));
622625
inspector->Set(ToV8String(isolate, "setMaxAsyncTaskStacks"),
623626
v8::FunctionTemplate::New(
624627
isolate, &InspectorExtension::SetMaxAsyncTaskStacks));
@@ -658,6 +661,12 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
658661
data->inspector()->ContextDestroyed(context);
659662
}
660663

664+
static void FreeContext(const v8::FunctionCallbackInfo<v8::Value>& args) {
665+
v8::Local<v8::Context> context = args.GetIsolate()->GetCurrentContext();
666+
IsolateData* data = IsolateData::FromContext(context);
667+
data->FreeContext(context);
668+
}
669+
661670
static void SetMaxAsyncTaskStacks(
662671
const v8::FunctionCallbackInfo<v8::Value>& args) {
663672
if (args.Length() != 1 || !args[0]->IsInt32()) {

‎deps/v8/test/inspector/runtime/context-destroyed-on-context-collected-expected.txt‎

Lines changed: 7 additions & 0 deletions
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+
}

‎deps/v8/test/inspector/runtime/context-destroyed-on-context-collected.js‎

Lines changed: 14 additions & 0 deletions
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)