Browse Source

fix: `nativeImage.createThumbnailFromPath` and `shell.openExternal` in renderer (#41875)

* fix: nativeImage.createThumbnailFromPath in renderer

* also fix shell.openExternal
Jeremy Rose 1 year ago
parent
commit
ed9fec7da4

+ 25 - 23
shell/common/api/electron_api_native_image_mac.mm

@@ -14,6 +14,7 @@
 
 #include "base/apple/foundation_util.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/task/bind_post_task.h"
 #include "gin/arguments.h"
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_helper/promise.h"
@@ -38,6 +39,23 @@ double safeShift(double in, double def) {
   return def;
 }
 
+void ReceivedThumbnailResult(CGSize size,
+                             gin_helper::Promise<gfx::Image> p,
+                             QLThumbnailRepresentation* thumbnail,
+                             NSError* error) {
+  if (error || !thumbnail) {
+    std::string err_msg([error.localizedDescription UTF8String]);
+    p.RejectWithErrorMessage("unable to retrieve thumbnail preview "
+                             "image for the given path: " +
+                             err_msg);
+  } else {
+    NSImage* result = [[NSImage alloc] initWithCGImage:[thumbnail CGImage]
+                                                  size:size];
+    gfx::Image image(result);
+    p.Resolve(image);
+  }
+}
+
 // static
 v8::Local<v8::Promise> NativeImage::CreateThumbnailFromPath(
     v8::Isolate* isolate,
@@ -70,31 +88,15 @@ v8::Local<v8::Promise> NativeImage::CreateThumbnailFromPath(
                      size:cg_size
                     scale:[screen backingScaleFactor]
       representationTypes:QLThumbnailGenerationRequestRepresentationTypeAll]);
-  __block gin_helper::Promise<gfx::Image> p = std::move(promise);
+  __block auto block_callback = base::BindPostTaskToCurrentDefault(
+      base::BindOnce(&ReceivedThumbnailResult, cg_size, std::move(promise)));
+  auto completionHandler =
+      ^(QLThumbnailRepresentation* thumbnail, NSError* error) {
+        std::move(block_callback).Run(thumbnail, error);
+      };
   [[QLThumbnailGenerator sharedGenerator]
       generateBestRepresentationForRequest:request
-                         completionHandler:^(
-                             QLThumbnailRepresentation* thumbnail,
-                             NSError* error) {
-                           if (error || !thumbnail) {
-                             std::string err_msg(
-                                 [error.localizedDescription UTF8String]);
-                             dispatch_async(dispatch_get_main_queue(), ^{
-                               p.RejectWithErrorMessage(
-                                   "unable to retrieve thumbnail preview "
-                                   "image for the given path: " +
-                                   err_msg);
-                             });
-                           } else {
-                             NSImage* result = [[NSImage alloc]
-                                 initWithCGImage:[thumbnail CGImage]
-                                            size:cg_size];
-                             gfx::Image image(result);
-                             dispatch_async(dispatch_get_main_queue(), ^{
-                               p.Resolve(image);
-                             });
-                           }
-                         }];
+                         completionHandler:completionHandler];
 
   return handle;
 }

+ 10 - 9
shell/common/platform_util_mac.mm

@@ -15,11 +15,15 @@
 #include "base/apple/osstatus_logging.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/functional/bind.h"
 #include "base/functional/callback.h"
 #include "base/logging.h"
 #include "base/mac/scoped_aedesc.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/task/thread_pool.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "net/base/apple/url_conversions.h"
 #include "ui/views/widget/widget.h"
 #include "url/gurl.h"
@@ -183,15 +187,12 @@ void OpenExternal(const GURL& url,
     return;
   }
 
-  bool activate = options.activate;
-  __block OpenCallback c = std::move(callback);
-  dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),
-                 ^{
-                   __block std::string error = OpenURL(ns_url, activate);
-                   dispatch_async(dispatch_get_main_queue(), ^{
-                     std::move(c).Run(error);
-                   });
-                 });
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE,
+      {base::MayBlock(), base::WithBaseSyncPrimitives(),
+       base::TaskPriority::USER_BLOCKING,
+       base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
+      base::BindOnce(&OpenURL, ns_url, options.activate), std::move(callback));
 }
 
 bool MoveItemToTrashWithError(const base::FilePath& full_path,

+ 10 - 1
spec/api-native-image-spec.ts

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 import { nativeImage } from 'electron/common';
-import { ifdescribe, ifit } from './lib/spec-helpers';
+import { ifdescribe, ifit, itremote, useRemoteContext } from './lib/spec-helpers';
 import * as path from 'node:path';
 
 describe('nativeImage module', () => {
@@ -426,6 +426,8 @@ describe('nativeImage module', () => {
   });
 
   ifdescribe(process.platform !== 'linux')('createThumbnailFromPath(path, size)', () => {
+    useRemoteContext({ webPreferences: { contextIsolation: false, nodeIntegration: true } });
+
     it('throws when invalid size is passed', async () => {
       const badSize = { width: -1, height: -1 };
 
@@ -473,6 +475,13 @@ describe('nativeImage module', () => {
       const result = await nativeImage.createThumbnailFromPath(imgPath, maxSize);
       expect(result.getSize()).to.deep.equal(maxSize);
     });
+
+    itremote('works in the renderer', async (path: string) => {
+      const { nativeImage } = require('electron');
+      const goodSize = { width: 100, height: 100 };
+      const result = await nativeImage.createThumbnailFromPath(path, goodSize);
+      expect(result.isEmpty()).to.equal(false);
+    }, [path.join(fixturesPath, 'assets', 'logo.png')]);
   });
 
   describe('addRepresentation()', () => {

+ 15 - 1
spec/api-shell-spec.ts

@@ -31,7 +31,7 @@ describe('shell module', () => {
     });
     afterEach(closeAllWindows);
 
-    it('opens an external link', async () => {
+    async function urlOpened () {
       let url = 'http://127.0.0.1';
       let requestReceived: Promise<any>;
       if (process.platform === 'linux') {
@@ -53,12 +53,26 @@ describe('shell module', () => {
         url = (await listen(server)).url;
         requestReceived = new Promise<void>(resolve => server.on('connection', () => resolve()));
       }
+      return { url, requestReceived };
+    }
 
+    it('opens an external link', async () => {
+      const { url, requestReceived } = await urlOpened();
       await Promise.all<void>([
         shell.openExternal(url),
         requestReceived
       ]);
     });
+
+    it('opens an external link in the renderer', async () => {
+      const { url, requestReceived } = await urlOpened();
+      const w = new BrowserWindow({ show: false, webPreferences: { sandbox: false, contextIsolation: false, nodeIntegration: true } });
+      await w.loadURL('about:blank');
+      await Promise.all<void>([
+        w.webContents.executeJavaScript(`require("electron").shell.openExternal(${JSON.stringify(url)})`),
+        requestReceived
+      ]);
+    });
   });
 
   describe('shell.trashItem()', () => {