feat: flesh out the api for //extensions (electron#21587)

nornagon · web-flow · commit fa42b5980e73 · 2020-01-13T14:56:28.000-08:00
diff --git a/filenames.gni b/filenames.gni
@@ -474,6 +474,8 @@ filenames = {
     "shell/common/gin_converters/callback_converter.h",
     "shell/common/gin_converters/content_converter.cc",
     "shell/common/gin_converters/content_converter.h",
+    "shell/common/gin_converters/extension_converter.cc",
+    "shell/common/gin_converters/extension_converter.h",
     "shell/common/gin_converters/file_dialog_converter.cc",
     "shell/common/gin_converters/file_dialog_converter.h",
     "shell/common/gin_converters/file_path_converter.h",
diff --git a/shell/browser/api/atom_api_session.cc b/shell/browser/api/atom_api_session.cc
@@ -66,7 +66,9 @@
 #include "ui/base/l10n/l10n_util.h"
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+#include "extensions/browser/extension_registry.h"
 #include "shell/browser/extensions/atom_extension_system.h"
+#include "shell/common/gin_converters/extension_converter.h"
 #endif
 
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
@@ -606,10 +608,51 @@ std::vector<base::FilePath::StringType> Session::GetPreloads() const {
 }
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
-void Session::LoadChromeExtension(const base::FilePath extension_path) {
+v8::Local<v8::Promise> Session::LoadExtension(
+    const base::FilePath& extension_path) {
+  gin_helper::Promise<const extensions::Extension*> promise(isolate());
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
+  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");
+  }
+
+  return handle;
+}
+
+void Session::RemoveExtension(const std::string& extension_id) {
   auto* extension_system = static_cast<extensions::AtomExtensionSystem*>(
       extensions::ExtensionSystem::Get(browser_context()));
-  extension_system->LoadExtension(extension_path);
+  extension_system->RemoveExtension(extension_id);
+}
+
+v8::Local<v8::Value> Session::GetExtension(const std::string& extension_id) {
+  auto* registry = extensions::ExtensionRegistry::Get(browser_context());
+  const extensions::Extension* extension =
+      registry->GetInstalledExtension(extension_id);
+  if (extension) {
+    return gin::ConvertToV8(isolate(), extension);
+  } else {
+    return v8::Null(isolate());
+  }
+}
+
+v8::Local<v8::Value> Session::GetAllExtensions() {
+  auto* registry = extensions::ExtensionRegistry::Get(browser_context());
+  auto installed_extensions = registry->GenerateInstalledExtensionsSet();
+  std::vector<const extensions::Extension*> extensions_vector;
+  for (const auto& extension : *installed_extensions) {
+    extensions_vector.emplace_back(extension.get());
+  }
+  return gin::ConvertToV8(isolate(), extensions_vector);
 }
 #endif
 
@@ -797,7 +840,10 @@ void Session::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setPreloads", &Session::SetPreloads)
       .SetMethod("getPreloads", &Session::GetPreloads)
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
-      .SetMethod("loadChromeExtension", &Session::LoadChromeExtension)
+      .SetMethod("loadExtension", &Session::LoadExtension)
+      .SetMethod("removeExtension", &Session::RemoveExtension)
+      .SetMethod("getExtension", &Session::GetExtension)
+      .SetMethod("getAllExtensions", &Session::GetAllExtensions)
 #endif
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
       .SetMethod("getSpellCheckerLanguages", &Session::GetSpellCheckerLanguages)
diff --git a/shell/browser/api/atom_api_session.h b/shell/browser/api/atom_api_session.h
@@ -96,7 +96,10 @@ class Session : public gin_helper::TrackableObject<Session>,
 #endif
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
-  void LoadChromeExtension(const base::FilePath extension_path);
+  v8::Local<v8::Promise> LoadExtension(const base::FilePath& extension_path);
+  void RemoveExtension(const std::string& extension_id);
+  v8::Local<v8::Value> GetExtension(const std::string& extension_id);
+  v8::Local<v8::Value> GetAllExtensions();
 #endif
 
  protected:
diff --git a/shell/browser/extensions/atom_extension_loader.cc b/shell/browser/extensions/atom_extension_loader.cc
@@ -78,7 +78,7 @@ const Extension* AtomExtensionLoader::LoadExtension(
   return extension.get();
 }
 
-void AtomExtensionLoader::ReloadExtension(ExtensionId extension_id) {
+void AtomExtensionLoader::ReloadExtension(const ExtensionId& extension_id) {
   const Extension* extension = ExtensionRegistry::Get(browser_context_)
                                    ->GetInstalledExtension(extension_id);
   // We shouldn't be trying to reload extensions that haven't been added.
@@ -94,8 +94,14 @@ void AtomExtensionLoader::ReloadExtension(ExtensionId extension_id) {
     return;
 }
 
+void AtomExtensionLoader::UnloadExtension(
+    const ExtensionId& extension_id,
+    extensions::UnloadedExtensionReason reason) {
+  extension_registrar_.RemoveExtension(extension_id, reason);
+}
+
 void AtomExtensionLoader::FinishExtensionReload(
-    const ExtensionId old_extension_id,
+    const ExtensionId& old_extension_id,
     scoped_refptr<const Extension> extension) {
   if (extension) {
     extension_registrar_.AddExtension(extension);
diff --git a/shell/browser/extensions/atom_extension_loader.h b/shell/browser/extensions/atom_extension_loader.h
@@ -41,12 +41,15 @@ class AtomExtensionLoader : public ExtensionRegistrar::Delegate {
   // reloading.
   // This may invalidate references to the old Extension object, so it takes the
   // ID by value.
-  void ReloadExtension(ExtensionId extension_id);
+  void ReloadExtension(const ExtensionId& extension_id);
+
+  void UnloadExtension(const ExtensionId& extension_id,
+                       extensions::UnloadedExtensionReason reason);
 
  private:
   // If the extension loaded successfully, enables it. If it's an app, launches
   // it. If the load failed, updates ShellKeepAliveRequester.
-  void FinishExtensionReload(const ExtensionId old_extension_id,
+  void FinishExtensionReload(const ExtensionId& old_extension_id,
                              scoped_refptr<const Extension> extension);
 
   // ExtensionRegistrar::Delegate:
diff --git a/shell/browser/extensions/atom_extension_system.cc b/shell/browser/extensions/atom_extension_system.cc
@@ -49,7 +49,7 @@ const Extension* AtomExtensionSystem::LoadExtension(
 }
 
 const Extension* AtomExtensionSystem::LoadApp(const base::FilePath& app_dir) {
-  CHECK(false);  // Should never call LoadApp
+  NOTIMPLEMENTED() << "Attempted to load platform app in Electron";
   return nullptr;
 }
 
@@ -66,6 +66,11 @@ void AtomExtensionSystem::ReloadExtension(const ExtensionId& extension_id) {
   extension_loader_->ReloadExtension(extension_id);
 }
 
+void AtomExtensionSystem::RemoveExtension(const ExtensionId& extension_id) {
+  extension_loader_->UnloadExtension(
+      extension_id, extensions::UnloadedExtensionReason::UNINSTALL);
+}
+
 void AtomExtensionSystem::Shutdown() {
   extension_loader_.reset();
 }
diff --git a/shell/browser/extensions/atom_extension_system.h b/shell/browser/extensions/atom_extension_system.h
@@ -53,6 +53,8 @@ class AtomExtensionSystem : public ExtensionSystem {
   // Reloads the extension with id |extension_id|.
   void ReloadExtension(const ExtensionId& extension_id);
 
+  void RemoveExtension(const ExtensionId& extension_id);
+
   // KeyedService implementation:
   void Shutdown() override;
 
diff --git a/shell/common/gin_converters/extension_converter.cc b/shell/common/gin_converters/extension_converter.cc
@@ -0,0 +1,21 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/common/gin_converters/extension_converter.h"
+
+#include "extensions/common/extension.h"
+#include "gin/dictionary.h"
+
+namespace gin {
+
+// static
+v8::Local<v8::Value> Converter<const extensions::Extension*>::ToV8(
+    v8::Isolate* isolate,
+    const extensions::Extension* extension) {
+  auto dict = gin::Dictionary::CreateEmpty(isolate);
+  dict.Set("id", extension->id());
+  return gin::ConvertToV8(isolate, dict);
+}
+
+}  // namespace gin
diff --git a/shell/common/gin_converters/extension_converter.h b/shell/common/gin_converters/extension_converter.h
@@ -0,0 +1,26 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_GIN_CONVERTERS_EXTENSION_CONVERTER_H_
+#define SHELL_COMMON_GIN_CONVERTERS_EXTENSION_CONVERTER_H_
+
+#include <string>
+
+#include "gin/converter.h"
+
+namespace extensions {
+class Extension;
+}  // namespace extensions
+
+namespace gin {
+
+template <>
+struct Converter<const extensions::Extension*> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const extensions::Extension* val);
+};
+
+}  // namespace gin
+
+#endif  // SHELL_COMMON_GIN_CONVERTERS_EXTENSION_CONVERTER_H_
diff --git a/spec-main/extensions-spec.ts b/spec-main/extensions-spec.ts
@@ -32,16 +32,48 @@ ifdescribe(process.electronBinding('features').isExtensionsEnabled())('chrome ex
     // extension in an in-memory session results in it being installed in the
     // default session.
     const customSession = session.fromPartition(`persist:${require('uuid').v4()}`);
-    (customSession as any).loadChromeExtension(path.join(fixtures, 'extensions', 'red-bg'))
+    (customSession as any).loadExtension(path.join(fixtures, 'extensions', 'red-bg'))
     const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } })
     await w.loadURL(url)
     const bg = await w.webContents.executeJavaScript('document.documentElement.style.backgroundColor')
     expect(bg).to.equal('red')
   })
 
+  it('removes an extension', async () => {
+    const customSession = session.fromPartition(`persist:${require('uuid').v4()}`)
+    const { id } = await (customSession as any).loadExtension(path.join(fixtures, 'extensions', 'red-bg'))
+    {
+      const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } })
+      await w.loadURL(url)
+      const bg = await w.webContents.executeJavaScript('document.documentElement.style.backgroundColor')
+      expect(bg).to.equal('red')
+    }
+    (customSession as any).removeExtension(id)
+    {
+      const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } })
+      await w.loadURL(url)
+      const bg = await w.webContents.executeJavaScript('document.documentElement.style.backgroundColor')
+      expect(bg).to.equal('')
+    }
+  })
+
+  it('lists loaded extensions in getAllExtensions', async () => {
+    const customSession = session.fromPartition(`persist:${require('uuid').v4()}`)
+    const e = await (customSession as any).loadExtension(path.join(fixtures, 'extensions', 'red-bg'))
+    expect((customSession as any).getAllExtensions()).to.deep.equal([e]);
+    (customSession as any).removeExtension(e.id)
+    expect((customSession as any).getAllExtensions()).to.deep.equal([])
+  })
+
+  it('gets an extension by id', async () => {
+    const customSession = session.fromPartition(`persist:${require('uuid').v4()}`)
+    const e = await (customSession as any).loadExtension(path.join(fixtures, 'extensions', 'red-bg'))
+    expect((customSession as any).getExtension(e.id)).to.deep.equal(e)
+  })
+
   it('confines an extension to the session it was loaded in', async () => {
     const customSession = session.fromPartition(`persist:${require('uuid').v4()}`);
-    (customSession as any).loadChromeExtension(path.join(fixtures, 'extensions', 'red-bg'))
+    (customSession as any).loadExtension(path.join(fixtures, 'extensions', 'red-bg'))
     const w = new BrowserWindow({ show: false }) // not in the session
     await w.loadURL(url)
     const bg = await w.webContents.executeJavaScript('document.documentElement.style.backgroundColor')
@@ -52,7 +84,7 @@ ifdescribe(process.electronBinding('features').isExtensionsEnabled())('chrome ex
     let content: any
     before(async () => {
       const customSession = session.fromPartition(`persist:${require('uuid').v4()}`);
-      (customSession as any).loadChromeExtension(path.join(fixtures, 'extensions', 'chrome-runtime'))
+      (customSession as any).loadExtension(path.join(fixtures, 'extensions', 'chrome-runtime'))
       const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } })
       try {
         await w.loadURL(url)
@@ -76,7 +108,7 @@ ifdescribe(process.electronBinding('features').isExtensionsEnabled())('chrome ex
   describe('chrome.storage', () => {
     it('stores and retrieves a key', async () => {
       const customSession = session.fromPartition(`persist:${require('uuid').v4()}`);
-      (customSession as any).loadChromeExtension(path.join(fixtures, 'extensions', 'chrome-storage'))
+      (customSession as any).loadExtension(path.join(fixtures, 'extensions', 'chrome-storage'))
       const w = new BrowserWindow({ show: false, webPreferences: { session: customSession, nodeIntegration: true } })
       try {
         const p = emittedOnce(ipcMain, 'storage-success')

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ const Extension* AtomExtensionSystem::LoadExtension(`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`const Extension* AtomExtensionSystem::LoadApp(const base::FilePath& app_dir) {`
`52`		`- CHECK(false); // Should never call LoadApp`
	`52`	`+ NOTIMPLEMENTED() << "Attempted to load platform app in Electron";`
`53`	`53`	`return nullptr;`
`54`	`54`	`}`
`55`	`55`
`@@ -66,6 +66,11 @@ void AtomExtensionSystem::ReloadExtension(const ExtensionId& extension_id) {`
`66`	`66`	`extension_loader_->ReloadExtension(extension_id);`
`67`	`67`	`}`
`68`	`68`
	`69`	`+void AtomExtensionSystem::RemoveExtension(const ExtensionId& extension_id) {`
	`70`	`+ extension_loader_->UnloadExtension(`
	`71`	`+ extension_id, extensions::UnloadedExtensionReason::UNINSTALL);`
	`72`	`+}`
	`73`	`+`
`69`	`74`	`void AtomExtensionSystem::Shutdown() {`
`70`	`75`	`extension_loader_.reset();`
`71`	`76`	`}`