[web snapshot] Pass exports to the serializer as a Local<PrimitiveArray>

This CL updates WebSnapshotSerializer::TakeSnapshot() to accept exports
as a Local<PrimitiveArray>.

Bug: v8:11525, v8:11706
Change-Id: Ie3a752ac7dbcc51fc4fb258eb44ce42d0cfc6a0f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2930173Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Vicky Kontoura <vkont@google.com>
Cr-Commit-Position: refs/heads/master@{#74936}

src/d8/d8.cc

@@ -730,8 +730,10 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
     data->realm_current_ = data->realm_switch_;
 
     if (options.web_snapshot_config) {
-      std::vector<std::string> exports;
-      if (!ReadLines(options.web_snapshot_config, exports)) {
+      MaybeLocal<PrimitiveArray> maybe_exports =
+          ReadLines(isolate, options.web_snapshot_config);
+      Local<PrimitiveArray> exports;
+      if (!maybe_exports.ToLocal(&exports)) {
         isolate->ThrowError("Web snapshots: unable to read config");
         CHECK(try_catch.HasCaught());
         ReportException(isolate, &try_catch);
@@ -1824,19 +1826,20 @@ void Shell::RealmTakeWebSnapshot(
   PerIsolateData* data = PerIsolateData::Get(isolate);
   int index = data->RealmIndexOrThrow(args, 0);
   if (index == -1) return;
-  // Create a std::vector<std::string> from the list of exports.
-  // TODO(v8:11525): Add an API in the serializer that accepts a Local<String>
-  // Array.
+  // Create a Local<PrimitiveArray> from the exports array.
   Local<Context> current_context = isolate->GetCurrentContext();
   Local<Array> exports_array = args[1].As<Array>();
-  std::vector<std::string> exports;
-  for (int i = 0, length = exports_array->Length(); i < length; ++i) {
-    Local<String> local_str = exports_array->Get(current_context, i)
-                                  .ToLocalChecked()
-                                  ->ToString(current_context)
-                                  .ToLocalChecked();
-    std::string str = ToSTLString(isolate, local_str);
-    exports.push_back(str);
+  int length = exports_array->Length();
+  Local<PrimitiveArray> exports = PrimitiveArray::New(isolate, length);
+  for (int i = 0; i < length; ++i) {
+    Local<Value> value;
+    Local<String> str;
+    if (!exports_array->Get(current_context, i).ToLocal(&value) ||
+        !value->ToString(current_context).ToLocal(&str) || str.IsEmpty()) {
+      isolate->ThrowError("Invalid argument");
+      return;
+    }
+    exports->Set(isolate, i, str);
   }
   // Enter the realm.
   Local<Context> realm = Local<Context>::New(isolate, data->realms_[index]);
@@ -3416,18 +3419,33 @@ char* Shell::ReadChars(const char* name, int* size_out) {
   return chars;
 }
 
-bool Shell::ReadLines(const char* name, std::vector<std::string>& lines) {
+MaybeLocal<PrimitiveArray> Shell::ReadLines(Isolate* isolate,
+                                            const char* name) {
   int length;
   const char* data = reinterpret_cast<const char*>(ReadChars(name, &length));
   if (data == nullptr) {
-    return false;
+    return MaybeLocal<PrimitiveArray>();
   }
   std::stringstream stream(data);
   std::string line;
+  std::vector<std::string> lines;
   while (std::getline(stream, line, '\n')) {
     lines.emplace_back(line);
   }
-  return true;
+  // Create a Local<PrimitiveArray> off the read lines.
+  int size = static_cast<int>(lines.size());
+  Local<PrimitiveArray> exports = PrimitiveArray::New(isolate, size);
+  for (int i = 0; i < size; ++i) {
+    MaybeLocal<String> maybe_str = v8::String::NewFromUtf8(
+        isolate, lines[i].c_str(), NewStringType::kNormal,
+        static_cast<int>(lines[i].length()));
+    Local<String> str;
+    if (!maybe_str.ToLocal(&str)) {
+      return MaybeLocal<PrimitiveArray>();
+    }
+    exports->Set(isolate, i, str);
+  }
+  return exports;
 }
 
 void Shell::ReadBuffer(const v8::FunctionCallbackInfo<v8::Value>& args) {

src/d8/d8.h

@@ -513,7 +513,8 @@ class Shell : public i::AllStatic {
   static void Version(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void ReadFile(const v8::FunctionCallbackInfo<v8::Value>& args);
   static char* ReadChars(const char* name, int* size_out);
-  static bool ReadLines(const char* name, std::vector<std::string>& lines);
+  static MaybeLocal<PrimitiveArray> ReadLines(Isolate* isolate,
+                                              const char* name);
   static void ReadBuffer(const v8::FunctionCallbackInfo<v8::Value>& args);
   static Local<String> ReadFromStdin(Isolate* isolate);
   static void ReadLine(const v8::FunctionCallbackInfo<v8::Value>& args) {

src/web-snapshot/web-snapshot.cc

@@ -57,23 +57,21 @@ WebSnapshotSerializer::WebSnapshotSerializer(v8::Isolate* isolate)
 
 WebSnapshotSerializer::~WebSnapshotSerializer() {}
 
-bool WebSnapshotSerializer::TakeSnapshot(
-    v8::Local<v8::Context> context, const std::vector<std::string>& exports,
-    WebSnapshotData& data_out) {
+bool WebSnapshotSerializer::TakeSnapshot(v8::Local<v8::Context> context,
+                                         v8::Local<v8::PrimitiveArray> exports,
+                                         WebSnapshotData& data_out) {
   if (string_ids_.size() > 0) {
     Throw("Web snapshot: Can't reuse WebSnapshotSerializer");
     return false;
   }
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
-  for (const std::string& export_name : exports) {
-    if (export_name.length() == 0) {
+  for (int i = 0, length = exports->Length(); i < length; ++i) {
+    v8::Local<v8::String> str =
+        exports->Get(v8_isolate, i)->ToString(context).ToLocalChecked();
+    if (str.IsEmpty()) {
       continue;
     }
-    v8::ScriptCompiler::Source source(
-        v8::String::NewFromUtf8(v8_isolate, export_name.c_str(),
-                                NewStringType::kNormal,
-                                static_cast<int>(export_name.length()))
-            .ToLocalChecked());
+    v8::ScriptCompiler::Source source(str);
     auto script = ScriptCompiler::Compile(context, &source).ToLocalChecked();
     v8::MaybeLocal<v8::Value> script_result = script->Run(context);
     v8::Local<v8::Object> v8_object;
@@ -85,7 +83,7 @@ bool WebSnapshotSerializer::TakeSnapshot(
     }
 
     auto object = Handle<JSObject>::cast(Utils::OpenHandle(*v8_object));
-    SerializeExport(object, export_name);
+    SerializeExport(object, Handle<String>::cast(Utils::OpenHandle(*str)));
   }
   WriteSnapshot(data_out.buffer, data_out.buffer_size);
   return !has_error();
@@ -369,18 +367,11 @@ void WebSnapshotSerializer::SerializePendingObject(Handle<JSObject> object) {
 // - String id (export name)
 // - Object id (exported object)
 void WebSnapshotSerializer::SerializeExport(Handle<JSObject> object,
-                                            const std::string& export_name) {
+                                            Handle<String> export_name) {
   // TODO(v8:11525): Support exporting functions.
   ++export_count_;
-  // TODO(v8:11525): How to avoid creating the String but still de-dupe?
-  Handle<String> export_name_string =
-      isolate_->factory()
-          ->NewStringFromOneByte(Vector<const uint8_t>(
-              reinterpret_cast<const uint8_t*>(export_name.c_str()),
-              static_cast<int>(export_name.length())))
-          .ToHandleChecked();
   uint32_t string_id = 0;
-  SerializeString(export_name_string, string_id);
+  SerializeString(export_name, string_id);
   uint32_t object_id = 0;
   SerializeObject(object, object_id);
   export_serializer_.WriteUint32(string_id);

src/web-snapshot/web-snapshot.h

@@ -83,7 +83,7 @@ class V8_EXPORT WebSnapshotSerializer
   ~WebSnapshotSerializer();
 
   bool TakeSnapshot(v8::Local<v8::Context> context,
-                    const std::vector<std::string>& exports,
+                    v8::Local<v8::PrimitiveArray> exports,
                     WebSnapshotData& data_out);
 
   // For inspecting the state after taking a snapshot.
@@ -121,7 +121,7 @@ class V8_EXPORT WebSnapshotSerializer
   void SerializeContext(Handle<Context> context, uint32_t& id);
   void SerializeObject(Handle<JSObject> object, uint32_t& id);
   void SerializePendingObject(Handle<JSObject> object);
-  void SerializeExport(Handle<JSObject> object, const std::string& export_name);
+  void SerializeExport(Handle<JSObject> object, Handle<String> export_name);
   void WriteValue(Handle<Object> object, ValueSerializer& serializer);
 
   ValueSerializer string_serializer_;

test/cctest/test-web-snapshots.cc

@@ -27,8 +27,10 @@ void TestWebSnapshotExtensive(
     v8::Context::Scope context_scope(new_context);
 
     CompileRun(snapshot_source);
-    std::vector<std::string> exports;
-    exports.push_back("foo");
+    v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
+    v8::Local<v8::String> str =
+        v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
+    exports->Set(isolate, 0, str);
     WebSnapshotSerializer serializer(isolate);
     CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
     CHECK(!serializer.has_error());
@@ -327,8 +329,10 @@ TEST(SFIDeduplication) {
         "foo.inner = foo.outer('hi');";
 
     CompileRun(snapshot_source);
-    std::vector<std::string> exports;
-    exports.push_back("foo");
+    v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
+    v8::Local<v8::String> str =
+        v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
+    exports->Set(isolate, 0, str);
     WebSnapshotSerializer serializer(isolate);
     CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
     CHECK(!serializer.has_error());
@@ -385,8 +389,10 @@ TEST(SFIDeduplicationAfterBytecodeFlushing) {
 
     CompileRun(snapshot_source);
 
-    std::vector<std::string> exports;
-    exports.push_back("foo");
+    v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
+    v8::Local<v8::String> str =
+        v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
+    exports->Set(isolate, 0, str);
     WebSnapshotSerializer serializer(isolate);
     CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
     CHECK(!serializer.has_error());
@@ -463,8 +469,10 @@ TEST(SFIDeduplicationOfFunctionsNotInSnapshot) {
         "}\n";
 
     CompileRun(snapshot_source);
-    std::vector<std::string> exports;
-    exports.push_back("foo");
+    v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
+    v8::Local<v8::String> str =
+        v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
+    exports->Set(isolate, 0, str);
     WebSnapshotSerializer serializer(isolate);
     CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
     CHECK(!serializer.has_error());