From ac61c74ddc2ce25d4d89a040771acd3f4bc9e585 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Nov 2024 16:04:18 -0600 Subject: [PATCH] fix: bugprone-narrowing-conversions warnings in NativeImage (#44708) * fix: bugprone-narrowing-conversions warning in NativeImage::memory_usage_ - fix signed / unsigned math by using base/numerics/safe_conversions - make memory_usage_ an int64_t so it can safely take the size_t returned by computeByteSize() Warning fixed by this commit: ../../electron/shell/common/api/electron_api_native_image.cc:155:26: warning: narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int32_t' (aka 'int') is implementation-defined [bugprone-narrowing-conversions] 155 | new_memory_usage = image_skia->bitmap()->computeByteSize(); * fix: bugprone-narrowing-conversions warnings in NativeImage::CreateFromBitmap() `SkImageInfo::MakeN32()` and `SkBitmap::allocN32Pixels()` both take int width and height args, but we were feeding them unsigned ints. ../../electron/shell/common/api/electron_api_native_image.cc:508:36: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions] 508 | auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType); | ^ ../../electron/shell/common/api/electron_api_native_image.cc:508:43: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions] 508 | auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType); | ^ ../../electron/shell/common/api/electron_api_native_image.cc:524:25: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions] 524 | bitmap.allocN32Pixels(width, height, false); | ^ ../../electron/shell/common/api/electron_api_native_image.cc:524:32: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions] 524 | bitmap.allocN32Pixels(width, height, false); | ^ ../../electron/shell/common/api/electron_api_native_image.cc:528:48: warning: narrowing conversion from 'double' to 'float' [bugprone-narrowing-conversions] 528 | gfx::ImageSkia::CreateFromBitmap(bitmap, scale_factor); --- shell/common/api/electron_api_native_image.cc | 25 +++++++++---------- shell/common/api/electron_api_native_image.h | 2 +- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/shell/common/api/electron_api_native_image.cc b/shell/common/api/electron_api_native_image.cc index ce3fa8a93e10..98b2a14bc29c 100644 --- a/shell/common/api/electron_api_native_image.cc +++ b/shell/common/api/electron_api_native_image.cc @@ -12,6 +12,7 @@ #include "base/files/file_util.h" #include "base/logging.h" #include "base/memory/ref_counted_memory.h" +#include "base/numerics/safe_conversions.h" #include "base/strings/pattern.h" #include "base/strings/utf_string_conversions.h" #include "gin/arguments.h" @@ -147,13 +148,13 @@ NativeImage::~NativeImage() { } void NativeImage::UpdateExternalAllocatedMemoryUsage() { - int32_t new_memory_usage = 0; + int64_t new_memory_usage = 0; if (image_.HasRepresentation(gfx::Image::kImageRepSkia)) { auto* const image_skia = image_.ToImageSkia(); - if (!image_skia->isNull()) { - new_memory_usage = image_skia->bitmap()->computeByteSize(); - } + if (!image_skia->isNull()) + new_memory_usage = + base::as_signed(image_skia->bitmap()->computeByteSize()); } isolate_->AdjustAmountOfExternalAllocatedMemory(new_memory_usage - @@ -491,9 +492,8 @@ gin::Handle NativeImage::CreateFromBitmap( return gin::Handle(); } - unsigned int width = 0; - unsigned int height = 0; - double scale_factor = 1.; + int width = 0; + int height = 0; if (!options.Get("width", &width)) { thrower.ThrowError("width is required"); @@ -505,6 +505,9 @@ gin::Handle NativeImage::CreateFromBitmap( return gin::Handle(); } + if (width <= 0 || height <= 0) + return CreateEmpty(thrower.isolate()); + auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType); auto size_bytes = info.computeMinByteSize(); @@ -514,16 +517,12 @@ gin::Handle NativeImage::CreateFromBitmap( return gin::Handle(); } - options.Get("scaleFactor", &scale_factor); - - if (width == 0 || height == 0) { - return CreateEmpty(thrower.isolate()); - } - SkBitmap bitmap; bitmap.allocN32Pixels(width, height, false); bitmap.writePixels({info, buffer_data.data(), bitmap.rowBytes()}); + float scale_factor = 1.0F; + options.Get("scaleFactor", &scale_factor); gfx::ImageSkia image_skia = gfx::ImageSkia::CreateFromBitmap(bitmap, scale_factor); diff --git a/shell/common/api/electron_api_native_image.h b/shell/common/api/electron_api_native_image.h index ab8929a153f2..a840c51445a0 100644 --- a/shell/common/api/electron_api_native_image.h +++ b/shell/common/api/electron_api_native_image.h @@ -140,7 +140,7 @@ class NativeImage final : public gin::Wrappable { gfx::Image image_; raw_ptr isolate_; - int32_t memory_usage_ = 0; + int64_t memory_usage_ = 0; }; } // namespace electron::api