feat: promisify app.getFileIcon() (#15742)

atom/browser/api/atom_api_app.cc

@@ -536,17 +536,15 @@ int ImportIntoCertStore(CertificateManagerModel* model,
 #endif
 
 void OnIconDataAvailable(v8::Isolate* isolate,
-                         const App::FileIconCallback& callback,
+                         scoped_refptr<util::Promise> promise,
                          gfx::Image* icon) {
   v8::Locker locker(isolate);
   v8::HandleScope handle_scope(isolate);
 
   if (icon && !icon->IsEmpty()) {
-    callback.Run(v8::Null(isolate), *icon);
+    promise->Resolve(*icon);
   } else {
-    v8::Local<v8::String> error_message =
-        v8::String::NewFromUtf8(isolate, "Failed to get file icon.");
-    callback.Run(v8::Exception::Error(error_message), gfx::Image());
+    promise->RejectWithErrorMessage("Failed to get file icon.");
   }
 }
 
@@ -1125,16 +1123,16 @@ JumpListResult App::SetJumpList(v8::Local<v8::Value> val,
 }
 #endif  // defined(OS_WIN)
 
-void App::GetFileIcon(const base::FilePath& path, mate::Arguments* args) {
-  mate::Dictionary options;
-  IconLoader::IconSize icon_size;
-  FileIconCallback callback;
-
+v8::Local<v8::Promise> App::GetFileIcon(const base::FilePath& path,
+                                        mate::Arguments* args) {
   v8::Locker locker(isolate());
   v8::HandleScope handle_scope(isolate());
 
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate());
   base::FilePath normalized_path = path.NormalizePathSeparators();
 
+  IconLoader::IconSize icon_size;
+  mate::Dictionary options;
   if (!args->GetNext(&options)) {
     icon_size = IconLoader::IconSize::NORMAL;
   } else {
@@ -1143,22 +1141,17 @@ void App::GetFileIcon(const base::FilePath& path, mate::Arguments* args) {
     icon_size = GetIconSizeByString(icon_size_string);
   }
 
-  if (!args->GetNext(&callback)) {
-    args->ThrowError("Missing required callback function");
-    return;
-  }
-
   auto* icon_manager = AtomBrowserMainParts::Get()->GetIconManager();
   gfx::Image* icon =
       icon_manager->LookupIconFromFilepath(normalized_path, icon_size);
   if (icon) {
-    callback.Run(v8::Null(isolate()), *icon);
+    promise->Resolve(*icon);
   } else {
-    icon_manager->LoadIcon(
-        normalized_path, icon_size,
-        base::Bind(&OnIconDataAvailable, isolate(), callback),
-        &cancelable_task_tracker_);
+    icon_manager->LoadIcon(normalized_path, icon_size,
+                           base::Bind(&OnIconDataAvailable, isolate(), promise),
+                           &cancelable_task_tracker_);
   }
+  return promise->GetHandle();
 }
 
 std::vector<mate::Dictionary> App::GetAppMetrics(v8::Isolate* isolate) {

atom/browser/api/atom_api_app.h

@@ -198,7 +198,8 @@ class App : public AtomBrowserClient::Delegate,
   void ImportCertificate(const base::DictionaryValue& options,
                          const net::CompletionCallback& callback);
 #endif
-  void GetFileIcon(const base::FilePath& path, mate::Arguments* args);
+  v8::Local<v8::Promise> GetFileIcon(const base::FilePath& path,
+                                     mate::Arguments* args);
 
   std::vector<mate::Dictionary> GetAppMetrics(v8::Isolate* isolate);
   v8::Local<v8::Value> GetGPUFeatureStatus(v8::Isolate* isolate);

docs/api/app.md

@@ -552,8 +552,29 @@ On _Windows_, there a 2 kinds of icons:
 - Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
 - Icons inside the file itself, like `.exe`, `.dll`, `.ico`.
 
-On _Linux_ and _macOS_, icons depend on the application associated with file
-mime type.
+On _Linux_ and _macOS_, icons depend on the application associated with file mime type.
+
+**[Deprecated Soon](promisification.md)**
+
+### `app.getFileIcon(path[, options])`
+
+* `path` String
+* `options` Object (optional)
+  * `size` String
+    * `small` - 16x16
+    * `normal` - 32x32
+    * `large` - 48x48 on _Linux_, 32x32 on _Windows_, unsupported on _macOS_.
+
+Returns `Promise<NativeImage>` - fulfilled with the app's icon, which is a [NativeImage](native-image.md).
+
+Fetches a path's associated icon.
+
+On _Windows_, there a 2 kinds of icons:
+
+- Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
+- Icons inside the file itself, like `.exe`, `.dll`, `.ico`.
+
+On _Linux_ and _macOS_, icons depend on the application associated with file mime type.
 
 ### `app.setPath(name, path)`
 

lib/browser/api/app.js

@@ -40,9 +40,14 @@ Object.assign(app, {
   }
 })
 
-const nativeFn = app.getAppMetrics
+const nativeGetFileIcon = app.getFileIcon
+app.getFileIcon = (path, options = {}, cb) => {
+  return deprecate.promisify(nativeGetFileIcon.call(app, path, options), cb)
+}
+
+const nativeAppMetrics = app.getAppMetrics
 app.getAppMetrics = () => {
-  const metrics = nativeFn.call(app)
+  const metrics = nativeAppMetrics.call(app)
   for (const metric of metrics) {
     if ('memory' in metric) {
       deprecate.removeProperty(metric, 'memory')

spec/api-app-spec.js

@@ -769,61 +769,45 @@ describe('app module', () => {
       }
     })
 
-    it('fetches a non-empty icon', done => {
-      app.getFileIcon(iconPath, (err, icon) => {
-        expect(err).to.be.null()
-        expect(icon.isEmpty()).to.be.false()
-        done()
-      })
+    it('fetches a non-empty icon', async () => {
+      const icon = await app.getFileIcon(iconPath)
+      expect(icon.isEmpty()).to.be.false()
     })
 
-    it('fetches normal icon size by default', done => {
-      app.getFileIcon(iconPath, (err, icon) => {
-        const size = icon.getSize()
+    it('fetches normal icon size by default', async () => {
+      const icon = await app.getFileIcon(iconPath)
+      const size = icon.getSize()
 
-        expect(err).to.be.null()
-        expect(size.height).to.equal(sizes.normal)
-        expect(size.width).to.equal(sizes.normal)
-        done()
-      })
+      expect(size.height).to.equal(sizes.normal)
+      expect(size.width).to.equal(sizes.normal)
     })
 
     describe('size option', () => {
-      it('fetches a small icon', (done) => {
-        app.getFileIcon(iconPath, { size: 'small' }, (err, icon) => {
-          const size = icon.getSize()
-          expect(err).to.be.null()
-          expect(size.height).to.equal(sizes.small)
-          expect(size.width).to.equal(sizes.small)
-          done()
-        })
+      it('fetches a small icon', async () => {
+        const icon = await app.getFileIcon(iconPath, { size: 'small' })
+        const size = icon.getSize()
+
+        expect(size.height).to.equal(sizes.small)
+        expect(size.width).to.equal(sizes.small)
       })
 
-      it('fetches a normal icon', (done) => {
-        app.getFileIcon(iconPath, { size: 'normal' }, (err, icon) => {
-          const size = icon.getSize()
-          expect(err).to.be.null()
-          expect(size.height).to.equal(sizes.normal)
-          expect(size.width).to.equal(sizes.normal)
-          done()
-        })
+      it('fetches a normal icon', async () => {
+        const icon = await app.getFileIcon(iconPath, { size: 'normal' })
+        const size = icon.getSize()
+
+        expect(size.height).to.equal(sizes.normal)
+        expect(size.width).to.equal(sizes.normal)
       })
 
-      it('fetches a large icon', function (done) {
+      it('fetches a large icon', async () => {
         // macOS does not support large icons
-        if (process.platform === 'darwin') {
-          // FIXME(alexeykuzmin): Skip the test.
-          // this.skip()
-          return done()
-        }
+        if (process.platform === 'darwin') return
 
-        app.getFileIcon(iconPath, { size: 'large' }, (err, icon) => {
-          const size = icon.getSize()
-          expect(err).to.be.null()
-          expect(size.height).to.equal(sizes.large)
-          expect(size.width).to.equal(sizes.large)
-          done()
-        })
+        const icon = await app.getFileIcon(iconPath, { size: 'large' })
+        const size = icon.getSize()
+
+        expect(size.height).to.equal(sizes.large)
+        expect(size.width).to.equal(sizes.large)
       })
     })
   })