Skip to content

Commit

Permalink
deps: cherry-pick f19b889 from V8 upstream
Browse files Browse the repository at this point in the history
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.

	R=dgozman@chromium.org,eostroukhov@chromium.com

	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 <dgozman@chromium.org>
	Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
	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 <jan.krems@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
  • Loading branch information
alexkozy authored and Eugene Ostroukhov committed Aug 11, 2017
1 parent 7307839 commit 73c59bb
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 0 deletions.
41 changes: 41 additions & 0 deletions deps/v8/src/inspector/inspected-context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,39 @@

namespace v8_inspector {

class InspectedContext::WeakCallbackData {
public:
WeakCallbackData(InspectedContext* context, V8InspectorImpl* inspector,
int groupId, int contextId)
: m_context(context),
m_inspector(inspector),
m_groupId(groupId),
m_contextId(contextId) {}

static void resetContext(const v8::WeakCallbackInfo<WeakCallbackData>& data) {
// InspectedContext is alive here because weak handler is still alive.
data.GetParameter()->m_context->m_weakCallbackData = nullptr;
data.GetParameter()->m_context->m_context.Reset();
data.SetSecondPassCallback(&callContextCollected);
}

static void callContextCollected(
const v8::WeakCallbackInfo<WeakCallbackData>& data) {
// InspectedContext can be dead here since anything can happen between first
// and second pass callback.
WeakCallbackData* callbackData = data.GetParameter();
callbackData->m_inspector->contextCollected(callbackData->m_groupId,
callbackData->m_contextId);
delete callbackData;
}

private:
InspectedContext* m_context;
V8InspectorImpl* m_inspector;
int m_groupId;
int m_contextId;
};

InspectedContext::InspectedContext(V8InspectorImpl* inspector,
const V8ContextInfo& info, int contextId)
: m_inspector(inspector),
Expand All @@ -26,6 +59,11 @@ InspectedContext::InspectedContext(V8InspectorImpl* inspector,
m_auxData(toString16(info.auxData)),
m_reported(false) {
v8::debug::SetContextId(info.context, contextId);
m_weakCallbackData =
new WeakCallbackData(this, m_inspector, m_contextGroupId, m_contextId);
m_context.SetWeak(m_weakCallbackData,
&InspectedContext::WeakCallbackData::resetContext,
v8::WeakCallbackType::kParameter);
if (!info.hasMemoryOnConsole) return;
v8::Context::Scope contextScope(info.context);
v8::Local<v8::Object> global = info.context->Global();
Expand All @@ -39,6 +77,9 @@ InspectedContext::InspectedContext(V8InspectorImpl* inspector,
}

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

// static
Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/inspector/inspected-context.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class InspectedContext {
friend class V8InspectorImpl;
InspectedContext(V8InspectorImpl*, const V8ContextInfo&, int contextId);

class WeakCallbackData;

V8InspectorImpl* m_inspector;
v8::Global<v8::Context> m_context;
int m_contextId;
Expand All @@ -53,6 +55,7 @@ class InspectedContext {
const String16 m_auxData;
bool m_reported;
std::unique_ptr<InjectedScript> m_injectedScript;
WeakCallbackData* m_weakCallbackData;

DISALLOW_COPY_AND_ASSIGN(InspectedContext);
};
Expand Down
4 changes: 4 additions & 0 deletions deps/v8/src/inspector/v8-inspector-impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ void V8InspectorImpl::contextCreated(const V8ContextInfo& info) {
void V8InspectorImpl::contextDestroyed(v8::Local<v8::Context> context) {
int contextId = InspectedContext::contextId(context);
int groupId = contextGroupId(context);
contextCollected(groupId, contextId);
}

void V8InspectorImpl::contextCollected(int groupId, int contextId) {
m_contextIdToGroupIdMap.erase(contextId);

ConsoleStorageMap::iterator storageIt = m_consoleStorageMap.find(groupId);
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/inspector/v8-inspector-impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class V8InspectorImpl : public V8Inspector {
const StringView& state) override;
void contextCreated(const V8ContextInfo&) override;
void contextDestroyed(v8::Local<v8::Context>) override;
void contextCollected(int contextGroupId, int contextId);
void resetContextGroup(int contextGroupId) override;
void idleStarted() override;
void idleFinished() override;
Expand Down
9 changes: 9 additions & 0 deletions deps/v8/test/inspector/inspector-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,9 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
inspector->Set(ToV8String(isolate, "fireContextDestroyed"),
v8::FunctionTemplate::New(
isolate, &InspectorExtension::FireContextDestroyed));
inspector->Set(
ToV8String(isolate, "freeContext"),
v8::FunctionTemplate::New(isolate, &InspectorExtension::FreeContext));
inspector->Set(ToV8String(isolate, "setMaxAsyncTaskStacks"),
v8::FunctionTemplate::New(
isolate, &InspectorExtension::SetMaxAsyncTaskStacks));
Expand Down Expand Up @@ -658,6 +661,12 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
data->inspector()->ContextDestroyed(context);
}

static void FreeContext(const v8::FunctionCallbackInfo<v8::Value>& args) {
v8::Local<v8::Context> context = args.GetIsolate()->GetCurrentContext();
IsolateData* data = IsolateData::FromContext(context);
data->FreeContext(context);
}

static void SetMaxAsyncTaskStacks(
const v8::FunctionCallbackInfo<v8::Value>& args) {
if (args.Length() != 1 || !args[0]->IsInt32()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Tests that contextDesrtoyed nofitication is fired when context is collected.
{
method : Runtime.executionContextDestroyed
params : {
executionContextId : <executionContextId>
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

let {session, contextGroup, Protocol} =
InspectorTest.start('Tests that contextDesrtoyed nofitication is fired when context is collected.');

(async function test() {
await Protocol.Runtime.enable();
Protocol.Runtime.onExecutionContextDestroyed(InspectorTest.logMessage);
contextGroup.addScript('inspector.freeContext()');
await Protocol.HeapProfiler.collectGarbage();
InspectorTest.completeTest();
})();

0 comments on commit 73c59bb

Please sign in to comment.