From 9a54ffb7c7263b14771ff078e2716696d8c9b73b Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 8 Jun 2020 16:23:42 -0500 Subject: [PATCH] fix: nativeImage remote serialization --- lib/browser/rpc-server.js | 11 +- lib/common/type-utils.ts | 53 ++++++++- lib/renderer/api/remote.js | 7 +- native_mate/native_mate/function_template.h | 13 +++ shell/common/api/atom_api_native_image.cc | 32 ++++-- shell/common/api/atom_api_native_image.h | 8 +- spec-main/api-remote-spec.ts | 116 +++++++++++++++++++- spec-main/package.json | 1 + spec-main/yarn.lock | 20 ++++ 9 files changed, 238 insertions(+), 23 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 42c460c90a523..f7166f411db76 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -8,6 +8,7 @@ const v8Util = process.electronBinding('v8_util'); const eventBinding = process.electronBinding('event'); const clipboard = process.electronBinding('clipboard'); const features = process.electronBinding('features'); +const { NativeImage } = process.electronBinding('native_image'); const { getContentScripts } = require('@electron/internal/browser/chrome-extension'); const { crashReporterInit } = require('@electron/internal/browser/crash-reporter-init'); @@ -17,7 +18,7 @@ const objectsRegistry = require('@electron/internal/browser/objects-registry'); const guestViewManager = require('@electron/internal/browser/guest-view-manager'); const bufferUtils = require('@electron/internal/common/buffer-utils'); const errorUtils = require('@electron/internal/common/error-utils'); -const typeUtils = require('@electron/internal/common/type-utils'); +const { deserialize, serialize } = require('@electron/internal/common/type-utils'); const { isPromise } = require('@electron/internal/common/is-promise'); const hasProp = {}.hasOwnProperty; @@ -77,6 +78,8 @@ const valueToMeta = function (sender, contextId, value, optimizeSimpleObject = f meta.type = 'buffer'; } else if (Array.isArray(value)) { meta.type = 'array'; + } else if (value instanceof NativeImage) { + meta.type = 'nativeimage'; } else if (value instanceof Error) { meta.type = 'error'; } else if (value instanceof Date) { @@ -95,6 +98,8 @@ const valueToMeta = function (sender, contextId, value, optimizeSimpleObject = f // Fill the meta object according to value's type. if (meta.type === 'array') { meta.members = value.map((el) => valueToMeta(sender, contextId, el, optimizeSimpleObject)); + } else if (meta.type === 'nativeimage') { + meta.value = serialize(value); } else if (meta.type === 'object' || meta.type === 'function') { meta.name = value.constructor ? value.constructor.name : ''; @@ -181,6 +186,8 @@ const removeRemoteListenersAndLogWarning = (sender, callIntoRenderer) => { const unwrapArgs = function (sender, frameId, contextId, args) { const metaToValue = function (meta) { switch (meta.type) { + case 'nativeimage': + return deserialize(meta.value); case 'value': return meta.value; case 'remote-object': @@ -496,7 +503,7 @@ ipcMainUtils.handle('ELECTRON_BROWSER_CLIPBOARD', function (event, method, ...ar throw new Error(`Invalid method: ${method}`); } - return typeUtils.serialize(electron.clipboard[method](...typeUtils.deserialize(args))); + return serialize(electron.clipboard[method](...deserialize(args))); }); if (features.isDesktopCapturerEnabled()) { diff --git a/lib/common/type-utils.ts b/lib/common/type-utils.ts index 2a5e94fc89bea..b3d36dcdebb98 100644 --- a/lib/common/type-utils.ts +++ b/lib/common/type-utils.ts @@ -6,13 +6,54 @@ const objectMap = function (source: Object, mapper: (value: any) => any) { return Object.fromEntries(targetEntries); }; +function serializeNativeImage (image: any) { + const representations = []; + const scaleFactors = image.getScaleFactors(); + + // Use Buffer when there's only one representation for better perf. + // This avoids compressing to/from PNG where it's not necessary to + // ensure uniqueness of dataURLs (since there's only one). + if (scaleFactors.length === 1) { + const scaleFactor = scaleFactors[0]; + const size = image.getSize(scaleFactor); + const buffer = image.toBitmap({ scaleFactor }); + representations.push({ scaleFactor, size, buffer }); + } else { + // Construct from dataURLs to ensure that they are not lost in creation. + for (const scaleFactor of scaleFactors) { + const size = image.getSize(scaleFactor); + const dataURL = image.toDataURL({ scaleFactor }); + representations.push({ scaleFactor, size, dataURL }); + } + } + return { __ELECTRON_SERIALIZED_NativeImage__: true, representations }; +} + +function deserializeNativeImage (value: any) { + const image = nativeImage.createEmpty(); + + // Use Buffer when there's only one representation for better perf. + // This avoids compressing to/from PNG where it's not necessary to + // ensure uniqueness of dataURLs (since there's only one). + if (value.representations.length === 1) { + const { buffer, size, scaleFactor } = value.representations[0]; + const { width, height } = size; + image.addRepresentation({ buffer, scaleFactor, width, height }); + } else { + // Construct from dataURLs to ensure that they are not lost in creation. + for (const rep of value.representations) { + const { dataURL, size, scaleFactor } = rep; + const { width, height } = size; + image.addRepresentation({ dataURL, scaleFactor, width, height }); + } + } + + return image; +} + export function serialize (value: any): any { if (value instanceof NativeImage) { - return { - buffer: value.toBitmap(), - size: value.getSize(), - __ELECTRON_SERIALIZED_NativeImage__: true - }; + return serializeNativeImage(value); } else if (Array.isArray(value)) { return value.map(serialize); } else if (value instanceof Buffer) { @@ -26,7 +67,7 @@ export function serialize (value: any): any { export function deserialize (value: any): any { if (value && value.__ELECTRON_SERIALIZED_NativeImage__) { - return nativeImage.createFromBitmap(value.buffer, value.size); + return deserializeNativeImage(value); } else if (Array.isArray(value)) { return value.map(deserialize); } else if (value instanceof Buffer) { diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index a5bbefb52312a..6b0df07ab99d4 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -1,10 +1,12 @@ 'use strict'; const v8Util = process.electronBinding('v8_util'); +const { NativeImage } = process.electronBinding('native_image'); const { CallbacksRegistry } = require('@electron/internal/renderer/callbacks-registry'); const bufferUtils = require('@electron/internal/common/buffer-utils'); const errorUtils = require('@electron/internal/common/error-utils'); +const { deserialize, serialize } = require('@electron/internal/common/type-utils'); const { isPromise } = require('@electron/internal/common/is-promise'); const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal'); @@ -34,7 +36,9 @@ function wrapArgs (args, visited = new Set()) { }; } - if (Array.isArray(value)) { + if (value instanceof NativeImage) { + return { type: 'nativeimage', value: serialize(value) }; + } else if (Array.isArray(value)) { visited.add(value); const meta = { type: 'array', @@ -214,6 +218,7 @@ function metaToValue (meta) { const types = { value: () => meta.value, array: () => meta.members.map((member) => metaToValue(member)), + nativeimage: () => deserialize(meta.value), buffer: () => bufferUtils.metaToBuffer(meta.value), promise: () => Promise.resolve({ then: metaToValue(meta.then) }), error: () => metaToPlainObject(meta), diff --git a/native_mate/native_mate/function_template.h b/native_mate/native_mate/function_template.h index 77dc90a1406e8..82d18047ec044 100644 --- a/native_mate/native_mate/function_template.h +++ b/native_mate/native_mate/function_template.h @@ -9,6 +9,7 @@ #include "base/callback.h" #include "base/logging.h" +#include "base/optional.h" #include "native_mate/arguments.h" #include "native_mate/wrappable_base.h" #include "v8/include/v8.h" @@ -104,6 +105,18 @@ bool GetNextArgument(Arguments* args, } } +// Support base::Optional as an argument. +template +bool GetNextArgument(Arguments* args, + int create_flags, + bool is_first, + base::Optional* result) { + T converted; + if (args->GetNext(&converted)) + result->emplace(std::move(converted)); + return true; +} + // For advanced use cases, we allow callers to request the unparsed Arguments // object and poke around in it directly. inline bool GetNextArgument(Arguments* args, diff --git a/shell/common/api/atom_api_native_image.cc b/shell/common/api/atom_api_native_image.cc index 2f7ee324a7a90..340dcb8451026 100644 --- a/shell/common/api/atom_api_native_image.cc +++ b/shell/common/api/atom_api_native_image.cc @@ -246,12 +246,20 @@ 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 @@ -259,9 +267,11 @@ float NativeImage::GetAspectRatio() { } mate::Handle NativeImage::Resize( - v8::Isolate* isolate, + mate::Arguments* args, const base::DictionaryValue& options) { - gfx::Size size = GetSize(); + const 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); @@ -271,11 +281,12 @@ mate::Handle NativeImage::Resize( 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 = @@ -289,8 +300,8 @@ mate::Handle NativeImage::Resize( gfx::ImageSkia resized = gfx::ImageSkiaOperations::CreateResizedImage( image_.AsImageSkia(), method, size); - return mate::CreateHandle(isolate, - new NativeImage(isolate, gfx::Image(resized))); + return mate::CreateHandle( + args->isolate(), new NativeImage(args->isolate(), gfx::Image(resized))); } mate::Handle NativeImage::Crop(v8::Isolate* isolate, @@ -504,6 +515,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/atom_api_native_image.h b/shell/common/api/atom_api_native_image.h index 5915c74e1034a..14b6be626269f 100644 --- a/shell/common/api/atom_api_native_image.h +++ b/shell/common/api/atom_api_native_image.h @@ -7,6 +7,7 @@ #include #include +#include #include "base/values.h" #include "native_mate/dictionary.h" @@ -84,16 +85,17 @@ class NativeImage : public mate::Wrappable { v8::Local ToPNG(mate::Arguments* args); v8::Local ToJPEG(v8::Isolate* isolate, int quality); v8::Local ToBitmap(mate::Arguments* args); + std::vector GetScaleFactors(); v8::Local GetBitmap(mate::Arguments* args); v8::Local GetNativeHandle(v8::Isolate* isolate, mate::Arguments* args); - mate::Handle Resize(v8::Isolate* isolate, + mate::Handle Resize(mate::Arguments* args, const base::DictionaryValue& options); mate::Handle Crop(v8::Isolate* isolate, const gfx::Rect& rect); std::string ToDataURL(mate::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 mate::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 bc5ee6cfbe90f..d74c19922c57b 100644 --- a/spec-main/api-remote-spec.ts +++ b/spec-main/api-remote-spec.ts @@ -2,7 +2,8 @@ import * as path from 'path' import { expect } from 'chai' import { closeWindow } from './window-helpers' -import { ipcMain, BrowserWindow } from 'electron' +import { ipcMain, BrowserWindow, nativeImage } from 'electron' +import { serialize, deserialize } from '../lib/common/type-utils'; describe('remote module', () => { const fixtures = path.join(__dirname, 'fixtures') @@ -33,6 +34,119 @@ describe('remote module', () => { return result } + describe('typeUtils serialization/deserialization', () => { + it('serializes and deserializes an empty NativeImage', () => { + const image = nativeImage.createEmpty(); + const serializedImage = serialize(image); + const empty = deserialize(serializedImage); + + expect(empty.isEmpty()).to.be.true(); + expect(empty.getAspectRatio()).to.equal(1); + expect(empty.toDataURL()).to.equal('data:image/png;base64,'); + expect(empty.toDataURL({ scaleFactor: 2.0 })).to.equal('data:image/png;base64,'); + expect(empty.getSize()).to.deep.equal({ width: 0, height: 0 }); + expect(empty.getBitmap()).to.be.empty(); + expect(empty.getBitmap({ scaleFactor: 2.0 })).to.be.empty(); + expect(empty.toBitmap()).to.be.empty(); + expect(empty.toBitmap({ scaleFactor: 2.0 })).to.be.empty(); + expect(empty.toJPEG(100)).to.be.empty(); + expect(empty.toPNG()).to.be.empty(); + expect(empty.toPNG({ scaleFactor: 2.0 })).to.be.empty(); + }); + + it('serializes and deserializes a non-empty NativeImage', () => { + const dataURL = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAIAAAACCAIAAAD91JpzAAAAFklEQVQYlWP8//8/AwMDEwMDAwMDAwAkBgMBBMzldwAAAABJRU5ErkJggg=='; + const image = nativeImage.createFromDataURL(dataURL); + const serializedImage = serialize(image); + const nonEmpty = deserialize(serializedImage); + + expect(nonEmpty.isEmpty()).to.be.false(); + expect(nonEmpty.getAspectRatio()).to.equal(1); + expect(nonEmpty.toDataURL()).to.not.be.empty(); + expect(nonEmpty.toDataURL({ scaleFactor: 1.0 })).to.equal(dataURL); + expect(nonEmpty.getSize()).to.deep.equal({ width: 2, height: 2 }); + expect(nonEmpty.getBitmap()).to.not.be.empty(); + expect(nonEmpty.toPNG()).to.not.be.empty(); + }); + + it('serializes and deserializes a non-empty NativeImage with multiple representations', () => { + const image = nativeImage.createEmpty(); + + const dataURL1 = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVQYlWNgAAIAAAUAAdafFs0AAAAASUVORK5CYII='; + image.addRepresentation({ scaleFactor: 1.0, dataURL: dataURL1 }); + + const dataURL2 = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAIAAAACCAIAAAD91JpzAAAAFklEQVQYlWP8//8/AwMDEwMDAwMDAwAkBgMBBMzldwAAAABJRU5ErkJggg=='; + image.addRepresentation({ scaleFactor: 2.0, dataURL: dataURL2 }); + + const serializedImage = serialize(image); + const nonEmpty = deserialize(serializedImage); + + expect(nonEmpty.isEmpty()).to.be.false(); + expect(nonEmpty.getAspectRatio()).to.equal(1); + expect(nonEmpty.getSize()).to.deep.equal({ width: 1, height: 1 }); + expect(nonEmpty.getBitmap()).to.not.be.empty(); + expect(nonEmpty.getBitmap({ scaleFactor: 1.0 })).to.not.be.empty(); + expect(nonEmpty.getBitmap({ scaleFactor: 2.0 })).to.not.be.empty(); + expect(nonEmpty.toBitmap()).to.not.be.empty(); + expect(nonEmpty.toBitmap({ scaleFactor: 1.0 })).to.not.be.empty(); + expect(nonEmpty.toBitmap({ scaleFactor: 2.0 })).to.not.be.empty(); + expect(nonEmpty.toPNG()).to.not.be.empty(); + expect(nonEmpty.toPNG({ scaleFactor: 1.0 })).to.not.be.empty(); + expect(nonEmpty.toPNG({ scaleFactor: 2.0 })).to.not.be.empty(); + expect(nonEmpty.toDataURL()).to.not.be.empty(); + expect(nonEmpty.toDataURL({ scaleFactor: 1.0 })).to.equal(dataURL1); + expect(nonEmpty.toDataURL({ scaleFactor: 2.0 })).to.equal(dataURL2); + }); + + it('serializes and deserializes an Array', () => { + const array = [1, 2, 3, 4, 5]; + const serialized = serialize(array); + const deserialized = deserialize(serialized); + + expect(deserialized).to.deep.equal(array); + }); + + it('serializes and deserializes a Buffer', () => { + const buffer = Buffer.from('hello world!', 'utf-8'); + const serialized = serialize(buffer); + const deserialized = deserialize(serialized); + + expect(deserialized).to.deep.equal(buffer); + }); + + it('serializes and deserializes a Boolean', () => { + const bool = true; + const serialized = serialize(bool); + const deserialized = deserialize(serialized); + + expect(deserialized).to.equal(bool); + }); + + it('serializes and deserializes a Number', () => { + const number = 42; + const serialized = serialize(number); + const deserialized = deserialize(serialized); + + expect(deserialized).to.equal(number); + }); + + it('serializes and deserializes a String', () => { + const str = 'hello world'; + const serialized = serialize(str); + const deserialized = deserialize(serialized); + + expect(deserialized).to.equal(str); + }); + + it('serializes and deserializes a simple Object', () => { + const obj = { hello: 'world', 'answer-to-everything': 42 }; + const serialized = serialize(obj); + const deserialized = deserialize(serialized); + + expect(deserialized).to.deep.equal(obj); + }); + }); + describe('remote.getGlobal filtering', () => { it('can return custom values', async () => { w.webContents.once('remote-get-global', (event, name) => { diff --git a/spec-main/package.json b/spec-main/package.json index e9130f7f1e657..f0e9f2521aa80 100644 --- a/spec-main/package.json +++ b/spec-main/package.json @@ -4,6 +4,7 @@ "main": "index.js", "version": "0.1.0", "devDependencies": { + "@types/dirty-chai": "^2.0.0", "@types/ws": "^7.2.0", "ws": "^7.2.1" } diff --git a/spec-main/yarn.lock b/spec-main/yarn.lock index b5e628fe1b0be..48f69ec66be72 100644 --- a/spec-main/yarn.lock +++ b/spec-main/yarn.lock @@ -2,6 +2,26 @@ # yarn lockfile v1 +"@types/chai-as-promised@*": + version "7.1.2" + resolved "https://registry.yarnpkg.com/@types/chai-as-promised/-/chai-as-promised-7.1.2.tgz#2f564420e81eaf8650169e5a3a6b93e096e5068b" + integrity sha512-PO2gcfR3Oxa+u0QvECLe1xKXOqYTzCmWf0FhLhjREoW3fPAVamjihL7v1MOVLJLsnAMdLcjkfrs01yvDMwVK4Q== + dependencies: + "@types/chai" "*" + +"@types/chai@*": + version "4.2.11" + resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.2.11.tgz#d3614d6c5f500142358e6ed24e1bf16657536c50" + integrity sha512-t7uW6eFafjO+qJ3BIV2gGUyZs27egcNRkUdalkud+Qa3+kg//f129iuOFivHDXQ+vnU3fDXuwgv0cqMCbcE8sw== + +"@types/dirty-chai@^2.0.0": + version "2.0.2" + resolved "https://registry.yarnpkg.com/@types/dirty-chai/-/dirty-chai-2.0.2.tgz#eeac4802329a41ed7815ac0c1a6360335bf77d0c" + integrity sha512-BruwIN/UQEU0ePghxEX+OyjngpOfOUKJQh3cmfeq2h2Su/g001iljVi3+Y2y2EFp3IPgjf4sMrRU33Hxv1FUqw== + dependencies: + "@types/chai" "*" + "@types/chai-as-promised" "*" + "@types/node@*": version "13.7.0" resolved "https://registry.yarnpkg.com/@types/node/-/node-13.7.0.tgz#b417deda18cf8400f278733499ad5547ed1abec4"