mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: spanify image utils (#44178)

Browse source 
* refactor: electron::util::AddImageSkiaRepFromJPEG() takes a span arg

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

* refactor: electron::util::AddImageSkiaRepFromPNG() takes a span arg

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

* refactor: electron::util::AddImageSkiaRepFromBuffer() takes a span arg

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

* feat: add Node-buffer-to-base-span helper function

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

* refactor: electron::api::NativeImage::CreateFromPNG() now takes a span param

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

* refactor: electron::api::NativeImage::CreateFromJPEG() now takes a span param

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

* refactor: use base::as_byte_span()

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

* fix: -Wunsafe-buffer-usage warning in NativeImage::CreateFromNamedImage()

Warning fixed by this commit:

../../electron/shell/common/api/electron_api_native_image_mac.mm:131:11: error: function introduces unsafe buffer manipulation [-Werror,-Wunsafe-buffer-usage]
  131 |           {reinterpret_cast<const uint8_t*>((char*)[png_data bytes]),
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  132 |            [png_data length]});
      |            ~~~~~~~~~~~~~~~~~~
../../electron/shell/common/api/electron_api_native_image_mac.mm:131:11: note: See //docs/unsafe_buffers.md for help.

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

* chore: add // SAFETY comment for Node-buffer-to-span func

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

* chore: add // SAFETY comment for NSData-to-span func

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>
This commit is contained in:
trop[bot] 2024-10-10 13:24:26 -04:00 committed by GitHub
parent 5bb1bb9b7a
commit 6964b79e68
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 77 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
shell/common/api/electron_api_native_image.cc
View file

@ -30,6 +30,7 @@
#include "shell/common/gin_helper/function_template_extensions.h"
#include "shell/common/gin_helper/object_template_builder.h"
#include "shell/common/node_includes.h"
#include "shell/common/node_util.h"
#include "shell/common/process_util.h"
#include "shell/common/skia_util.h"
#include "shell/common/thread_restrictions.h"
@ -397,20 +398,18 @@ void NativeImage::AddRepresentation(const gin_helper::Dictionary& options) {
v8::Local<v8::Value> buffer;
GURL url;
if (options.Get("buffer", &buffer) && node::Buffer::HasInstance(buffer)) {
auto* data = reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer));
auto size = node::Buffer::Length(buffer);
skia_rep_added = electron::util::AddImageSkiaRepFromBuffer(
&image_skia, data, size, width, height, scale_factor);
&image_skia, electron::util::as_byte_span(buffer), width, height,
scale_factor);
} else if (options.Get("dataURL", &url)) {
std::string mime_type, charset, data;
if (net::DataURL::Parse(url, &mime_type, &charset, &data)) {
auto* data_ptr = reinterpret_cast<const unsigned char*>(data.c_str());
if (mime_type == "image/png") {
skia_rep_added = electron::util::AddImageSkiaRepFromPNG(
&image_skia, data_ptr, data.size(), scale_factor);
&image_skia, base::as_byte_span(data), scale_factor);
} else if (mime_type == "image/jpeg") {
skia_rep_added = electron::util::AddImageSkiaRepFromJPEG(
&image_skia, data_ptr, data.size(), scale_factor);
&image_skia, base::as_byte_span(data), scale_factor);
}
}
}
@ -442,22 +441,20 @@ gin::Handle<NativeImage> NativeImage::Create(v8::Isolate* isolate,
}
// static
gin::Handle<NativeImage> NativeImage::CreateFromPNG(v8::Isolate* isolate,
const char* buffer,
size_t length) {
gin::Handle<NativeImage> NativeImage::CreateFromPNG(
v8::Isolate* isolate,
const base::span<const uint8_t> data) {
gfx::ImageSkia image_skia;
electron::util::AddImageSkiaRepFromPNG(
&image_skia, reinterpret_cast<const unsigned char*>(buffer), length, 1.0);
electron::util::AddImageSkiaRepFromPNG(&image_skia, data, 1.0);
return Create(isolate, gfx::Image(image_skia));
}
// static
gin::Handle<NativeImage> NativeImage::CreateFromJPEG(v8::Isolate* isolate,
const char* buffer,
size_t length) {
gin::Handle<NativeImage> NativeImage::CreateFromJPEG(
v8::Isolate* isolate,
const base::span<const uint8_t> buffer) {
gfx::ImageSkia image_skia;
electron::util::AddImageSkiaRepFromJPEG(
&image_skia, reinterpret_cast<const unsigned char*>(buffer), length, 1.0);
electron::util::AddImageSkiaRepFromJPEG(&image_skia, buffer, 1.0);
return Create(isolate, gfx::Image(image_skia));
}
@ -509,7 +506,8 @@ gin::Handle<NativeImage> NativeImage::CreateFromBitmap(
auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType);
auto size_bytes = info.computeMinByteSize();
if (size_bytes != node::Buffer::Length(buffer)) {
const auto buffer_data = electron::util::as_byte_span(buffer);
if (size_bytes != buffer_data.size()) {
thrower.ThrowError("invalid buffer size");
return gin::Handle<NativeImage>();
}
@ -522,7 +520,7 @@ gin::Handle<NativeImage> NativeImage::CreateFromBitmap(
SkBitmap bitmap;
bitmap.allocN32Pixels(width, height, false);
bitmap.writePixels({info, node::Buffer::Data(buffer), bitmap.rowBytes()});
bitmap.writePixels({info, buffer_data.data(), bitmap.rowBytes()});
gfx::ImageSkia image_skia =
gfx::ImageSkia::CreateFromBitmap(bitmap, scale_factor);
@ -553,8 +551,8 @@ gin::Handle<NativeImage> NativeImage::CreateFromBuffer(
gfx::ImageSkia image_skia;
electron::util::AddImageSkiaRepFromBuffer(
&image_skia, reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
node::Buffer::Length(buffer), width, height, scale_factor);
&image_skia, electron::util::as_byte_span(buffer), width, height,
scale_factor);
return Create(args->isolate(), gfx::Image(image_skia));
}
@ -564,9 +562,9 @@ gin::Handle<NativeImage> NativeImage::CreateFromDataURL(v8::Isolate* isolate,
std::string mime_type, charset, data;
if (net::DataURL::Parse(url, &mime_type, &charset, &data)) {
if (mime_type == "image/png")
return CreateFromPNG(isolate, data.c_str(), data.size());
else if (mime_type == "image/jpeg")
return CreateFromJPEG(isolate, data.c_str(), data.size());
return CreateFromPNG(isolate, base::as_byte_span(data));
if (mime_type == "image/jpeg")
return CreateFromJPEG(isolate, base::as_byte_span(data));
}
return CreateEmpty(isolate);

10
shell/common/api/electron_api_native_image.h
View file

@ -9,6 +9,7 @@
#include <vector>
#include "base/containers/flat_map.h"
#include "base/containers/span.h"
#include "base/memory/raw_ptr.h"
#include "base/values.h"
#include "gin/wrappable.h"
@ -61,11 +62,10 @@ class NativeImage final : public gin::Wrappable<NativeImage> {
static gin::Handle<NativeImage> Create(v8::Isolate* isolate,
const gfx::Image& image);
static gin::Handle<NativeImage> CreateFromPNG(v8::Isolate* isolate,
const char* buffer,
size_t length);
static gin::Handle<NativeImage> CreateFromJPEG(v8::Isolate* isolate,
const char* buffer,
size_t length);
base::span<const uint8_t> data);
static gin::Handle<NativeImage> CreateFromJPEG(
v8::Isolate* isolate,
base::span<const uint8_t> data);
static gin::Handle<NativeImage> CreateFromPath(v8::Isolate* isolate,
const base::FilePath& path);
static gin::Handle<NativeImage> CreateFromBitmap(

20
shell/common/api/electron_api_native_image_mac.mm
View file

@ -26,6 +26,19 @@
namespace electron::api {
namespace {
base::span<const uint8_t> as_byte_span(NSData* data) {
// SAFETY: There is no NSData API that passes the UNSAFE_BUFFER_USAGE
// test, so let's isolate the unsafe API use into this function. Instead of
// calling '[data bytes]' and '[data length]' directly, the rest of our
// code should prefer to use spans returned by this function.
return UNSAFE_BUFFERS(base::span{
reinterpret_cast<const uint8_t*>([data bytes]), [data length]});
}
} // namespace
NSData* bufferFromNSImage(NSImage* image) {
CGImageRef ref = [image CGImageForProposedRect:nil context:nil hints:nil];
NSBitmapImageRep* rep = [[NSBitmapImageRep alloc] initWithCGImage:ref];
@ -127,9 +140,7 @@ gin::Handle<NativeImage> NativeImage::CreateFromNamedImage(gin::Arguments* args,
NSData* png_data = bufferFromNSImage(image);
if (args->GetNext(&hsl_shift) && hsl_shift.size() == 3) {
gfx::Image gfx_image = gfx::Image::CreateFrom1xPNGBytes(
{reinterpret_cast<const uint8_t*>((char*)[png_data bytes]),
[png_data length]});
auto gfx_image = gfx::Image::CreateFrom1xPNGBytes(as_byte_span(png_data));
color_utils::HSL shift = {safeShift(hsl_shift[0], -1),
safeShift(hsl_shift[1], 0.5),
safeShift(hsl_shift[2], 0.5)};
@ -139,8 +150,7 @@ gin::Handle<NativeImage> NativeImage::CreateFromNamedImage(gin::Arguments* args,
.AsNSImage());
}
return CreateFromPNG(args->isolate(), (char*)[png_data bytes],
[png_data length]);
return CreateFromPNG(args->isolate(), as_byte_span(png_data));
}
}

11
shell/common/node_util.cc
View file

@ -4,6 +4,7 @@
#include "shell/common/node_util.h"
#include "base/compiler_specific.h"
#include "base/logging.h"
#include "gin/converter.h"
#include "gin/dictionary.h"
@ -65,4 +66,14 @@ void EmitWarning(v8::Isolate* isolate,
emit_warning.Run(warning_msg, warning_type, "");
}
// SAFETY: There is no node::Buffer API that passes the UNSAFE_BUFFER_USAGE
// test, so let's isolate the unsafe API use into this function. Instead of
// calling `Buffer::Data()` and `Buffer::Length()` directly, the rest of our
// code should prefer to use spans returned by this function.
base::span<uint8_t> as_byte_span(v8::Local<v8::Value> node_buffer) {
auto* data = reinterpret_cast<uint8_t*>(node::Buffer::Data(node_buffer));
const auto size = node::Buffer::Length(node_buffer);
return UNSAFE_BUFFERS(base::span{data, size});
}
} // namespace electron::util

6
shell/common/node_util.h
View file

@ -8,6 +8,7 @@
#include <string_view>
#include <vector>
#include "base/containers/span.h"
#include "v8/include/v8-forward.h"
namespace node {
@ -36,6 +37,11 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
std::vector<v8::Local<v8::String>>* parameters,
std::vector<v8::Local<v8::Value>>* arguments);
// Convenience function to view a Node buffer's data as a base::span().
// Analogous to base::as_byte_span()
[[nodiscard]] base::span<uint8_t> as_byte_span(
v8::Local<v8::Value> node_buffer);
} // namespace electron::util
#endif // ELECTRON_SHELL_COMMON_NODE_UTIL_H_

28
shell/common/skia_util.cc
View file

@ -56,11 +56,10 @@ float GetScaleFactorFromPath(const base::FilePath& path) {
}
bool AddImageSkiaRepFromPNG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
const base::span<const uint8_t> data,
double scale_factor) {
SkBitmap bitmap;
if (!gfx::PNGCodec::Decode(data, size, &bitmap))
if (!gfx::PNGCodec::Decode(data.data(), data.size(), &bitmap))
return false;
image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
@ -68,10 +67,9 @@ bool AddImageSkiaRepFromPNG(gfx::ImageSkia* image,
}
bool AddImageSkiaRepFromJPEG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
const base::span<const uint8_t> data,
double scale_factor) {
auto bitmap = gfx::JPEGCodec::Decode(data, size);
auto bitmap = gfx::JPEGCodec::Decode(data.data(), data.size());
if (!bitmap)
return false;
@ -89,29 +87,28 @@ bool AddImageSkiaRepFromJPEG(gfx::ImageSkia* image,
}
bool AddImageSkiaRepFromBuffer(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
const base::span<const uint8_t> data,
int width,
int height,
double scale_factor) {
// Try PNG first.
if (AddImageSkiaRepFromPNG(image, data, size, scale_factor))
if (AddImageSkiaRepFromPNG(image, data, scale_factor))
return true;
// Try JPEG second.
if (AddImageSkiaRepFromJPEG(image, data, size, scale_factor))
if (AddImageSkiaRepFromJPEG(image, data, scale_factor))
return true;
if (width == 0 || height == 0)
return false;
auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType);
if (size < info.computeMinByteSize())
if (data.size() < info.computeMinByteSize())
return false;
SkBitmap bitmap;
bitmap.allocN32Pixels(width, height, false);
bitmap.writePixels({info, data, bitmap.rowBytes()});
bitmap.writePixels({info, data.data(), bitmap.rowBytes()});
image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
return true;
@ -127,11 +124,8 @@ bool AddImageSkiaRepFromPath(gfx::ImageSkia* image,
return false;
}
const auto* data =
reinterpret_cast<const unsigned char*>(file_contents.data());
size_t size = file_contents.size();
return AddImageSkiaRepFromBuffer(image, data, size, 0, 0, scale_factor);
return AddImageSkiaRepFromBuffer(image, base::as_byte_span(file_contents), 0,
0, scale_factor);
}
bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,

15
shell/common/skia_util.h
View file

@ -5,9 +5,13 @@
#ifndef ELECTRON_SHELL_COMMON_SKIA_UTIL_H_
#define ELECTRON_SHELL_COMMON_SKIA_UTIL_H_
#include <cstdint>
#include "base/containers/span.h"
namespace base {
class FilePath;
}
} // namespace base
namespace gfx {
class ImageSkia;
@ -19,20 +23,17 @@ bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
const base::FilePath& path);
bool AddImageSkiaRepFromBuffer(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
base::span<const uint8_t> data,
int width,
int height,
double scale_factor);
bool AddImageSkiaRepFromJPEG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
base::span<const uint8_t> data,
double scale_factor);
bool AddImageSkiaRepFromPNG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
base::span<const uint8_t> data,
double scale_factor);
#if BUILDFLAG(IS_WIN)