diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index 081431b3f4..e202e174d9 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -80,49 +80,71 @@ float GetScaleFactorFromOptions(mate::Arguments* args) { return scale_factor; } -bool AddImageSkiaRep(gfx::ImageSkia* image, - const unsigned char* data, - size_t size, - int width, - int height, - double scale_factor) { - auto decoded = std::make_unique(); +bool AddImageSkiaRepFromPNG(gfx::ImageSkia* image, + const unsigned char* data, + size_t size, + double scale_factor) { + SkBitmap bitmap; + if (!gfx::PNGCodec::Decode(data, size, &bitmap)) + return false; - // Try PNG first. - if (!gfx::PNGCodec::Decode(data, size, decoded.get())) { - // Try JPEG. - decoded = gfx::JPEGCodec::Decode(data, size); - if (decoded) { - // `JPEGCodec::Decode()` doesn't tell `SkBitmap` instance it creates - // that all of its pixels are opaque, that's why the bitmap gets - // an alpha type `kPremul_SkAlphaType` instead of `kOpaque_SkAlphaType`. - // Let's fix it here. - // TODO(alexeykuzmin): This workaround should be removed - // when the `JPEGCodec::Decode()` code is fixed. - // See https://github.com/electron/electron/issues/11294. - decoded->setAlphaType(SkAlphaType::kOpaque_SkAlphaType); - } - } - - if (!decoded) { - // Try Bitmap - if (width > 0 && height > 0) { - decoded.reset(new SkBitmap); - decoded->allocN32Pixels(width, height, false); - decoded->setPixels( - const_cast(reinterpret_cast(data))); - } else { - return false; - } - } - - image->AddRepresentation(gfx::ImageSkiaRep(*decoded, scale_factor)); + image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor)); return true; } -bool AddImageSkiaRep(gfx::ImageSkia* image, - const base::FilePath& path, - double scale_factor) { +bool AddImageSkiaRepFromJPEG(gfx::ImageSkia* image, + const unsigned char* data, + size_t size, + double scale_factor) { + auto bitmap = gfx::JPEGCodec::Decode(data, size); + if (!bitmap) + return false; + + // `JPEGCodec::Decode()` doesn't tell `SkBitmap` instance it creates + // that all of its pixels are opaque, that's why the bitmap gets + // an alpha type `kPremul_SkAlphaType` instead of `kOpaque_SkAlphaType`. + // Let's fix it here. + // TODO(alexeykuzmin): This workaround should be removed + // when the `JPEGCodec::Decode()` code is fixed. + // See https://github.com/electron/electron/issues/11294. + bitmap->setAlphaType(SkAlphaType::kOpaque_SkAlphaType); + + image->AddRepresentation(gfx::ImageSkiaRep(*bitmap, scale_factor)); + return true; +} + +bool AddImageSkiaRepFromBuffer(gfx::ImageSkia* image, + const unsigned char* data, + size_t size, + int width, + int height, + double scale_factor) { + // Try PNG first. + if (AddImageSkiaRepFromPNG(image, data, size, scale_factor)) + return true; + + // Try JPEG second. + if (AddImageSkiaRepFromJPEG(image, data, size, scale_factor)) + return true; + + if (width == 0 || height == 0) + return false; + + auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType); + if (size < info.computeMinByteSize()) + return false; + + SkBitmap bitmap; + bitmap.allocN32Pixels(width, height, false); + bitmap.setPixels(const_cast(reinterpret_cast(data))); + + image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor)); + return true; +} + +bool AddImageSkiaRepFromPath(gfx::ImageSkia* image, + const base::FilePath& path, + double scale_factor) { std::string file_contents; { base::ThreadRestrictions::ScopedAllowIO allow_io; @@ -133,7 +155,8 @@ bool AddImageSkiaRep(gfx::ImageSkia* image, const unsigned char* data = reinterpret_cast(file_contents.data()); size_t size = file_contents.size(); - return AddImageSkiaRep(image, data, size, 0, 0, scale_factor); + + return AddImageSkiaRepFromBuffer(image, data, size, 0, 0, scale_factor); } bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image, @@ -142,12 +165,12 @@ bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image, std::string filename(path.BaseName().RemoveExtension().AsUTF8Unsafe()); if (base::MatchPattern(filename, "*@*x")) // Don't search for other representations if the DPI has been specified. - return AddImageSkiaRep(image, path, GetScaleFactorFromPath(path)); + return AddImageSkiaRepFromPath(image, path, GetScaleFactorFromPath(path)); else - succeed |= AddImageSkiaRep(image, path, 1.0f); + succeed |= AddImageSkiaRepFromPath(image, path, 1.0f); for (const ScaleFactorPair& pair : kScaleFactorPairs) - succeed |= AddImageSkiaRep( + succeed |= AddImageSkiaRepFromPath( image, path.InsertBeforeExtensionASCII(pair.name), pair.scale); return succeed; } @@ -422,19 +445,20 @@ void NativeImage::AddRepresentation(const mate::Dictionary& options) { v8::Local buffer; GURL url; if (options.Get("buffer", &buffer) && node::Buffer::HasInstance(buffer)) { - AddImageSkiaRep( - &image_skia, - reinterpret_cast(node::Buffer::Data(buffer)), - node::Buffer::Length(buffer), width, height, scale_factor); - skia_rep_added = true; + auto* data = reinterpret_cast(node::Buffer::Data(buffer)); + auto size = node::Buffer::Length(buffer); + skia_rep_added = AddImageSkiaRepFromBuffer(&image_skia, data, size, 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)) { - if (mime_type == "image/png" || mime_type == "image/jpeg") { - AddImageSkiaRep(&image_skia, - reinterpret_cast(data.c_str()), - data.size(), width, height, scale_factor); - skia_rep_added = true; + auto* data_ptr = reinterpret_cast(data.c_str()); + if (mime_type == "image/png") { + skia_rep_added = AddImageSkiaRepFromPNG(&image_skia, data_ptr, + data.size(), scale_factor); + } else if (mime_type == "image/jpeg") { + skia_rep_added = AddImageSkiaRepFromJPEG(&image_skia, data_ptr, + data.size(), scale_factor); } } } @@ -573,9 +597,9 @@ mate::Handle NativeImage::CreateFromBuffer( } gfx::ImageSkia image_skia; - AddImageSkiaRep(&image_skia, - reinterpret_cast(node::Buffer::Data(buffer)), - node::Buffer::Length(buffer), width, height, scale_factor); + AddImageSkiaRepFromBuffer( + &image_skia, reinterpret_cast(node::Buffer::Data(buffer)), + node::Buffer::Length(buffer), width, height, scale_factor); return Create(args->isolate(), gfx::Image(image_skia)); } diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index 2ffd28e6d5..475e6db177 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -156,6 +156,11 @@ describe('nativeImage module', () => { expect(nativeImage.createFromBuffer(Buffer.from([])).isEmpty()).to.be.true() }) + it('returns an empty image when the buffer is too small', () => { + const image = nativeImage.createFromBuffer(Buffer.from([1, 2, 3, 4]), { width: 100, height: 100 }) + expect(image.isEmpty()).to.be.true() + }) + it('returns an image created from the given buffer', () => { const imageA = nativeImage.createFromPath(path.join(__dirname, 'fixtures', 'assets', 'logo.png')) @@ -476,6 +481,18 @@ describe('nativeImage module', () => { }) describe('addRepresentation()', () => { + it('does not add representation when the buffer is too small', () => { + const image = nativeImage.createEmpty() + + image.addRepresentation({ + buffer: Buffer.from([1, 2, 3, 4]), + width: 100, + height: 100 + }) + + expect(image.isEmpty()).to.be.true() + }) + it('supports adding a buffer representation for a scale factor', () => { const image = nativeImage.createEmpty()