From c3256a51cb2f5d9e12277ebe4a314d15fac6f869 Mon Sep 17 00:00:00 2001
From: Karol Kosek <krkk@serenityos.org>
Date: Sun, 14 Nov 2021 21:42:12 +0100
Subject: [PATCH] LibGfx: Remove scale factor option from
 try_load_from_fd_and_close()

... and bring it back to try_load_from_file().

Prior to this change, changing the scaling option to x2 in the Display
Settings resulted in the following crash:

  WindowServer(15:15): ASSERTION FAILED: bitmap->width() % scale_factor
  == 0 ./Userland/Libraries/LibGfx/Bitmap.cpp:126

That was caused by two minor overlooked yaks:

- First, Bitmap::try_load_from_fd_and_close() tried to respect your
  scale factor.

  While requesting a bitmap from file can make a switcheroo to give you
  a higher resolution bitmap, doing the same when you already have an fd
  might violate the unveil agreement.
  ... but, it didn't do that.

  It read bitmaps from requested fds, but also pretended all system
  bitmaps in /res/ are the HiDPI ones when you enabled that mode.

- d85d741c59 used this function to deduplicate try_load_from_file().

  It actually made this bug a lot easier to replicate!


Closes #10920
---
 Userland/Libraries/LibGfx/Bitmap.cpp | 38 +++++++++++++++-------------
 Userland/Libraries/LibGfx/Bitmap.h   |  2 +-
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/Userland/Libraries/LibGfx/Bitmap.cpp b/Userland/Libraries/LibGfx/Bitmap.cpp
index 592f0a03c5..5d2c6e823b 100644
--- a/Userland/Libraries/LibGfx/Bitmap.cpp
+++ b/Userland/Libraries/LibGfx/Bitmap.cpp
@@ -102,14 +102,6 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_create_wrapper(BitmapFormat format, I
 }
 
 ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_file(String const& path, int scale_factor)
-{
-    int fd = open(path.characters(), O_RDONLY);
-    if (fd < 0)
-        return Error::from_errno(errno);
-    return try_load_from_fd_and_close(fd, path, scale_factor);
-}
-
-ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_fd_and_close(int fd, String const& path, int scale_factor)
 {
     if (scale_factor > 1 && path.starts_with("/res/")) {
         LexicalPath lexical_path { path };
@@ -120,18 +112,28 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_fd_and_close(int fd, String
         highdpi_icon_path.appendff("-{}x.", scale_factor);
         highdpi_icon_path.append(lexical_path.extension());
 
-        auto file = TRY(MappedFile::map_from_fd_and_close(fd, highdpi_icon_path.to_string()));
-        if (auto decoder = ImageDecoder::try_create(file->bytes())) {
-            auto bitmap = decoder->frame(0).image;
-            VERIFY(bitmap->width() % scale_factor == 0);
-            VERIFY(bitmap->height() % scale_factor == 0);
-            bitmap->m_size.set_width(bitmap->width() / scale_factor);
-            bitmap->m_size.set_height(bitmap->height() / scale_factor);
-            bitmap->m_scale = scale_factor;
-            return bitmap.release_nonnull();
-        }
+        auto highdpi_icon_string = highdpi_icon_path.to_string();
+        int fd = open(highdpi_icon_string.characters(), O_RDONLY);
+        if (fd < 0)
+            return Error::from_errno(errno);
+
+        auto bitmap = TRY(try_load_from_fd_and_close(fd, highdpi_icon_string));
+        VERIFY(bitmap->width() % scale_factor == 0);
+        VERIFY(bitmap->height() % scale_factor == 0);
+        bitmap->m_size.set_width(bitmap->width() / scale_factor);
+        bitmap->m_size.set_height(bitmap->height() / scale_factor);
+        bitmap->m_scale = scale_factor;
+        return bitmap;
     }
 
+    int fd = open(path.characters(), O_RDONLY);
+    if (fd < 0)
+        return Error::from_errno(errno);
+    return try_load_from_fd_and_close(fd, path);
+}
+
+ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_fd_and_close(int fd, String const& path)
+{
     auto file = TRY(MappedFile::map_from_fd_and_close(fd, path));
     if (auto decoder = ImageDecoder::try_create(file->bytes())) {
         if (auto bitmap = decoder->frame(0).image)
diff --git a/Userland/Libraries/LibGfx/Bitmap.h b/Userland/Libraries/LibGfx/Bitmap.h
index c3eebd6a35..d6d8471b27 100644
--- a/Userland/Libraries/LibGfx/Bitmap.h
+++ b/Userland/Libraries/LibGfx/Bitmap.h
@@ -93,7 +93,7 @@ public:
     [[nodiscard]] static ErrorOr<NonnullRefPtr<Bitmap>> try_create_shareable(BitmapFormat, IntSize const&, int intrinsic_scale = 1);
     [[nodiscard]] static ErrorOr<NonnullRefPtr<Bitmap>> try_create_wrapper(BitmapFormat, IntSize const&, int intrinsic_scale, size_t pitch, void*);
     [[nodiscard]] static ErrorOr<NonnullRefPtr<Bitmap>> try_load_from_file(String const& path, int scale_factor = 1);
-    [[nodiscard]] static ErrorOr<NonnullRefPtr<Bitmap>> try_load_from_fd_and_close(int fd, String const& path, int scale_factor = 1);
+    [[nodiscard]] static ErrorOr<NonnullRefPtr<Bitmap>> try_load_from_fd_and_close(int fd, String const& path);
     [[nodiscard]] static ErrorOr<NonnullRefPtr<Bitmap>> try_create_with_anonymous_buffer(BitmapFormat, Core::AnonymousBuffer, IntSize const&, int intrinsic_scale, Vector<RGBA32> const& palette);
     static ErrorOr<NonnullRefPtr<Bitmap>> try_create_from_serialized_byte_buffer(ByteBuffer&&);