refactor: createThumbnailFromPath takes size not maxSize (#37362)

refactor: createThumbnailFromPath takes size not maxSize

docs/api/native-image.md

@@ -119,13 +119,15 @@ Returns `NativeImage`
 
 Creates an empty `NativeImage` instance.
 
-### `nativeImage.createThumbnailFromPath(path, maxSize)` _macOS_ _Windows_
+### `nativeImage.createThumbnailFromPath(path, size)` _macOS_ _Windows_
 
 * `path` string - path to a file that we intend to construct a thumbnail out of.
-* `maxSize` [Size](structures/size.md) - the maximum width and height (positive numbers) the thumbnail returned can be. The Windows implementation will ignore `maxSize.height` and scale the height according to `maxSize.width`.
+* `size` [Size](structures/size.md) - the desired width and height (positive numbers) of the thumbnail.
 
 Returns `Promise<NativeImage>` - fulfilled with the file's thumbnail preview image, which is a [NativeImage](native-image.md).
 
+Note: The Windows implementation will ignore `size.height` and scale the height according to `size.width`.
+
 ### `nativeImage.createFromPath(path)`
 
 * `path` string

docs/breaking-changes.md

@@ -14,6 +14,41 @@ This document uses the following convention to categorize breaking changes:
 
 ## Planned Breaking API Changes (24.0)
 
+### API Changed: `nativeImage.createThumbnailFromPath(path, size)`
+
+The `maxSize` parameter has been changed to `size` to reflect that the size passed in will be the size the thumbnail created. Previously, Windows would not scale the image up if it were smaller than `maxSize`, and
+macOS would always set the size to `maxSize`. Behavior is now the same across platforms.
+
+Updated Behavior:
+
+```js
+// a 128x128 image.
+const imagePath = path.join('path', 'to', 'capybara.png')
+
+// Scaling up a smaller image.
+const upSize = { width: 256, height: 256 }
+nativeImage.createThumbnailFromPath(imagePath, upSize).then(result => {
+  console.log(result.getSize()) // { width: 256, height: 256 }
+})
+
+// Scaling down a larger image.
+const downSize = { width: 64, height: 64 }
+nativeImage.createThumbnailFromPath(imagePath, downSize).then(result => {
+  console.log(result.getSize()) // { width: 64, height: 64 }
+})
+```
+
+Previous Behavior (on Windows):
+
+```js
+// a 128x128 image
+const imagePath = path.join('path', 'to', 'capybara.png')
+const size = { width: 256, height: 256 }
+nativeImage.createThumbnailFromPath(imagePath, size).then(result => {
+  console.log(result.getSize()) // { width: 128, height: 128 }
+})
+```
+
 ### Deprecated: `BrowserWindow.setTrafficLightPosition(position)`
 
 `BrowserWindow.setTrafficLightPosition(position)` has been deprecated, the

shell/common/api/electron_api_native_image_win.cc

@@ -43,7 +43,7 @@ v8::Local<v8::Promise> NativeImage::CreateThumbnailFromPath(
 
   if (FAILED(hr)) {
     promise.RejectWithErrorMessage(
-        "failed to create IShellItem from the given path");
+        "Failed to create IShellItem from the given path");
     return handle;
   }
 
@@ -53,21 +53,20 @@ v8::Local<v8::Promise> NativeImage::CreateThumbnailFromPath(
                         IID_PPV_ARGS(&pThumbnailCache));
   if (FAILED(hr)) {
     promise.RejectWithErrorMessage(
-        "failed to acquire local thumbnail cache reference");
+        "Failed to acquire local thumbnail cache reference");
     return handle;
   }
 
   // Populate the IShellBitmap
   Microsoft::WRL::ComPtr<ISharedBitmap> pThumbnail;
-  WTS_CACHEFLAGS flags;
+  hr = pThumbnailCache->GetThumbnail(
-  WTS_THUMBNAILID thumbId;
+      pItem.Get(), size.width(),
-  hr = pThumbnailCache->GetThumbnail(pItem.Get(), size.width(),
+      WTS_FLAGS::WTS_SCALETOREQUESTEDSIZE | WTS_FLAGS::WTS_SCALEUP, &pThumbnail,
-                                     WTS_FLAGS::WTS_NONE, &pThumbnail, &flags,
+      NULL, NULL);
-                                     &thumbId);
 
   if (FAILED(hr)) {
     promise.RejectWithErrorMessage(
-        "failed to get thumbnail from local thumbnail cache reference");
+        "Failed to get thumbnail from local thumbnail cache reference");
     return handle;
   }
 
@@ -75,14 +74,14 @@ v8::Local<v8::Promise> NativeImage::CreateThumbnailFromPath(
   HBITMAP hBitmap = NULL;
   hr = pThumbnail->GetSharedBitmap(&hBitmap);
   if (FAILED(hr)) {
-    promise.RejectWithErrorMessage("failed to extract bitmap from thumbnail");
+    promise.RejectWithErrorMessage("Failed to extract bitmap from thumbnail");
     return handle;
   }
 
   // convert HBITMAP to gfx::Image
   BITMAP bitmap;
   if (!GetObject(hBitmap, sizeof(bitmap), &bitmap)) {
-    promise.RejectWithErrorMessage("could not convert HBITMAP to BITMAP");
+    promise.RejectWithErrorMessage("Could not convert HBITMAP to BITMAP");
     return handle;
   }
 

spec/api-native-image-spec.ts

@@ -447,6 +447,30 @@ describe('nativeImage module', () => {
       const result = await nativeImage.createThumbnailFromPath(goodPath, goodSize);
       expect(result.isEmpty()).to.equal(false);
     });
+
+    it('returns the correct size if larger than the initial image', async () => {
+      // capybara.png is a 128x128 image.
+      const imgPath = path.join(fixturesPath, 'assets', 'capybara.png');
+      const size = { width: 256, height: 256 };
+      const result = await nativeImage.createThumbnailFromPath(imgPath, size);
+      expect(result.getSize()).to.deep.equal(size);
+    });
+
+    it('returns the correct size if is the same as the initial image', async () => {
+      // capybara.png is a 128x128 image.
+      const imgPath = path.join(fixturesPath, 'assets', 'capybara.png');
+      const size = { width: 128, height: 128 };
+      const result = await nativeImage.createThumbnailFromPath(imgPath, size);
+      expect(result.getSize()).to.deep.equal(size);
+    });
+
+    it('returns the correct size if smaller than the initial image', async () => {
+      // capybara.png is a 128x128 image.
+      const imgPath = path.join(fixturesPath, 'assets', 'capybara.png');
+      const maxSize = { width: 64, height: 64 };
+      const result = await nativeImage.createThumbnailFromPath(imgPath, maxSize);
+      expect(result.getSize()).to.deep.equal(maxSize);
+    });
   });
 
   describe('addRepresentation()', () => {