chore: update chrome.scripting.execute logic (#45369)

chore: update chrome.scripting.execute logic

Refs:
- 6123601
- 6142377
- 6173554

shell/browser/extensions/api/scripting/scripting_api.cc

@@ -25,7 +25,6 @@
 #include "extensions/browser/load_and_localize_file.h"
 #include "extensions/browser/script_executor.h"
 #include "extensions/browser/scripting_constants.h"
-#include "extensions/browser/scripting_utils.h"
 #include "extensions/browser/user_script_manager.h"
 #include "extensions/common/api/extension_types.h"
 #include "extensions/common/api/scripts_internal.h"
@@ -49,9 +48,6 @@ namespace extensions {
 
 namespace {
 
-constexpr std::string_view kCouldNotLoadFileError = "Could not load file: '*'.";
-constexpr std::string_view kDuplicateFileSpecifiedError =
-    "Duplicate file specified: '*'.";
 constexpr std::string_view kEmptyMatchesError =
     "Script with ID '*' must specify 'matches'.";
 constexpr char kExactlyOneOfCssAndFilesError[] =
@@ -106,25 +102,9 @@ std::string InjectionKeyForFile(const mojom::HostID& host_id,
                                               /*code=*/std::string());
 }
 
-// Constructs an array of file sources from the read file `data`.
-std::vector<InjectedFileSource> ConstructFileSources(
-    std::vector<std::unique_ptr<std::string>> data,
-    std::vector<std::string> file_names) {
-  // Note: CHECK (and not DCHECK) because if it fails, we have an out-of-bounds
-  // access.
-  CHECK_EQ(data.size(), file_names.size());
-  const size_t num_sources = data.size();
-  std::vector<InjectedFileSource> sources;
-  sources.reserve(num_sources);
-  for (size_t i = 0; i < num_sources; ++i)
-    sources.emplace_back(std::move(file_names[i]), std::move(data[i]));
-
-  return sources;
-}
-
 std::vector<mojom::JSSourcePtr> FileSourcesToJSSources(
     const Extension& extension,
-    std::vector<InjectedFileSource> file_sources) {
+    std::vector<scripting::InjectedFileSource> file_sources) {
   std::vector<mojom::JSSourcePtr> js_sources;
   js_sources.reserve(file_sources.size());
   for (auto& file_source : file_sources) {
@@ -138,7 +118,7 @@ std::vector<mojom::JSSourcePtr> FileSourcesToJSSources(
 
 std::vector<mojom::CSSSourcePtr> FileSourcesToCSSSources(
     const Extension& extension,
-    std::vector<InjectedFileSource> file_sources) {
+    std::vector<scripting::InjectedFileSource> file_sources) {
   mojom::HostID host_id(mojom::HostID::HostType::kExtensions, extension.id());
 
   std::vector<mojom::CSSSourcePtr> css_sources;
@@ -153,105 +133,6 @@ std::vector<mojom::CSSSourcePtr> FileSourcesToCSSSources(
   return css_sources;
 }
 
-// Checks `files` and populates `resources_out` with the appropriate extension
-// resource. Returns true on success; on failure, populates `error_out`.
-bool GetFileResources(const std::vector<std::string>& files,
-                      const Extension& extension,
-                      std::vector<ExtensionResource>* resources_out,
-                      std::string* error_out) {
-  if (files.empty()) {
-    static constexpr char kAtLeastOneFileError[] =
-        "At least one file must be specified.";
-    *error_out = kAtLeastOneFileError;
-    return false;
-  }
-
-  std::vector<ExtensionResource> resources;
-  for (const auto& file : files) {
-    ExtensionResource resource = extension.GetResource(file);
-    if (resource.extension_root().empty() || resource.relative_path().empty()) {
-      *error_out = ErrorUtils::FormatErrorMessage(kCouldNotLoadFileError, file);
-      return false;
-    }
-
-    // ExtensionResource doesn't implement an operator==.
-    if (base::Contains(resources, resource.relative_path(),
-                       &ExtensionResource::relative_path)) {
-      // Disallow duplicates. Note that we could allow this, if we wanted (and
-      // there *might* be reason to with JS injection, to perform an operation
-      // twice?). However, this matches content script behavior, and injecting
-      // twice can be done by chaining calls to executeScript() / insertCSS().
-      // This isn't a robust check, and could probably be circumvented by
-      // passing two paths that look different but are the same - but in that
-      // case, we just try to load and inject the script twice, which is
-      // inefficient, but safe.
-      *error_out =
-          ErrorUtils::FormatErrorMessage(kDuplicateFileSpecifiedError, file);
-      return false;
-    }
-
-    resources.push_back(std::move(resource));
-  }
-
-  resources_out->swap(resources);
-  return true;
-}
-
-using ResourcesLoadedCallback =
-    base::OnceCallback<void(std::vector<InjectedFileSource>,
-                            std::optional<std::string>)>;
-
-// Checks the loaded content of extension resources. Invokes `callback` with
-// the constructed file sources on success or with an error on failure.
-void CheckLoadedResources(std::vector<std::string> file_names,
-                          ResourcesLoadedCallback callback,
-                          std::vector<std::unique_ptr<std::string>> file_data,
-                          std::optional<std::string> load_error) {
-  if (load_error) {
-    std::move(callback).Run({}, std::move(load_error));
-    return;
-  }
-
-  std::vector<InjectedFileSource> file_sources =
-      ConstructFileSources(std::move(file_data), std::move(file_names));
-
-  for (const auto& source : file_sources) {
-    DCHECK(source.data);
-    // TODO(devlin): What necessitates this encoding requirement? Is it needed
-    // for blink injection?
-    if (!base::IsStringUTF8(*source.data)) {
-      static constexpr char kBadFileEncodingError[] =
-          "Could not load file '*'. It isn't UTF-8 encoded.";
-      std::string error = ErrorUtils::FormatErrorMessage(kBadFileEncodingError,
-                                                         source.file_name);
-      std::move(callback).Run({}, std::move(error));
-      return;
-    }
-  }
-
-  std::move(callback).Run(std::move(file_sources), std::nullopt);
-}
-
-// Checks the specified `files` for validity, and attempts to load and localize
-// them, invoking `callback` with the result. Returns true on success; on
-// failure, populates `error`.
-bool CheckAndLoadFiles(std::vector<std::string> files,
-                       const Extension& extension,
-                       bool requires_localization,
-                       ResourcesLoadedCallback callback,
-                       std::string* error) {
-  std::vector<ExtensionResource> resources;
-  if (!GetFileResources(files, extension, &resources, error))
-    return false;
-
-  LoadAndLocalizeResources(
-      extension, resources, requires_localization,
-      script_parsing::GetMaxScriptLength(),
-      base::BindOnce(&CheckLoadedResources, std::move(files),
-                     std::move(callback)));
-  return true;
-}
-
 // Returns an error message string for when an extension cannot access a page it
 // is attempting to.
 std::string GetCannotAccessPageErrorMessage(const PermissionsData& permissions,
@@ -556,12 +437,6 @@ api::scripting::RegisteredContentScript CreateRegisteredContentScriptInfo(
 
 }  // namespace
 
-InjectedFileSource::InjectedFileSource(std::string file_name,
-                                       std::unique_ptr<std::string> data)
-    : file_name(std::move(file_name)), data(std::move(data)) {}
-InjectedFileSource::InjectedFileSource(InjectedFileSource&&) = default;
-InjectedFileSource::~InjectedFileSource() = default;
-
 ScriptingExecuteScriptFunction::ScriptingExecuteScriptFunction() = default;
 ScriptingExecuteScriptFunction::~ScriptingExecuteScriptFunction() = default;
 
@@ -637,7 +512,7 @@ ExtensionFunction::ResponseAction ScriptingExecuteScriptFunction::Run() {
 }
 
 void ScriptingExecuteScriptFunction::DidLoadResources(
-    std::vector<InjectedFileSource> file_sources,
+    std::vector<scripting::InjectedFileSource> file_sources,
     std::optional<std::string> load_error) {
   if (load_error) {
     Respond(Error(std::move(*load_error)));
@@ -667,29 +542,14 @@ bool ScriptingExecuteScriptFunction::Execute(
 
   mojom::ExecutionWorld execution_world =
       ConvertExecutionWorld(injection_.world);
+  // scripting.executeScript() doesn't support selecting execution world id.
+  std::optional<std::string> execution_world_id = std::nullopt;
+  bool inject_immediately = injection_.inject_immediately.value_or(false);
 
-  // Extensions can specify that the script should be injected "immediately".
+  scripting::ExecuteScript(
-  // In this case, we specify kDocumentStart as the injection time. Due to
+      extension()->id(), std::move(sources), execution_world,
-  // inherent raciness between tab creation and load and this function
+      execution_world_id, script_executor, frame_scope, frame_ids,
-  // execution, there is no guarantee that it will actually happen at
+      inject_immediately, user_gesture(),
-  // document start, but the renderer will appropriately inject it
-  // immediately if document start has already passed.
-  mojom::RunLocation run_location =
-      injection_.inject_immediately && *injection_.inject_immediately
-          ? mojom::RunLocation::kDocumentStart
-          : mojom::RunLocation::kDocumentIdle;
-  script_executor->ExecuteScript(
-      mojom::HostID(mojom::HostID::HostType::kExtensions, extension()->id()),
-      mojom::CodeInjection::NewJs(mojom::JSInjection::New(
-          std::move(sources), execution_world, /*world_id=*/std::nullopt,
-          blink::mojom::WantResultOption::kWantResult,
-          user_gesture() ? blink::mojom::UserActivationOption::kActivate
-                         : blink::mojom::UserActivationOption::kDoNotActivate,
-          blink::mojom::PromiseResultOption::kAwait)),
-      frame_scope, frame_ids,
-      mojom::MatchOriginAsFallbackBehavior::kMatchForAboutSchemeAndClimbTree,
-      run_location, ScriptExecutor::DEFAULT_PROCESS,
-      /* webview_src */ GURL(),
       base::BindOnce(&ScriptingExecuteScriptFunction::OnScriptExecuted, this));
 
   return true;
@@ -777,7 +637,7 @@ ExtensionFunction::ResponseAction ScriptingInsertCSSFunction::Run() {
 }
 
 void ScriptingInsertCSSFunction::DidLoadResources(
-    std::vector<InjectedFileSource> file_sources,
+    std::vector<scripting::InjectedFileSource> file_sources,
     std::optional<std::string> load_error) {
   if (load_error) {
     Respond(Error(std::move(*load_error)));
@@ -864,8 +724,10 @@ ExtensionFunction::ResponseAction ScriptingRemoveCSSFunction::Run() {
 
   if (injection.files) {
     std::vector<ExtensionResource> resources;
-    if (!GetFileResources(*injection.files, *extension(), &resources, &error))
+    if (!scripting::GetFileResources(*injection.files, *extension(), &resources,
+                                     &error)) {
       return RespondNow(Error(std::move(error)));
+    }
 
     // Note: Since we're just removing the CSS, we don't actually need to load
     // the file here. It's okay for `code` to be empty in this case.

shell/browser/extensions/api/scripting/scripting_api.h

@@ -15,22 +15,11 @@
 #include "extensions/browser/extension_function.h"
 #include "extensions/browser/script_executor.h"
 #include "extensions/browser/scripting_utils.h"
-#include "extensions/common/mojom/code_injection.mojom-forward.h"
+#include "extensions/common/mojom/code_injection.mojom.h"
 #include "extensions/common/user_script.h"
 
 namespace extensions {
 
-// A simple helper struct to represent a read file (either CSS or JS) to be
-// injected.
-struct InjectedFileSource {
-  InjectedFileSource(std::string file_name, std::unique_ptr<std::string> data);
-  InjectedFileSource(InjectedFileSource&&);
-  ~InjectedFileSource();
-
-  std::string file_name;
-  std::unique_ptr<std::string> data;
-};
-
 class ScriptingExecuteScriptFunction : public ExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("scripting.executeScript", SCRIPTING_EXECUTESCRIPT)
@@ -48,7 +37,7 @@ class ScriptingExecuteScriptFunction : public ExtensionFunction {
   ~ScriptingExecuteScriptFunction() override;
 
   // Called when the resource files to be injected has been loaded.
-  void DidLoadResources(std::vector<InjectedFileSource> file_sources,
+  void DidLoadResources(std::vector<scripting::InjectedFileSource> file_sources,
                         std::optional<std::string> load_error);
 
   // Triggers the execution of `sources` in the appropriate context.
@@ -77,7 +66,7 @@ class ScriptingInsertCSSFunction : public ExtensionFunction {
   ~ScriptingInsertCSSFunction() override;
 
   // Called when the resource files to be injected has been loaded.
-  void DidLoadResources(std::vector<InjectedFileSource> file_sources,
+  void DidLoadResources(std::vector<scripting::InjectedFileSource> file_sources,
                         std::optional<std::string> load_error);
 
   // Triggers the execution of `sources` in the appropriate context.