fix: nativeImage remote serialization (#23797)

* 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.

* refactor: correctly serialize nativeImage/buffer with typeUtils (#23666)

* refactor: correctly serialize nativeImage/buffer with typeUtils

* test: add serialization specs

* fix: construct from dataURL

* test: test for dataURL specificity

* refactor: use typeutils for nativeImage serialization

* fix: ensure nativeImage serialization main->renderer

* chore: fix FTBFS

Co-authored-by: Charles Kerr <[email protected]>

lib/browser/remote/server.ts

@@ -5,11 +5,12 @@ import { EventEmitter } from 'events';
 import objectsRegistry from './objects-registry';
 import { ipcMainInternal } from '../ipc-main-internal';
 import * as guestViewManager from '@electron/internal/browser/guest-view-manager';
-import { isPromise, isSerializableObject } from '@electron/internal/common/type-utils';
+import { isPromise, isSerializableObject, deserialize, serialize } from '@electron/internal/common/type-utils';
 
 const v8Util = process.electronBinding('v8_util');
 const eventBinding = process.electronBinding('event');
 const features = process.electronBinding('features');
+const { NativeImage } = process.electronBinding('native_image');
 
 if (!features.isRemoteModuleEnabled()) {
   throw new Error('remote module is disabled');
@@ -114,6 +115,9 @@ type MetaType = {
 } | {
   type: 'promise',
   then: MetaType
+} | {
+  type: 'nativeimage'
+  value: electron.NativeImage
 }
 
 // Convert a real value into meta data.
@@ -124,6 +128,8 @@ const valueToMeta = function (sender: electron.WebContents, contextId: string, v
     // Recognize certain types of objects.
     if (value instanceof Buffer) {
       type = 'buffer';
+    } else if (value instanceof NativeImage) {
+      type = 'nativeimage';
     } else if (Array.isArray(value)) {
       type = 'array';
     } else if (value instanceof Error) {
@@ -147,6 +153,8 @@ const valueToMeta = function (sender: electron.WebContents, contextId: string, v
       type,
       members: value.map((el: any) => valueToMeta(sender, contextId, el, optimizeSimpleObject))
     };
+  } else if (type === 'nativeimage') {
+    return { type, value: serialize(value) };
   } else if (type === 'object' || type === 'function') {
     return {
       type,
@@ -234,7 +242,10 @@ type MetaTypeFromRenderer = {
 } | {
   type: 'object',
   name: string,
-  members: { name: string, value: MetaTypeFromRenderer }[]
+  members: {
+    name: string,
+    value: MetaTypeFromRenderer
+  }[]
 } | {
   type: 'function-with-return-value',
   value: MetaTypeFromRenderer
@@ -243,6 +254,14 @@ type MetaTypeFromRenderer = {
   id: number,
   location: string,
   length: number
+} | {
+  type: 'nativeimage',
+  value: {
+    size: electron.Size,
+    buffer: Buffer,
+    scaleFactor: number,
+    dataURL: string
+  }[]
 }
 
 const fakeConstructor = (constructor: Function, name: string) =>
@@ -260,6 +279,8 @@ 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':
+        return deserialize(meta.value);
       case 'value':
         return meta.value;
       case 'remote-object':

lib/common/type-utils.ts

@@ -23,6 +23,7 @@ const serializableTypes = [
   ArrayBuffer
 ];
 
+// https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm#Supported_types
 export function isSerializableObject (value: any) {
   return value === null || ArrayBuffer.isView(value) || serializableTypes.some(type => value instanceof type);
 }
@@ -33,14 +34,55 @@ 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
-    };
-  } else if (Array.isArray(value)) {
+    return serializeNativeImage(value);
+  } if (Array.isArray(value)) {
     return value.map(serialize);
   } else if (isSerializableObject(value)) {
     return value;
@@ -53,7 +95,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 (isSerializableObject(value)) {

lib/renderer/api/remote.js

@@ -2,9 +2,10 @@
 
 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');
+const { isPromise, isSerializableObject, serialize, deserialize } = require('@electron/internal/common/type-utils');
 const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal');
 
 const callbacksRegistry = new CallbacksRegistry();
@@ -33,7 +34,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',
@@ -213,6 +216,7 @@ function metaToValue (meta) {
   const types = {
     value: () => meta.value,
     array: () => meta.members.map((member) => metaToValue(member)),
+    nativeimage: () => deserialize(meta.value),
     buffer: () => Buffer.from(meta.value.buffer, meta.value.byteOffset, meta.value.byteLength),
     promise: () => Promise.resolve({ then: metaToValue(meta.then) }),
     error: () => metaToError(meta),

shell/common/api/electron_api_native_image.cc

@@ -247,22 +247,32 @@ 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());
+}
+
+std::vector<float> NativeImage::GetScaleFactors() {
+  gfx::ImageSkia image_skia = image_.AsImageSkia();
+  return image_skia.GetSupportedScales();
 }
 
-float NativeImage::GetAspectRatio() {
-  gfx::Size size = GetSize();
+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
     return static_cast<float>(size.width()) / static_cast<float>(size.height());
 }
 
-gin::Handle<NativeImage> NativeImage::Resize(
-    v8::Isolate* isolate,
-    const base::DictionaryValue& options) {
-  gfx::Size size = GetSize();
+gin::Handle<NativeImage> NativeImage::Resize(gin::Arguments* args,
+                                             base::DictionaryValue options) {
+  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 +282,12 @@ gin::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 =
@@ -290,8 +301,8 @@ gin::Handle<NativeImage> NativeImage::Resize(
 
   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> NativeImage::Crop(v8::Isolate* isolate,
@@ -506,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)

shell/common/api/electron_api_native_image.h

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

shell/common/gin_helper/function_template.h

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/optional.h"
 #include "gin/arguments.h"
 #include "shell/common/gin_helper/arguments.h"
 #include "shell/common/gin_helper/destroyable.h"
@@ -92,6 +93,19 @@ bool GetNextArgument(gin::Arguments* args,
   }
 }
 
+// Support base::Optional as an argument.
+template <typename T>
+bool GetNextArgument(gin::Arguments* args,
+                     int create_flags,
+                     bool is_first,
+                     base::Optional<T>* result) {
+  T converted;
+  // Use gin::Arguments::GetNext which always advances |next| counter.
+  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(gin::Arguments* args,

spec-main/api-remote-spec.ts

@@ -3,7 +3,8 @@ import { expect } from 'chai'
 import { closeWindow } from './window-helpers'
 import { ifdescribe } from './spec-helpers';
 
-import { ipcMain, BrowserWindow } from 'electron'
+import { ipcMain, BrowserWindow, nativeImage } from 'electron'
+import { serialize, deserialize } from '../lib/common/type-utils';
 
 const features = process.electronBinding('features')
 
@@ -36,6 +37,143 @@ ifdescribe(features.isRemoteModuleEnabled())('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 Date', () => {
+      const date = new Date();
+      const serialized = serialize(date);
+      const deserialized = deserialize(serialized);
+
+      expect(deserialized).to.equal(date);
+    });
+
+    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 Regexp', () => {
+      const regex = new RegExp('ab+c');
+      const serialized = serialize(regex);
+      const deserialized = deserialize(serialized);
+
+      expect(deserialized).to.equal(regex);
+    });
+
+    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 an Error', () => {
+      const err = new Error('oh crap');
+      const serialized = serialize(err);
+      const deserialized = deserialize(serialized);
+
+      expect(deserialized).to.equal(err);
+    });
+
+    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) => {