LibGfx: Return bool not ErrorOr<bool> from ImageDecoderPlugin::sniff()

Nobody made use of the ErrorOr return value and it just added more
chance of confusion, since it was not clear if failing to sniff an
image should return an error or false. The answer was false, if you
returned Error you'd crash the ImageDecoder.
This commit is contained in:
MacDue 2023-02-26 18:02:50 +00:00 committed by Andreas Kling
parent 8d9cb538d6
commit 6cf8eeb7a4
22 changed files with 40 additions and 40 deletions

View file

@ -31,7 +31,7 @@
TEST_CASE(test_bmp)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("rgba32-1.bmp"sv)));
EXPECT(MUST(Gfx::BMPImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::BMPImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::BMPImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -46,7 +46,7 @@ TEST_CASE(test_bmp)
TEST_CASE(test_gif)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("download-animation.gif"sv)));
EXPECT(MUST(Gfx::GIFImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::GIFImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::GIFImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -61,7 +61,7 @@ TEST_CASE(test_gif)
TEST_CASE(test_not_ico)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("buggie.png"sv)));
EXPECT(!MUST(Gfx::ICOImageDecoderPlugin::sniff(file->bytes())));
EXPECT(!Gfx::ICOImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::ICOImageDecoderPlugin::create(file->bytes()));
EXPECT(!plugin_decoder->initialize());
@ -75,7 +75,7 @@ TEST_CASE(test_not_ico)
TEST_CASE(test_bmp_embedded_in_ico)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("serenity.ico"sv)));
EXPECT(MUST(Gfx::ICOImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::ICOImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::ICOImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -89,7 +89,7 @@ TEST_CASE(test_bmp_embedded_in_ico)
TEST_CASE(test_jpg)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("rgb24.jpg"sv)));
EXPECT(MUST(Gfx::JPEGImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::JPEGImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::JPEGImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -104,7 +104,7 @@ TEST_CASE(test_jpg)
TEST_CASE(test_pbm)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("buggie-raw.pbm"sv)));
EXPECT(MUST(Gfx::PBMImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::PBMImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::PBMImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -119,7 +119,7 @@ TEST_CASE(test_pbm)
TEST_CASE(test_pgm)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("buggie-raw.pgm"sv)));
EXPECT(MUST(Gfx::PGMImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::PGMImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::PGMImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -134,7 +134,7 @@ TEST_CASE(test_pgm)
TEST_CASE(test_png)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("buggie.png"sv)));
EXPECT(MUST(Gfx::PNGImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::PNGImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::PNGImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -149,7 +149,7 @@ TEST_CASE(test_png)
TEST_CASE(test_ppm)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("buggie-raw.ppm"sv)));
EXPECT(MUST(Gfx::PPMImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::PPMImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::PPMImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -224,7 +224,7 @@ TEST_CASE(test_targa_top_left_compressed)
TEST_CASE(test_webp_simple_lossy)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("simple-vp8.webp"sv)));
EXPECT(MUST(Gfx::WebPImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::WebPImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::WebPImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -238,7 +238,7 @@ TEST_CASE(test_webp_simple_lossy)
TEST_CASE(test_webp_simple_lossless)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("simple-vp8l.webp"sv)));
EXPECT(MUST(Gfx::WebPImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::WebPImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::WebPImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -252,7 +252,7 @@ TEST_CASE(test_webp_simple_lossless)
TEST_CASE(test_webp_extended_lossy)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("extended-lossy.webp"sv)));
EXPECT(MUST(Gfx::WebPImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::WebPImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::WebPImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -266,7 +266,7 @@ TEST_CASE(test_webp_extended_lossy)
TEST_CASE(test_webp_extended_lossless)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("extended-lossless.webp"sv)));
EXPECT(MUST(Gfx::WebPImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::WebPImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::WebPImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());
@ -280,7 +280,7 @@ TEST_CASE(test_webp_extended_lossless)
TEST_CASE(test_webp_extended_lossless_animated)
{
auto file = MUST(Core::MappedFile::map(TEST_INPUT("extended-lossless-animated.webp"sv)));
EXPECT(MUST(Gfx::WebPImageDecoderPlugin::sniff(file->bytes())));
EXPECT(Gfx::WebPImageDecoderPlugin::sniff(file->bytes()));
auto plugin_decoder = MUST(Gfx::WebPImageDecoderPlugin::create(file->bytes()));
EXPECT(plugin_decoder->initialize());

View file

@ -210,7 +210,7 @@ Icon FileIconProvider::icon_for_executable(DeprecatedString const& path)
bitmap = s_executable_icon.bitmap_for_size(icon_section.image_size);
} else {
// FIXME: Use the ImageDecoder service.
if (Gfx::PNGImageDecoderPlugin::sniff({ section->raw_data(), section->size() }).release_value_but_fixme_should_propagate_errors()) {
if (Gfx::PNGImageDecoderPlugin::sniff({ section->raw_data(), section->size() })) {
auto png_decoder = Gfx::PNGImageDecoderPlugin::create({ section->raw_data(), section->size() }).release_value_but_fixme_should_propagate_errors();
if (png_decoder->initialize()) {
auto frame_or_error = png_decoder->frame(0);

View file

@ -1505,7 +1505,7 @@ bool BMPImageDecoderPlugin::initialize()
return !decode_bmp_header(*m_context).is_error();
}
ErrorOr<bool> BMPImageDecoderPlugin::sniff(ReadonlyBytes data)
bool BMPImageDecoderPlugin::sniff(ReadonlyBytes data)
{
BMPLoadingContext context;
context.file_bytes = data.data();

View file

@ -16,7 +16,7 @@ class ICOImageDecoderPlugin;
class BMPImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<BMPImageDecoderPlugin>> create_as_included_in_ico(Badge<ICOImageDecoderPlugin>, ReadonlyBytes);

View file

@ -656,7 +656,7 @@ bool DDSImageDecoderPlugin::initialize()
&& m_context->data[3] == 0x20;
}
ErrorOr<bool> DDSImageDecoderPlugin::sniff(ReadonlyBytes data)
bool DDSImageDecoderPlugin::sniff(ReadonlyBytes data)
{
// The header is always at least 128 bytes, so if the file is smaller, it can't be a DDS.
return data.size() > 128

View file

@ -235,7 +235,7 @@ struct DDSLoadingContext;
class DDSImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
virtual ~DDSImageDecoderPlugin() override;

View file

@ -569,7 +569,7 @@ bool GIFImageDecoderPlugin::initialize()
return !decode_gif_header(stream).is_error();
}
ErrorOr<bool> GIFImageDecoderPlugin::sniff(ReadonlyBytes data)
bool GIFImageDecoderPlugin::sniff(ReadonlyBytes data)
{
FixedMemoryStream stream { data };
return !decode_gif_header(stream).is_error();

View file

@ -15,7 +15,7 @@ struct GIFLoadingContext;
class GIFImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
virtual ~GIFImageDecoderPlugin() override;

View file

@ -149,7 +149,7 @@ ErrorOr<void> ICOImageDecoderPlugin::load_ico_bitmap(ICOLoadingContext& context,
return Error::from_string_literal("Index out of bounds");
ICOImageDescriptor& desc = context.images[real_index];
if (TRY(PNGImageDecoderPlugin::sniff({ context.data + desc.offset, desc.size }))) {
if (PNGImageDecoderPlugin::sniff({ context.data + desc.offset, desc.size })) {
auto png_decoder = TRY(PNGImageDecoderPlugin::create({ context.data + desc.offset, desc.size }));
if (png_decoder->initialize()) {
auto decoded_png_frame = TRY(png_decoder->frame(0));
@ -181,7 +181,7 @@ ErrorOr<void> ICOImageDecoderPlugin::load_ico_bitmap(ICOLoadingContext& context,
}
}
ErrorOr<bool> ICOImageDecoderPlugin::sniff(ReadonlyBytes data)
bool ICOImageDecoderPlugin::sniff(ReadonlyBytes data)
{
FixedMemoryStream stream { data };
return !decode_ico_header(stream).is_error();

View file

@ -14,7 +14,7 @@ struct ICOLoadingContext;
class ICOImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
virtual ~ICOImageDecoderPlugin() override;

View file

@ -22,7 +22,7 @@
namespace Gfx {
struct ImagePluginInitializer {
ErrorOr<bool> (*sniff)(ReadonlyBytes) = nullptr;
bool (*sniff)(ReadonlyBytes) = nullptr;
ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> (*create)(ReadonlyBytes) = nullptr;
};
@ -53,7 +53,7 @@ static constexpr ImagePluginWithMIMETypeInitializer s_initializers_with_mime_typ
static OwnPtr<ImageDecoderPlugin> probe_and_sniff_for_appropriate_plugin(ReadonlyBytes bytes)
{
for (auto& plugin : s_initializers) {
auto sniff_result = plugin.sniff(bytes).release_value_but_fixme_should_propagate_errors();
auto sniff_result = plugin.sniff(bytes);
if (!sniff_result)
continue;
auto plugin_decoder = plugin.create(bytes).release_value_but_fixme_should_propagate_errors();

View file

@ -1363,7 +1363,7 @@ bool JPEGImageDecoderPlugin::initialize()
return true;
}
ErrorOr<bool> JPEGImageDecoderPlugin::sniff(ReadonlyBytes data)
bool JPEGImageDecoderPlugin::sniff(ReadonlyBytes data)
{
return data.size() > 3
&& data.data()[0] == 0xFF

View file

@ -16,7 +16,7 @@ struct JPEGLoadingContext;
class JPEGImageDecoderPlugin : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
virtual ~JPEGImageDecoderPlugin() override;

View file

@ -1034,7 +1034,7 @@ bool PNGImageDecoderPlugin::initialize()
return decode_png_header(*m_context);
}
ErrorOr<bool> PNGImageDecoderPlugin::sniff(ReadonlyBytes data)
bool PNGImageDecoderPlugin::sniff(ReadonlyBytes data)
{
PNGLoadingContext context;
context.data = data.data();

View file

@ -14,7 +14,7 @@ struct PNGLoadingContext;
class PNGImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
virtual ~PNGImageDecoderPlugin() override;

View file

@ -49,7 +49,7 @@ struct PortableImageMapLoadingContext {
template<typename TContext>
class PortableImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
PortableImageDecoderPlugin(u8 const*, size_t);
@ -133,7 +133,7 @@ ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> PortableImageDecoderPlugin<TContext>:
}
template<typename TContext>
ErrorOr<bool> PortableImageDecoderPlugin<TContext>::sniff(ReadonlyBytes data)
bool PortableImageDecoderPlugin<TContext>::sniff(ReadonlyBytes data)
{
using Context = TContext;
if (data.size() < 2)

View file

@ -200,7 +200,7 @@ bool QOIImageDecoderPlugin::initialize()
return !decode_header_and_update_context(*m_context->stream).is_error();
}
ErrorOr<bool> QOIImageDecoderPlugin::sniff(ReadonlyBytes data)
bool QOIImageDecoderPlugin::sniff(ReadonlyBytes data)
{
FixedMemoryStream stream { { data.data(), data.size() } };
return !decode_qoi_header(stream).is_error();

View file

@ -38,7 +38,7 @@ struct QOILoadingContext {
class QOIImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
virtual ~QOIImageDecoderPlugin() override = default;

View file

@ -522,7 +522,7 @@ bool WebPImageDecoderPlugin::initialize()
return !decode_webp_header(*m_context).is_error();
}
ErrorOr<bool> WebPImageDecoderPlugin::sniff(ReadonlyBytes data)
bool WebPImageDecoderPlugin::sniff(ReadonlyBytes data)
{
WebPLoadingContext context;
context.data = data;

View file

@ -14,7 +14,7 @@ struct WebPLoadingContext;
class WebPImageDecoderPlugin final : public ImageDecoderPlugin {
public:
static ErrorOr<bool> sniff(ReadonlyBytes);
static bool sniff(ReadonlyBytes);
static ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> create(ReadonlyBytes);
virtual ~WebPImageDecoderPlugin() override;

View file

@ -269,7 +269,7 @@ ErrorOr<ByteBuffer> Filter::decode_jbig2(ReadonlyBytes)
ErrorOr<ByteBuffer> Filter::decode_dct(ReadonlyBytes bytes)
{
if (Gfx::JPEGImageDecoderPlugin::sniff({ bytes.data(), bytes.size() }).release_value_but_fixme_should_propagate_errors()) {
if (Gfx::JPEGImageDecoderPlugin::sniff({ bytes.data(), bytes.size() })) {
auto decoder = Gfx::JPEGImageDecoderPlugin::create({ bytes.data(), bytes.size() }).release_value_but_fixme_should_propagate_errors();
if (decoder->initialize()) {
auto frame = TRY(decoder->frame(0));

View file

@ -142,19 +142,19 @@ void SpiceAgent::on_message_received()
} else {
ErrorOr<Gfx::ImageFrameDescriptor> frame_or_error = Gfx::ImageFrameDescriptor {};
if (type == ClipboardType::PNG) {
if (Gfx::PNGImageDecoderPlugin::sniff({ data_buffer.data(), data_buffer.size() }).release_value_but_fixme_should_propagate_errors()) {
if (Gfx::PNGImageDecoderPlugin::sniff({ data_buffer.data(), data_buffer.size() })) {
auto png_decoder = Gfx::PNGImageDecoderPlugin::create({ data_buffer.data(), data_buffer.size() }).release_value_but_fixme_should_propagate_errors();
if (png_decoder->initialize())
frame_or_error = png_decoder->frame(0);
}
} else if (type == ClipboardType::BMP) {
if (Gfx::BMPImageDecoderPlugin::sniff({ data_buffer.data(), data_buffer.size() }).release_value_but_fixme_should_propagate_errors()) {
if (Gfx::BMPImageDecoderPlugin::sniff({ data_buffer.data(), data_buffer.size() })) {
auto bmp_decoder = Gfx::BMPImageDecoderPlugin::create({ data_buffer.data(), data_buffer.size() }).release_value_but_fixme_should_propagate_errors();
if (bmp_decoder->initialize())
frame_or_error = bmp_decoder->frame(0);
}
} else if (type == ClipboardType::JPEG) {
if (Gfx::JPEGImageDecoderPlugin::sniff({ data_buffer.data(), data_buffer.size() }).release_value_but_fixme_should_propagate_errors()) {
if (Gfx::JPEGImageDecoderPlugin::sniff({ data_buffer.data(), data_buffer.size() })) {
auto jpeg_decoder = Gfx::JPEGImageDecoderPlugin::create({ data_buffer.data(), data_buffer.size() }).release_value_but_fixme_should_propagate_errors();
if (jpeg_decoder->initialize())
frame_or_error = jpeg_decoder->frame(0);