From ee0f67d5410ab6fc3f504c4d27e2aec772f1f4f0 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 18 May 2020 09:29:24 -0700 Subject: [PATCH] fix: nativeImage remote serialization (#23543) We weren't serializing nativeImages properly in the remote module, leading to gin conversion errors when trying to, for example, create a new context menu in the renderer with icons using nativeImage. This fixes that by adding a new special case to handle them. --- lib/browser/remote/server.ts | 17 +++++++ lib/renderer/api/remote.js | 11 ++++- shell/common/api/electron_api_native_image.cc | 33 +++++++++----- shell/common/api/electron_api_native_image.h | 8 ++-- spec-main/api-remote-spec.ts | 45 +++++++++++++++++++ 5 files changed, 100 insertions(+), 14 deletions(-) diff --git a/lib/browser/remote/server.ts b/lib/browser/remote/server.ts index 2bdeed3ea4a3..bd9bfabbd051 100644 --- a/lib/browser/remote/server.ts +++ b/lib/browser/remote/server.ts @@ -5,6 +5,7 @@ import { EventEmitter } from 'events'; import objectsRegistry from './objects-registry'; import { ipcMainInternal } from '../ipc-main-internal'; import { isPromise, isSerializableObject } from '@electron/internal/common/type-utils'; +import { Size } from 'electron/main'; const v8Util = process.electronBinding('v8_util'); const eventBinding = process.electronBinding('event'); @@ -242,6 +243,9 @@ type MetaTypeFromRenderer = { id: number, location: string, length: number +} | { + type: 'nativeimage', + value: { size: Size, buffer: Buffer, scaleFactor: number }[] } const fakeConstructor = (constructor: Function, name: string) => @@ -259,6 +263,19 @@ const fakeConstructor = (constructor: Function, name: string) => const unwrapArgs = function (sender: electron.WebContents, frameId: number, contextId: string, args: any[]) { const metaToValue = function (meta: MetaTypeFromRenderer): any { switch (meta.type) { + case 'nativeimage': { + const image = electron.nativeImage.createEmpty(); + for (const rep of meta.value) { + const { buffer, size, scaleFactor } = rep; + image.addRepresentation({ + buffer, + width: size.width, + height: size.height, + scaleFactor + }); + } + return image; + } case 'value': return meta.value; case 'remote-object': diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index 571fe2d29be1..dc154e519830 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -2,6 +2,7 @@ const v8Util = process.electronBinding('v8_util'); const { hasSwitch } = process.electronBinding('command_line'); +const { NativeImage } = process.electronBinding('native_image'); const { CallbacksRegistry } = require('@electron/internal/renderer/remote/callbacks-registry'); const { isPromise, isSerializableObject } = require('@electron/internal/common/type-utils'); @@ -35,7 +36,15 @@ function wrapArgs (args, visited = new Set()) { }; } - if (Array.isArray(value)) { + if (value instanceof NativeImage) { + const images = []; + for (const scaleFactor of value.getScaleFactors()) { + const size = value.getSize(scaleFactor); + const buffer = value.toBitmap({ scaleFactor }); + images.push({ buffer, scaleFactor, size }); + } + return { type: 'nativeimage', value: images }; + } else if (Array.isArray(value)) { visited.add(value); const meta = { type: 'array', diff --git a/shell/common/api/electron_api_native_image.cc b/shell/common/api/electron_api_native_image.cc index 50ff68a48250..41f64ae3e686 100644 --- a/shell/common/api/electron_api_native_image.cc +++ b/shell/common/api/electron_api_native_image.cc @@ -248,21 +248,32 @@ bool NativeImage::IsEmpty() { return image_.IsEmpty(); } -gfx::Size NativeImage::GetSize() { - return image_.Size(); +gfx::Size NativeImage::GetSize(const base::Optional scale_factor) { + float sf = scale_factor.value_or(1.0f); + gfx::ImageSkiaRep image_rep = image_.AsImageSkia().GetRepresentation(sf); + + return gfx::Size(image_rep.GetWidth(), image_rep.GetHeight()); } -float NativeImage::GetAspectRatio() { - gfx::Size size = GetSize(); +std::vector NativeImage::GetScaleFactors() { + gfx::ImageSkia image_skia = image_.AsImageSkia(); + return image_skia.GetSupportedScales(); +} + +float NativeImage::GetAspectRatio(const base::Optional scale_factor) { + float sf = scale_factor.value_or(1.0f); + gfx::Size size = GetSize(sf); if (size.IsEmpty()) return 1.f; else return static_cast(size.width()) / static_cast(size.height()); } -gin::Handle NativeImage::Resize(v8::Isolate* isolate, +gin::Handle NativeImage::Resize(gin::Arguments* args, base::DictionaryValue options) { - gfx::Size size = GetSize(); + float scale_factor = GetScaleFactorFromOptions(args); + + gfx::Size size = GetSize(scale_factor); int width = size.width(); int height = size.height(); bool width_set = options.GetInteger("width", &width); @@ -272,11 +283,12 @@ gin::Handle NativeImage::Resize(v8::Isolate* isolate, if (width_set && !height_set) { // Scale height to preserve original aspect ratio size.set_height(width); - size = gfx::ScaleToRoundedSize(size, 1.f, 1.f / GetAspectRatio()); + size = + gfx::ScaleToRoundedSize(size, 1.f, 1.f / GetAspectRatio(scale_factor)); } else if (height_set && !width_set) { // Scale width to preserve original aspect ratio size.set_width(height); - size = gfx::ScaleToRoundedSize(size, GetAspectRatio(), 1.f); + size = gfx::ScaleToRoundedSize(size, GetAspectRatio(scale_factor), 1.f); } skia::ImageOperations::ResizeMethod method = @@ -290,8 +302,8 @@ gin::Handle NativeImage::Resize(v8::Isolate* isolate, gfx::ImageSkia resized = gfx::ImageSkiaOperations::CreateResizedImage( image_.AsImageSkia(), method, size); - return gin::CreateHandle(isolate, - new NativeImage(isolate, gfx::Image(resized))); + return gin::CreateHandle( + args->isolate(), new NativeImage(args->isolate(), gfx::Image(resized))); } gin::Handle NativeImage::Crop(v8::Isolate* isolate, @@ -505,6 +517,7 @@ void NativeImage::BuildPrototype(v8::Isolate* isolate, .SetMethod("toJPEG", &NativeImage::ToJPEG) .SetMethod("toBitmap", &NativeImage::ToBitmap) .SetMethod("getBitmap", &NativeImage::GetBitmap) + .SetMethod("getScaleFactors", &NativeImage::GetScaleFactors) .SetMethod("getNativeHandle", &NativeImage::GetNativeHandle) .SetMethod("toDataURL", &NativeImage::ToDataURL) .SetMethod("isEmpty", &NativeImage::IsEmpty) diff --git a/shell/common/api/electron_api_native_image.h b/shell/common/api/electron_api_native_image.h index 1cb438265a43..4b9ced24602f 100644 --- a/shell/common/api/electron_api_native_image.h +++ b/shell/common/api/electron_api_native_image.h @@ -7,6 +7,7 @@ #include #include +#include #include "base/values.h" #include "gin/handle.h" @@ -84,15 +85,16 @@ class NativeImage : public gin_helper::Wrappable { v8::Local ToPNG(gin::Arguments* args); v8::Local ToJPEG(v8::Isolate* isolate, int quality); v8::Local ToBitmap(gin::Arguments* args); + std::vector GetScaleFactors(); v8::Local GetBitmap(gin::Arguments* args); v8::Local GetNativeHandle(gin_helper::ErrorThrower thrower); - gin::Handle Resize(v8::Isolate* isolate, + gin::Handle Resize(gin::Arguments* args, base::DictionaryValue options); gin::Handle Crop(v8::Isolate* isolate, const gfx::Rect& rect); std::string ToDataURL(gin::Arguments* args); bool IsEmpty(); - gfx::Size GetSize(); - float GetAspectRatio(); + gfx::Size GetSize(const base::Optional scale_factor); + float GetAspectRatio(const base::Optional scale_factor); void AddRepresentation(const gin_helper::Dictionary& options); // Mark the image as template image. diff --git a/spec-main/api-remote-spec.ts b/spec-main/api-remote-spec.ts index 7d3c9b43371c..68564ad2432f 100644 --- a/spec-main/api-remote-spec.ts +++ b/spec-main/api-remote-spec.ts @@ -5,6 +5,7 @@ import { ifdescribe } from './spec-helpers'; import { ipcMain, BrowserWindow } from 'electron/main'; import { emittedOnce } from './events-helpers'; +import { NativeImage } from 'electron/common'; const features = process.electronBinding('features'); @@ -222,6 +223,50 @@ ifdescribe(features.isRemoteModuleEnabled())('remote module', () => { }); }); + describe('nativeImage serialization', () => { + const w = makeWindow(); + const remotely = makeRemotely(w); + + it('can serialize an empty nativeImage', async () => { + const getEmptyImage = (img: NativeImage) => img.isEmpty(); + + w().webContents.once('remote-get-global', (event) => { + event.returnValue = getEmptyImage; + }); + + await expect(remotely(() => { + const emptyImage = require('electron').nativeImage.createEmpty(); + return require('electron').remote.getGlobal('someFunction')(emptyImage); + })).to.eventually.be.true(); + }); + + it('can serialize a non-empty nativeImage', async () => { + const getNonEmptyImage = (img: NativeImage) => img.getSize(); + + w().webContents.once('remote-get-global', (event) => { + event.returnValue = getNonEmptyImage; + }); + + await expect(remotely(() => { + const { nativeImage } = require('electron'); + const nonEmptyImage = nativeImage.createFromDataURL('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAIAAAACCAIAAAD91JpzAAAAFklEQVQYlWP8//8/AwMDEwMDAwMDAwAkBgMBBMzldwAAAABJRU5ErkJggg=='); + return require('electron').remote.getGlobal('someFunction')(nonEmptyImage); + })).to.eventually.deep.equal({ width: 2, height: 2 }); + }); + + it('can properly create a menu with an nativeImage icon in the renderer', async () => { + await expect(remotely(() => { + const { remote, nativeImage } = require('electron'); + remote.Menu.buildFromTemplate([ + { + label: 'hello', + icon: nativeImage.createEmpty() + } + ]); + })).to.be.fulfilled(); + }); + }); + describe('remote listeners', () => { afterEach(closeAllWindows);