src: add public APIs to manage v8::TracingController

We added a hack for this a while ago for Electron, so let’s remove that hack and make this an official API. Refs: #28724 Refs: #33800 PR-URL: #33850 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
nodejs · Sep 25, 2020 · 0affe86 · 0affe86
1 parent a19933f
commit 0affe86
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 6 deletions.
diff --git a/src/node.h b/src/node.h
@@ -496,6 +496,15 @@ NODE_EXTERN MultiIsolatePlatform* CreatePlatform(
     v8::TracingController* tracing_controller);
 NODE_EXTERN void FreePlatform(MultiIsolatePlatform* platform);
 
+// Get/set the currently active tracing controller. Using CreatePlatform()
+// will implicitly set this by default. This is global and should be initialized
+// along with the v8::Platform instance that is being used. `controller`
+// is allowed to be `nullptr`.
+// This is used for tracing events from Node.js itself. V8 uses the tracing
+// controller returned from the active `v8::Platform` instance.
+NODE_EXTERN v8::TracingController* GetTracingController();
+NODE_EXTERN void SetTracingController(v8::TracingController* controller);
+
 NODE_EXTERN void EmitBeforeExit(Environment* env);
 NODE_EXTERN int EmitExit(Environment* env);
 NODE_EXTERN void RunAtExit(Environment* env);

diff --git a/src/node_platform.cc b/src/node_platform.cc
@@ -328,7 +328,8 @@ NodePlatform::NodePlatform(int thread_pool_size,
   // TODO(addaleax): It's a bit icky that we use global state here, but we can't
   // really do anything about it unless V8 starts exposing a way to access the
   // current v8::Platform instance.
-  tracing::TraceEventHelper::SetTracingController(tracing_controller_);
+  SetTracingController(tracing_controller_);
+  DCHECK_EQ(GetTracingController(), tracing_controller_);
   worker_thread_task_runner_ =
       std::make_shared<WorkerThreadsTaskRunner>(thread_pool_size);
 }

diff --git a/src/node_platform.h b/src/node_platform.h
@@ -11,7 +11,6 @@
 #include "libplatform/libplatform.h"
 #include "node.h"
 #include "node_mutex.h"
-#include "tracing/agent.h"
 #include "uv.h"
 
 namespace node {

diff --git a/src/node_v8_platform-inl.h b/src/node_v8_platform-inl.h
@@ -8,6 +8,7 @@
 #include "env-inl.h"
 #include "node.h"
 #include "node_metadata.h"
+#include "node_platform.h"
 #include "node_options.h"
 #include "tracing/node_trace_writer.h"
 #include "tracing/trace_event.h"

diff --git a/src/tracing/trace_event.cc b/src/tracing/trace_event.cc
@@ -1,4 +1,5 @@
 #include "tracing/trace_event.h"
+#include "node.h"
 
 namespace node {
 namespace tracing {
@@ -24,4 +25,13 @@ void TraceEventHelper::SetTracingController(v8::TracingController* controller) {
 }
 
 }  // namespace tracing
+
+v8::TracingController* GetTracingController() {
+  return tracing::TraceEventHelper::GetTracingController();
+}
+
+void SetTracingController(v8::TracingController* controller) {
+  tracing::TraceEventHelper::SetTracingController(controller);
+}
+
 }  // namespace node
diff --git a/src/tracing/trace_event.h b/src/tracing/trace_event.h
@@ -5,8 +5,8 @@
 #ifndef SRC_TRACING_TRACE_EVENT_H_
 #define SRC_TRACING_TRACE_EVENT_H_
 
-#include "node_platform.h"
 #include "v8-platform.h"
+#include "tracing/agent.h"
 #include "trace_event_common.h"
 #include <atomic>
 
@@ -310,9 +310,7 @@ const int kZeroNumArgs = 0;
 const decltype(nullptr) kGlobalScope = nullptr;
 const uint64_t kNoId = 0;
 
-// Extern (for now) because embedders need access to TraceEventHelper.
-// Refs: https://github.com/nodejs/node/pull/28724
-class NODE_EXTERN TraceEventHelper {
+class TraceEventHelper {
  public:
   static v8::TracingController* GetTracingController();
   static void SetTracingController(v8::TracingController* controller);

diff --git a/test/cctest/test_platform.cc b/test/cctest/test_platform.cc
@@ -57,3 +57,22 @@ TEST_F(PlatformTest, SkipNewTasksInFlushForegroundTasks) {
   EXPECT_EQ(3, run_count);
   EXPECT_FALSE(platform->FlushForegroundTasks(isolate_));
 }
+
+TEST_F(PlatformTest, TracingControllerNullptr) {
+  v8::TracingController* orig_controller = node::GetTracingController();
+  node::SetTracingController(nullptr);
+  EXPECT_EQ(node::GetTracingController(), nullptr);
+
+  v8::Isolate::Scope isolate_scope(isolate_);
+  const v8::HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env {handle_scope, argv};
+
+  node::LoadEnvironment(*env, [&](const node::StartExecutionCallbackInfo& info)
+                                  -> v8::MaybeLocal<v8::Value> {
+    return v8::Null(isolate_);
+  });
+
+  node::SetTracingController(orig_controller);
+  EXPECT_EQ(node::GetTracingController(), orig_controller);
+}