url: reduce revokeObjectURL cpp calls

PR-URL: #47728 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
nodejs · May 3, 2023 · e4b6b79 · e4b6b79
1 parent 8d1588a
commit e4b6b79
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 24 deletions.
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -24,7 +24,6 @@ const {
   StringPrototypeIncludes,
   StringPrototypeIndexOf,
   StringPrototypeSlice,
-  StringPrototypeSplit,
   StringPrototypeStartsWith,
   Symbol,
   SymbolIterator,
@@ -1024,10 +1023,7 @@ ObjectDefineProperties(URL, {
 });
 
 function installObjectURLMethods() {
-  const {
-    storeDataObject,
-    revokeDataObject,
-  } = internalBinding('blob');
+  const bindingBlob = internalBinding('blob');
 
   function createObjectURL(obj) {
     const cryptoRandom = lazyCryptoRandom();
@@ -1040,22 +1036,13 @@ function installObjectURLMethods() {
 
     const id = cryptoRandom.randomUUID();
 
-    storeDataObject(id, obj[blob.kHandle], obj.size, obj.type);
+    bindingBlob.storeDataObject(id, obj[blob.kHandle], obj.size, obj.type);
 
     return `blob:nodedata:${id}`;
   }
 
   function revokeObjectURL(url) {
-    url = `${url}`;
-    try {
-      // TODO(@anonrig): Remove this try/catch by calling `parse` directly.
-      const parsed = new URL(url);
-      const split = StringPrototypeSplit(parsed.pathname, ':');
-      if (split.length === 2)
-        revokeDataObject(split[1]);
-    } catch {
-      // If there's an error, it's ignored.
-    }
+    bindingBlob.revokeObjectURL(`${url}`);
   }
 
   ObjectDefineProperties(URL, {

diff --git a/src/node_blob.cc b/src/node_blob.cc
@@ -1,4 +1,5 @@
 #include "node_blob.h"
+#include "ada.h"
 #include "async_wrap-inl.h"
 #include "base_object-inl.h"
 #include "env-inl.h"
@@ -7,6 +8,7 @@
 #include "node_errors.h"
 #include "node_external_reference.h"
 #include "node_file.h"
+#include "util.h"
 #include "v8.h"
 
 #include <algorithm>
@@ -119,7 +121,7 @@ void Blob::Initialize(
   SetMethod(context, target, "createBlob", New);
   SetMethod(context, target, "storeDataObject", StoreDataObject);
   SetMethod(context, target, "getDataObject", GetDataObject);
-  SetMethod(context, target, "revokeDataObject", RevokeDataObject);
+  SetMethod(context, target, "revokeObjectURL", RevokeObjectURL);
   SetMethod(context, target, "concat", Concat);
   SetMethod(context, target, "createBlobFromFilePath", BlobFromFilePath);
 }
@@ -414,15 +416,29 @@ void Blob::StoreDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
         std::string(*type, type.length())));
 }
 
-void Blob::RevokeDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
+// TODO(@anonrig): Add V8 Fast API to the following function
+void Blob::RevokeObjectURL(const FunctionCallbackInfo<Value>& args) {
+  CHECK_GE(args.Length(), 1);
+  CHECK(args[0]->IsString());
   BlobBindingData* binding_data = Realm::GetBindingData<BlobBindingData>(args);
-
   Environment* env = Environment::GetCurrent(args);
-  CHECK(args[0]->IsString());  // ID key
+  Utf8Value input(env->isolate(), args[0].As<String>());
+  auto out = ada::parse<ada::url_aggregator>(input.ToStringView());
 
-  Utf8Value key(env->isolate(), args[0]);
+  if (!out) {
+    return;
+  }
+
+  auto pathname = out->get_pathname();
+  auto start_index = pathname.find(':');
 
-  binding_data->revoke_data_object(std::string(*key, key.length()));
+  if (start_index != std::string_view::npos && start_index != pathname.size()) {
+    auto end_index = pathname.find(':', start_index + 1);
+    if (end_index == std::string_view::npos) {
+      auto id = std::string(pathname.substr(start_index + 1));
+      binding_data->revoke_data_object(id);
+    }
+  }
 }
 
 void Blob::GetDataObject(const v8::FunctionCallbackInfo<v8::Value>& args) {
@@ -538,7 +554,7 @@ void Blob::RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(Blob::ToSlice);
   registry->Register(Blob::StoreDataObject);
   registry->Register(Blob::GetDataObject);
-  registry->Register(Blob::RevokeDataObject);
+  registry->Register(Blob::RevokeObjectURL);
   registry->Register(Blob::Reader::Pull);
   registry->Register(Concat);
   registry->Register(BlobFromFilePath);

diff --git a/src/node_blob.h b/src/node_blob.h
@@ -37,7 +37,7 @@ class Blob : public BaseObject {
   static void ToSlice(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void StoreDataObject(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetDataObject(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void RevokeDataObject(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void RevokeObjectURL(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
       Environment* env);