fix: [extensions] load extensions on the IO thread (#21811)

electron · Jan 21, 2020 · 100a85f · 100a85f
1 parent acb5b75
commit 100a85f
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 35 deletions.
diff --git a/shell/browser/api/atom_api_session.cc b/shell/browser/api/atom_api_session.cc
@@ -615,15 +615,20 @@ v8::Local<v8::Promise> Session::LoadExtension(
 
   auto* extension_system = static_cast<extensions::AtomExtensionSystem*>(
       extensions::ExtensionSystem::Get(browser_context()));
-  // TODO(nornagon): make LoadExtension() asynchronous.
-  auto* extension = extension_system->LoadExtension(extension_path);
-
-  if (extension) {
-    promise.Resolve(extension);
-  } else {
-    // TODO(nornagon): plumb through error message from extension loader.
-    promise.RejectWithErrorMessage("Failed to load extension");
-  }
+  extension_system->LoadExtension(
+      extension_path,
+      base::BindOnce(
+          [](gin_helper::Promise<const extensions::Extension*> promise,
+             const extensions::Extension* extension) {
+            if (extension) {
+              promise.Resolve(extension);
+            } else {
+              // TODO(nornagon): plumb through error message from extension
+              // loader.
+              promise.RejectWithErrorMessage("Failed to load extension");
+            }
+          },
+          std::move(promise)));
 
   return handle;
 }

diff --git a/shell/browser/extensions/atom_extension_loader.cc b/shell/browser/extensions/atom_extension_loader.cc
@@ -4,6 +4,8 @@
 
 #include "shell/browser/extensions/atom_extension_loader.h"
 
+#include <utility>
+
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ -66,16 +68,14 @@ AtomExtensionLoader::AtomExtensionLoader(
 
 AtomExtensionLoader::~AtomExtensionLoader() = default;
 
-const Extension* AtomExtensionLoader::LoadExtension(
-    const base::FilePath& extension_dir) {
-  // TODO(nornagon): load extensions asynchronously on
-  // GetExtensionFileTaskRunner()
-  base::ScopedAllowBlockingForTesting allow_blocking;
-  scoped_refptr<const Extension> extension = LoadUnpacked(extension_dir);
-  if (extension)
-    extension_registrar_.AddExtension(extension);
-
-  return extension.get();
+void AtomExtensionLoader::LoadExtension(
+    const base::FilePath& extension_dir,
+    base::OnceCallback<void(const Extension*)> loaded) {
+  base::PostTaskAndReplyWithResult(
+      GetExtensionFileTaskRunner().get(), FROM_HERE,
+      base::BindOnce(&LoadUnpacked, extension_dir),
+      base::BindOnce(&AtomExtensionLoader::FinishExtensionLoad,
+                     weak_factory_.GetWeakPtr(), std::move(loaded)));
 }
 
 void AtomExtensionLoader::ReloadExtension(const ExtensionId& extension_id) {
@@ -100,6 +100,15 @@ void AtomExtensionLoader::UnloadExtension(
   extension_registrar_.RemoveExtension(extension_id, reason);
 }
 
+void AtomExtensionLoader::FinishExtensionLoad(
+    base::OnceCallback<void(const Extension*)> done,
+    scoped_refptr<const Extension> extension) {
+  if (extension) {
+    extension_registrar_.AddExtension(extension);
+  }
+  std::move(done).Run(extension.get());
+}
+
 void AtomExtensionLoader::FinishExtensionReload(
     const ExtensionId& old_extension_id,
     scoped_refptr<const Extension> extension) {

diff --git a/shell/browser/extensions/atom_extension_loader.h b/shell/browser/extensions/atom_extension_loader.h
@@ -34,7 +34,9 @@ class AtomExtensionLoader : public ExtensionRegistrar::Delegate {
 
   // Loads an unpacked extension from a directory synchronously. Returns the
   // extension on success, or nullptr otherwise.
-  const Extension* LoadExtension(const base::FilePath& extension_dir);
+  void LoadExtension(
+      const base::FilePath& extension_dir,
+      base::OnceCallback<void(const Extension* extension)> loaded);
 
   // Starts reloading the extension. A keep-alive is maintained until the
   // reload succeeds/fails. If the extension is an app, it will be launched upon
@@ -52,6 +54,9 @@ class AtomExtensionLoader : public ExtensionRegistrar::Delegate {
   void FinishExtensionReload(const ExtensionId& old_extension_id,
                              scoped_refptr<const Extension> extension);
 
+  void FinishExtensionLoad(base::OnceCallback<void(const Extension*)> loaded,
+                           scoped_refptr<const Extension> extension);
+
   // ExtensionRegistrar::Delegate:
   void PreAddExtension(const Extension* extension,
                        const Extension* old_extension) override;

diff --git a/shell/browser/extensions/atom_extension_system.cc b/shell/browser/extensions/atom_extension_system.cc
@@ -6,6 +6,7 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ -43,14 +44,10 @@ AtomExtensionSystem::AtomExtensionSystem(BrowserContext* browser_context)
 
 AtomExtensionSystem::~AtomExtensionSystem() = default;
 
-const Extension* AtomExtensionSystem::LoadExtension(
-    const base::FilePath& extension_dir) {
-  return extension_loader_->LoadExtension(extension_dir);
-}
-
-const Extension* AtomExtensionSystem::LoadApp(const base::FilePath& app_dir) {
-  NOTIMPLEMENTED() << "Attempted to load platform app in Electron";
-  return nullptr;
+void AtomExtensionSystem::LoadExtension(
+    const base::FilePath& extension_dir,
+    base::OnceCallback<void(const Extension*)> loaded) {
+  extension_loader_->LoadExtension(extension_dir, std::move(loaded));
 }
 
 void AtomExtensionSystem::FinishInitialization() {

diff --git a/shell/browser/extensions/atom_extension_system.h b/shell/browser/extensions/atom_extension_system.h
@@ -39,13 +39,8 @@ class AtomExtensionSystem : public ExtensionSystem {
 
   // Loads an unpacked extension from a directory. Returns the extension on
   // success, or nullptr otherwise.
-  const Extension* LoadExtension(const base::FilePath& extension_dir);
-
-  // Loads an unpacked platform app from a directory. Returns the extension on
-  // success, or nullptr otherwise.
-  // Currently this just calls LoadExtension, as apps are not loaded differently
-  // than other extensions. Use LaunchApp() to actually launch the loaded app.
-  const Extension* LoadApp(const base::FilePath& app_dir);
+  void LoadExtension(const base::FilePath& extension_dir,
+                     base::OnceCallback<void(const Extension*)> loaded);
 
   // Finish initialization for the shell extension system.
   void FinishInitialization();