refactor: remove redundant NativeImage::GetBitmap() (#46736)

* refactor: remove redundant NativeImage::GetBitmap()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* docs: mark NativeImage.getBitmap() as deprecated

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* have getBitmap() emit a deprecation warning

Co-authored-by: David Sanders <dsanders11@ucsbalum.com>

* docs: update obsolete refefence to getBitmap()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* test: update obsolete refefences to getBitmap()

Co-authored-by: Charles Kerr <charles@charleskerr.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
Co-authored-by: David Sanders <dsanders11@ucsbalum.com>

docs/api/native-image.md

@@ -271,16 +271,12 @@ changes:
 
 Returns `string` - The [Data URL][data-url] of the image.
 
-#### `image.getBitmap([options])`
+#### `image.getBitmap([options])` _Deprecated_
 
 * `options` Object (optional)
   * `scaleFactor` Number (optional) - Defaults to 1.0.
 
-Returns `Buffer` - A [Buffer][buffer] that contains the image's raw bitmap pixel data.
+Legacy alias for `image.toBitmap()`.
-
-The difference between `getBitmap()` and `toBitmap()` is that `getBitmap()` does not
-copy the bitmap data, so you have to use the returned Buffer immediately in
-current event loop tick; otherwise the data might be changed or destroyed.
 
 #### `image.getNativeHandle()` _macOS_
 

docs/api/web-contents.md

@@ -887,7 +887,7 @@ const { BrowserWindow } = require('electron')
 
 const win = new BrowserWindow({ webPreferences: { offscreen: true } })
 win.webContents.on('paint', (event, dirty, image) => {
-  // updateBitmap(dirty, image.getBitmap())
+  // updateBitmap(dirty, image.toBitmap())
 })
 win.loadURL('https://github.com')
 ```

docs/breaking-changes.md

@@ -27,6 +27,19 @@ window is not currently visible.
 
 If you were using `app.commandLine` to control the behavior of the  main process, you should do this via `process.argv`.
 
+### Deprecated: `NativeImage.getBitmap()`
+
+`NativeImage.toBitmap()` returns a newly-allocated copy of the bitmap. `NativeImage.getBitmap()` was originally an alternative function that returned the original instead of a copy. This changed when sandboxing was introduced, so both return a copy and are functionally equivalent.
+
+Client code should call `NativeImage.toBitmap()` instead:
+
+```js
+// Deprecated
+bitmap = image.getBitmap()
+// Use this instead
+bitmap = image.toBitmap()
+```
+
 ## Planned Breaking API Changes (36.0)
 
 ### Removed:`isDefault` and `status` properties on `PrinterInfo`

shell/common/api/electron_api_native_image.cc

@@ -287,17 +287,16 @@ std::string NativeImage::ToDataURL(gin::Arguments* args) {
 }
 
 v8::Local<v8::Value> NativeImage::GetBitmap(gin::Arguments* args) {
-  float scale_factor = GetScaleFactorFromOptions(args);
+  static bool deprecated_warning_issued = false;
 
-  const SkBitmap bitmap =
+  if (!deprecated_warning_issued) {
-      image_.AsImageSkia().GetRepresentation(scale_factor).GetBitmap();
+    deprecated_warning_issued = true;
-  SkPixelRef* ref = bitmap.pixelRef();
+    util::EmitWarning(isolate_,
-  if (!ref)
+                      "getBitmap() is deprecated, use toBitmap() instead.",
-    return node::Buffer::New(args->isolate(), 0).ToLocalChecked();
+                      "DeprecationWarning");
-  return node::Buffer::Copy(args->isolate(),
+  }
-                            reinterpret_cast<char*>(ref->pixels()),
+
-                            bitmap.computeByteSize())
+  return ToBitmap(args);
-      .ToLocalChecked();
 }
 
 v8::Local<v8::Value> NativeImage::GetNativeHandle(

spec/api-browser-window-spec.ts

@@ -4599,7 +4599,7 @@ describe('BrowserWindow module', () => {
 
           try {
             const expectedSize = rect.width * rect.height * 4;
-            expect(image.getBitmap()).to.be.an.instanceOf(Buffer).with.lengthOf(expectedSize);
+            expect(image.toBitmap()).to.be.an.instanceOf(Buffer).with.lengthOf(expectedSize);
             done();
           } catch (e) {
             done(e);

spec/api-native-image-spec.ts

@@ -4,6 +4,7 @@ import { expect } from 'chai';
 
 import * as path from 'node:path';
 
+import { expectDeprecationMessages } from './lib/deprecate-helpers';
 import { ifdescribe, ifit, itremote, useRemoteContext } from './lib/spec-helpers';
 
 describe('nativeImage module', () => {
@@ -78,15 +79,20 @@ describe('nativeImage module', () => {
   });
 
   describe('createEmpty()', () => {
-    it('returns an empty image', () => {
+    it('returns an empty image', async () => {
       const empty = nativeImage.createEmpty();
       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();
+      await expectDeprecationMessages(
-      expect(empty.getBitmap({ scaleFactor: 2.0 })).to.be.empty();
+        () => {
+          expect(empty.getBitmap()).to.be.empty();
+          expect(empty.getBitmap({ scaleFactor: 2.0 })).to.be.empty();
+        },
+        'getBitmap() is deprecated, use toBitmap() instead.'
+      );
       expect(empty.toBitmap()).to.be.empty();
       expect(empty.toBitmap({ scaleFactor: 2.0 })).to.be.empty();
       expect(empty.toJPEG(100)).to.be.empty();

spec/ts-smoke/electron/main.ts

@@ -1291,7 +1291,7 @@ const win4 = new BrowserWindow({
 });
 
 win4.webContents.on('paint', (event, dirty, _image) => {
-  console.log(dirty, _image.getBitmap());
+  console.log(dirty, _image.toBitmap());
 });
 
 win4.webContents.on('devtools-open-url', (event, url) => {