From 91f257044dcfb55c43bf37bfe71b6f47e858aef9 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Fri, 19 Apr 2024 10:56:43 -0500 Subject: [PATCH] fix: `nativeImage.createThumbnailFromPath` and `shell.openExternal` in renderer (#41909) * fix: nativeImage.createThumbnailFromPath in renderer Co-authored-by: Jeremy Rose * also fix shell.openExternal Co-authored-by: Jeremy Rose --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Jeremy Rose --- .../api/electron_api_native_image_mac.mm | 48 ++++++++++--------- shell/common/platform_util_mac.mm | 19 ++++---- spec/api-native-image-spec.ts | 11 ++++- spec/api-shell-spec.ts | 16 ++++++- 4 files changed, 60 insertions(+), 34 deletions(-) diff --git a/shell/common/api/electron_api_native_image_mac.mm b/shell/common/api/electron_api_native_image_mac.mm index cf9c6cdcfa37..e45eaae9a938 100644 --- a/shell/common/api/electron_api_native_image_mac.mm +++ b/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 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 NativeImage::CreateThumbnailFromPath( v8::Isolate* isolate, @@ -70,31 +88,15 @@ v8::Local NativeImage::CreateThumbnailFromPath( size:cg_size scale:[screen backingScaleFactor] representationTypes:QLThumbnailGenerationRequestRepresentationTypeAll]); - __block gin_helper::Promise 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; } diff --git a/shell/common/platform_util_mac.mm b/shell/common/platform_util_mac.mm index 60419c7b28da..32972a5dfe44 100644 --- a/shell/common/platform_util_mac.mm +++ b/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, diff --git a/spec/api-native-image-spec.ts b/spec/api-native-image-spec.ts index 0e277043a0a7..f37f42b97803 100644 --- a/spec/api-native-image-spec.ts +++ b/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', () => { @@ -424,6 +424,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 }; @@ -471,6 +473,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()', () => { diff --git a/spec/api-shell-spec.ts b/spec/api-shell-spec.ts index 749677311bd5..a2e9cef96ff8 100644 --- a/spec/api-shell-spec.ts +++ b/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; if (process.platform === 'linux') { @@ -53,12 +53,26 @@ describe('shell module', () => { url = (await listen(server)).url; requestReceived = new Promise(resolve => server.on('connection', () => resolve())); } + return { url, requestReceived }; + } + it('opens an external link', async () => { + const { url, requestReceived } = await urlOpened(); await Promise.all([ 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([ + w.webContents.executeJavaScript(`require("electron").shell.openExternal(${JSON.stringify(url)})`), + requestReceived + ]); + }); }); describe('shell.trashItem()', () => {