[blink] Move ModuleRequest creation to ModuleRecord::ModuleRequests

Change ModuleRecord::ModuleRequests to return directly
Vector<ModuleRequest>. This avoids the overhead of two temporary
Vectors for source position and specifiers.

Remove ModuleRecord::ModuleRequestPositions as it is no longer needed.

Bug: 945094
Change-Id: Ie6400e7511f2a5879bbf95223f4eeb021b4ae0c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414288
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Auto-Submit: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809432}

third_party/blink/renderer/bindings/bindings.gni

@@ -53,6 +53,7 @@ bindings_core_v8_files =
                     "core/v8/maplike.h",
                     "core/v8/module_record.cc",
                     "core/v8/module_record.h",
+                    "core/v8/module_request.h",
                     "core/v8/native_value_traits.h",
                     "core/v8/native_value_traits_impl.cc",
                     "core/v8/native_value_traits_impl.h",

third_party/blink/renderer/bindings/core/v8/module_record.cc

@@ -206,38 +206,25 @@ void ModuleRecord::ReportException(ScriptState* script_state,
   V8ScriptRunner::ReportException(script_state->GetIsolate(), exception);
 }
 
-Vector<String> ModuleRecord::ModuleRequests(ScriptState* script_state,
-                                            v8::Local<v8::Module> record) {
-  if (record.IsEmpty())
-    return Vector<String>();
-
-  Vector<String> ret;
-
-  int length = record->GetModuleRequestsLength();
-  ret.ReserveInitialCapacity(length);
-  for (int i = 0; i < length; ++i) {
-    v8::Local<v8::String> v8_name = record->GetModuleRequest(i);
-    ret.push_back(ToCoreString(v8_name));
-  }
-  return ret;
-}
-
-Vector<TextPosition> ModuleRecord::ModuleRequestPositions(
+Vector<ModuleRequest> ModuleRecord::ModuleRequests(
     ScriptState* script_state,
     v8::Local<v8::Module> record) {
   if (record.IsEmpty())
-    return Vector<TextPosition>();
-
-  Vector<TextPosition> ret;
+    return Vector<ModuleRequest>();
 
+  Vector<ModuleRequest> requests;
   int length = record->GetModuleRequestsLength();
-  ret.ReserveInitialCapacity(length);
+  requests.ReserveInitialCapacity(length);
+
   for (int i = 0; i < length; ++i) {
+    v8::Local<v8::String> v8_name = record->GetModuleRequest(i);
     v8::Location v8_loc = record->GetModuleRequestLocation(i);
-    ret.emplace_back(OrdinalNumber::FromZeroBasedInt(v8_loc.GetLineNumber()),
-                     OrdinalNumber::FromZeroBasedInt(v8_loc.GetColumnNumber()));
+    TextPosition position(
+        OrdinalNumber::FromZeroBasedInt(v8_loc.GetLineNumber()),
+        OrdinalNumber::FromZeroBasedInt(v8_loc.GetColumnNumber()));
+    requests.emplace_back(ToCoreString(v8_name), position);
   }
-  return ret;
+  return requests;
 }
 
 v8::Local<v8::Value> ModuleRecord::V8Namespace(v8::Local<v8::Module> record) {

third_party/blink/renderer/bindings/core/v8/module_record.h

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_MODULE_RECORD_H_
 #define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_MODULE_RECORD_H_
 
+#include "third_party/blink/renderer/bindings/core/v8/module_request.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_source_location_type.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_code_cache.h"
 #include "third_party/blink/renderer/core/core_export.h"
@@ -126,11 +127,8 @@ class CORE_EXPORT ModuleRecord final {
 
   static void ReportException(ScriptState*, v8::Local<v8::Value> exception);
 
-  static Vector<String> ModuleRequests(ScriptState*,
-                                       v8::Local<v8::Module> record);
-  static Vector<TextPosition> ModuleRequestPositions(
-      ScriptState*,
-      v8::Local<v8::Module> record);
+  static Vector<ModuleRequest> ModuleRequests(ScriptState*,
+                                              v8::Local<v8::Module> record);
 
   static v8::Local<v8::Value> V8Namespace(v8::Local<v8::Module> record);
 

third_party/blink/renderer/bindings/core/v8/module_record_test.cc

@@ -7,6 +7,7 @@
 #include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/bindings/core/v8/boxed_v8_module.h"
+#include "third_party/blink/renderer/bindings/core/v8/module_request.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_source_code.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_value.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
@@ -122,7 +123,9 @@ TEST(ModuleRecordTest, moduleRequests) {
   ASSERT_FALSE(module.IsEmpty());
 
   auto requests = ModuleRecord::ModuleRequests(scope.GetScriptState(), module);
-  EXPECT_THAT(requests, testing::ContainerEq<Vector<String>>({"a", "b"}));
+  EXPECT_EQ(2u, requests.size());
+  EXPECT_EQ("a", requests[0].specifier);
+  EXPECT_EQ("b", requests[1].specifier);
 }
 
 TEST(ModuleRecordTest, instantiateNoDeps) {

third_party/blink/renderer/bindings/core/v8/module_request.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_MODULE_REQUEST_H_
+#define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_MODULE_REQUEST_H_
+
+#include "third_party/blink/renderer/platform/wtf/text/text_position.h"
+#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
+
+namespace blink {
+
+struct ModuleRequest {
+  String specifier;
+  TextPosition position;
+  ModuleRequest(const String& specifier, const TextPosition& position)
+      : specifier(specifier), position(position) {}
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_MODULE_REQUEST_H_

third_party/blink/renderer/core/loader/modulescript/module_tree_linker.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/loader/modulescript/module_tree_linker.h"
 
 #include "third_party/blink/renderer/bindings/core/v8/module_record.h"
+#include "third_party/blink/renderer/bindings/core/v8/module_request.h"
 #include "third_party/blink/renderer/core/loader/modulescript/module_script_fetch_request.h"
 #include "third_party/blink/renderer/core/loader/modulescript/module_tree_linker_registry.h"
 #include "third_party/blink/renderer/core/script/module_script.h"
@@ -379,7 +380,7 @@ void ModuleTreeLinker::FetchDescendants(const ModuleScript* module_script) {
 
   // <spec step="5">For each string requested of
   // record.[[RequestedModules]],</spec>
-  Vector<Modulator::ModuleRequest> module_requests =
+  Vector<ModuleRequest> module_requests =
       modulator_->ModuleRequestsFromModuleRecord(record);
 
   for (const auto& module_request : module_requests) {
@@ -572,7 +573,7 @@ ScriptValue ModuleTreeLinker::FindFirstParseError(
 
   // <spec step="5.1">Let childSpecifiers be the value of moduleScript's
   // record's [[RequestedModules]] internal slot.</spec>
-  Vector<Modulator::ModuleRequest> child_specifiers =
+  Vector<ModuleRequest> child_specifiers =
       modulator_->ModuleRequestsFromModuleRecord(record);
 
   for (const auto& module_request : child_specifiers) {

third_party/blink/renderer/core/loader/modulescript/module_tree_linker_test.cc

@@ -163,17 +163,7 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator {
   Vector<ModuleRequest> ModuleRequestsFromModuleRecord(
       v8::Local<v8::Module> module_record) override {
     ScriptState::Scope scope(script_state_);
-    Vector<String> specifiers =
-        ModuleRecord::ModuleRequests(script_state_, module_record);
-    Vector<TextPosition> positions =
-        ModuleRecord::ModuleRequestPositions(script_state_, module_record);
-    DCHECK_EQ(specifiers.size(), positions.size());
-    Vector<ModuleRequest> requests;
-    requests.ReserveInitialCapacity(specifiers.size());
-    for (wtf_size_t i = 0; i < specifiers.size(); ++i) {
-      requests.emplace_back(specifiers[i], positions[i]);
-    }
-    return requests;
+    return ModuleRecord::ModuleRequests(script_state_, module_record);
   }
 
   Member<ScriptState> script_state_;

third_party/blink/renderer/core/script/modulator.h

@@ -10,6 +10,7 @@
 #include "services/network/public/mojom/referrer_policy.mojom-blink-forward.h"
 #include "third_party/blink/public/platform/web_url_request.h"
 #include "third_party/blink/renderer/bindings/core/v8/module_record.h"
+#include "third_party/blink/renderer/bindings/core/v8/module_request.h"
 #include "third_party/blink/renderer/bindings/core/v8/sanitize_script_errors.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_code_cache.h"
 #include "third_party/blink/renderer/core/core_export.h"
@@ -190,12 +191,6 @@ class CORE_EXPORT Modulator : public GarbageCollected<Modulator>,
 
   virtual ScriptValue InstantiateModule(v8::Local<v8::Module>, const KURL&) = 0;
 
-  struct ModuleRequest {
-    String specifier;
-    TextPosition position;
-    ModuleRequest(const String& specifier, const TextPosition& position)
-        : specifier(specifier), position(position) {}
-  };
   virtual Vector<ModuleRequest> ModuleRequestsFromModuleRecord(
       v8::Local<v8::Module>) = 0;
 

third_party/blink/renderer/core/script/modulator_impl_base.cc

@@ -282,21 +282,10 @@ ScriptValue ModulatorImplBase::InstantiateModule(
   return ModuleRecord::Instantiate(script_state_, module_record, source_url);
 }
 
-Vector<Modulator::ModuleRequest>
-ModulatorImplBase::ModuleRequestsFromModuleRecord(
+Vector<ModuleRequest> ModulatorImplBase::ModuleRequestsFromModuleRecord(
     v8::Local<v8::Module> module_record) {
   ScriptState::Scope scope(script_state_);
-  Vector<String> specifiers =
-      ModuleRecord::ModuleRequests(script_state_, module_record);
-  Vector<TextPosition> positions =
-      ModuleRecord::ModuleRequestPositions(script_state_, module_record);
-  DCHECK_EQ(specifiers.size(), positions.size());
-  Vector<ModuleRequest> requests;
-  requests.ReserveInitialCapacity(specifiers.size());
-  for (wtf_size_t i = 0; i < specifiers.size(); ++i) {
-    requests.emplace_back(specifiers[i], positions[i]);
-  }
-  return requests;
+  return ModuleRecord::ModuleRequests(script_state_, module_record);
 }
 
 void ModulatorImplBase::ProduceCacheModuleTreeTopLevel(
@@ -325,7 +314,7 @@ void ModulatorImplBase::ProduceCacheModuleTree(
 
   module_script->ProduceCache();
 
-  Vector<Modulator::ModuleRequest> child_specifiers =
+  Vector<ModuleRequest> child_specifiers =
       ModuleRequestsFromModuleRecord(record);
 
   for (const auto& module_request : child_specifiers) {

third_party/blink/renderer/core/testing/dummy_modulator.cc

@@ -163,7 +163,7 @@ ScriptValue DummyModulator::InstantiateModule(v8::Local<v8::Module>,
   return ScriptValue();
 }
 
-Vector<Modulator::ModuleRequest> DummyModulator::ModuleRequestsFromModuleRecord(
+Vector<ModuleRequest> DummyModulator::ModuleRequestsFromModuleRecord(
     v8::Local<v8::Module>) {
   NOTREACHED();
   return Vector<ModuleRequest>();

third_party/blink/renderer/core/testing/dummy_modulator.h

@@ -7,6 +7,7 @@
 
 #include "base/single_thread_task_runner.h"
 #include "third_party/blink/renderer/bindings/core/v8/module_record.h"
+#include "third_party/blink/renderer/bindings/core/v8/module_request.h"
 #include "third_party/blink/renderer/core/script/modulator.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"