Browse Source

fix: nativeImage remote serialization (#24021)

Co-authored-by: Shelley Vohr <[email protected]>
Charles Kerr 4 years ago
parent
commit
4fb7b33b2b

+ 9 - 2
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()) {

+ 47 - 6
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) {

+ 6 - 1
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),

+ 13 - 0
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 <typename T>
+bool GetNextArgument(Arguments* args,
+                     int create_flags,
+                     bool is_first,
+                     base::Optional<T>* 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,

+ 22 - 10
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<float> 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<float> NativeImage::GetScaleFactors() {
+  gfx::ImageSkia image_skia = image_.AsImageSkia();
+  return image_skia.GetSupportedScales();
+}
+
+float NativeImage::GetAspectRatio(const base::Optional<float> 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> 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> 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> 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> 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)

+ 5 - 3
shell/common/api/atom_api_native_image.h

@@ -7,6 +7,7 @@
 
 #include <map>
 #include <string>
+#include <vector>
 
 #include "base/values.h"
 #include "native_mate/dictionary.h"
@@ -84,16 +85,17 @@ class NativeImage : public mate::Wrappable<NativeImage> {
   v8::Local<v8::Value> ToPNG(mate::Arguments* args);
   v8::Local<v8::Value> ToJPEG(v8::Isolate* isolate, int quality);
   v8::Local<v8::Value> ToBitmap(mate::Arguments* args);
+  std::vector<float> GetScaleFactors();
   v8::Local<v8::Value> GetBitmap(mate::Arguments* args);
   v8::Local<v8::Value> GetNativeHandle(v8::Isolate* isolate,
                                        mate::Arguments* args);
-  mate::Handle<NativeImage> Resize(v8::Isolate* isolate,
+  mate::Handle<NativeImage> Resize(mate::Arguments* args,
                                    const base::DictionaryValue& options);
   mate::Handle<NativeImage> 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<float> scale_factor);
+  float GetAspectRatio(const base::Optional<float> scale_factor);
   void AddRepresentation(const mate::Dictionary& options);
 
   // Mark the image as template image.

+ 115 - 1
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) => {

+ 1 - 0
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"
   }

+ 20 - 0
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"