src: simplify c++ diagnostics channel API

Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #65158
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

Author: jasnell

src/node_diagnostics_channel.cc

@@ -178,11 +178,11 @@ void Channel::Unlink() {
   publish_fn_.Reset();
 }
 
-Channel* Channel::Get(Environment* env, const char* name) {
+BaseObjectPtr<Channel> Channel::Get(Environment* env, std::string_view name) {
   Realm* realm = env->principal_realm();
   BindingData* binding = realm->GetBindingData<BindingData>();
   if (binding == nullptr) {
-    return nullptr;
+    return {};
   }
   uint32_t index = binding->GetOrCreateChannelIndex(std::string(name));
 
@@ -208,22 +208,24 @@ Channel* Channel::Get(Environment* env, const char* name) {
              .ToLocalChecked()
              ->NewInstance(context)
              .ToLocal(&wrap)) {
-      return nullptr;
+      return {};
     }
 
     binding->channels_[index] = MakeDetachedBaseObject<Channel>(
         env, wrap, binding, index, std::string(name));
   }
 
-  Channel* channel = binding->channels_[index].get();
+  auto& channel = binding->channels_[index];
 
   // Late-bind: link to the JS channel when the callback is available.
   if (!binding->link_callback_.IsEmpty() && !channel->IsLinked()) {
     Isolate* isolate = env->isolate();
     HandleScope handle_scope(isolate);
     Local<Context> context = env->context();
-    Local<String> js_name = String::NewFromUtf8(isolate, name).ToLocalChecked();
-    Local<Value> argv[] = {js_name, Integer::NewFromUnsigned(isolate, index)};
+    Local<Value> argv[] = {
+      ToV8Value(context, name).ToLocalChecked(),
+      Integer::NewFromUnsigned(isolate, index),
+    };
     Local<Value> result;
     if (binding->link_callback_.Get(isolate)
             ->Call(context, v8::Undefined(isolate), arraysize(argv), argv)

src/node_diagnostics_channel.h

@@ -73,8 +73,7 @@ class Channel : public BaseObject {
           uint32_t index,
           std::string name);
 
-  // Returns a non-owning pointer. Lifetime is managed by BindingData.
-  static Channel* Get(Environment* env, const char* name);
+  static BaseObjectPtr<Channel> Get(Environment* env, std::string_view name);
 
   inline bool HasSubscribers() const {
     return binding_data_ != nullptr && binding_data_->subscribers_[index_] > 0;

src/permission/permission.cc

@@ -304,12 +304,10 @@ BaseObjectPtr<diagnostics_channel::Channel> Permission::GetOrCreateChannel(
     channels_.erase(it);
   }
   auto channel_name = GetDiagnosticsChannelName(scope);
-  diagnostics_channel::Channel* ch =
-      diagnostics_channel::Channel::Get(env, channel_name.data());
-  if (ch != nullptr) {
+  if (auto ch = diagnostics_channel::Channel::Get(env, channel_name)) {
     channels_.emplace(scope,
                       BaseObjectWeakPtr<diagnostics_channel::Channel>(ch));
-    return BaseObjectPtr<diagnostics_channel::Channel>(ch);
+    return ch;
   }
   return {};
 }

test/cctest/test_diagnostics_channel.cc

@@ -3,6 +3,7 @@
 #include "gtest/gtest.h"
 #include "node_test_fixture.h"
 
+using node::BaseObjectPtr;
 using node::diagnostics_channel::Channel;
 
 class DiagnosticsChannelTest : public EnvironmentTestFixture {};
@@ -279,15 +280,15 @@ TEST_F(DiagnosticsChannelTest, NativeChannelsGrowSubscriberStorage) {
       "globalThis.__dc.subscribe('test:cctest:grow:0', "
       "                          globalThis.__firstSubscriber);");
 
-  Channel* first = Channel::Get(*env, "test:cctest:grow:0");
-  ASSERT_NE(first, nullptr);
+  auto first = Channel::Get(*env, "test:cctest:grow:0");
+  ASSERT_TRUE(first);
   ASSERT_TRUE(first->HasSubscribers());
 
-  Channel* last = nullptr;
+  BaseObjectPtr<Channel> last;
   for (size_t i = 1; i <= 1024; i++) {
     std::string name = "test:cctest:grow:" + std::to_string(i);
     last = Channel::Get(*env, name.c_str());
-    ASSERT_NE(last, nullptr);
+    ASSERT_TRUE(last);
   }
 
   RunJS(isolate_,