refactor: move gfx::ImageSkia functions to their own util file (#19460)

* initial refactor

* fix lint

* 🚚🚚🚚🚚🚚

* change year in copyright?

* lint more

* remove bad code

* add missing header

* fix: add missing ifdef guard

* fix variable call

filenames.gni

@@ -567,6 +567,8 @@ filenames = {
     "shell/common/platform_util_win.cc",
     "shell/common/promise_util.h",
     "shell/common/promise_util.cc",
+    "shell/common/skia_util.h",
+    "shell/common/skia_util.cc",
     "shell/renderer/api/atom_api_renderer_ipc.cc",
     "shell/renderer/api/atom_api_spell_check_client.cc",
     "shell/renderer/api/atom_api_spell_check_client.h",

shell/common/api/atom_api_native_image.cc

@@ -21,6 +21,7 @@
 #include "shell/common/native_mate_converters/gurl_converter.h"
 #include "shell/common/native_mate_converters/value_converter.h"
 #include "shell/common/node_includes.h"
+#include "shell/common/skia_util.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
 #include "third_party/skia/include/core/SkPixelRef.h"
@@ -45,32 +46,6 @@ namespace api {
 
 namespace {
 
-struct ScaleFactorPair {
-  const char* name;
-  float scale;
-};
-
-ScaleFactorPair kScaleFactorPairs[] = {
-    // The "@2x" is put as first one to make scale matching faster.
-    {"@2x", 2.0f},   {"@3x", 3.0f},     {"@1x", 1.0f},     {"@4x", 4.0f},
-    {"@5x", 5.0f},   {"@1.25x", 1.25f}, {"@1.33x", 1.33f}, {"@1.4x", 1.4f},
-    {"@1.5x", 1.5f}, {"@1.8x", 1.8f},   {"@2.5x", 2.5f},
-};
-
-float GetScaleFactorFromPath(const base::FilePath& path) {
-  std::string filename(path.BaseName().RemoveExtension().AsUTF8Unsafe());
-
-  // We don't try to convert string to float here because it is very very
-  // expensive.
-  for (const auto& kScaleFactorPair : kScaleFactorPairs) {
-    if (base::EndsWith(filename, kScaleFactorPair.name,
-                       base::CompareCase::INSENSITIVE_ASCII))
-      return kScaleFactorPair.scale;
-  }
-
-  return 1.0f;
-}
-
 // Get the scale factor from options object at the first argument
 float GetScaleFactorFromOptions(mate::Arguments* args) {
   float scale_factor = 1.0f;
@@ -80,101 +55,6 @@ float GetScaleFactorFromOptions(mate::Arguments* args) {
   return scale_factor;
 }
 
-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;
-
-  image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
-  return true;
-}
-
-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.writePixels({info, data, bitmap.rowBytes()});
-
-  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;
-    if (!asar::ReadFileToString(path, &file_contents))
-      return false;
-  }
-
-  const unsigned char* data =
-      reinterpret_cast<const unsigned char*>(file_contents.data());
-  size_t size = file_contents.size();
-
-  return AddImageSkiaRepFromBuffer(image, data, size, 0, 0, scale_factor);
-}
-
-bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
-                                   const base::FilePath& path) {
-  bool succeed = false;
-  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 AddImageSkiaRepFromPath(image, path, GetScaleFactorFromPath(path));
-  else
-    succeed |= AddImageSkiaRepFromPath(image, path, 1.0f);
-
-  for (const ScaleFactorPair& pair : kScaleFactorPairs)
-    succeed |= AddImageSkiaRepFromPath(
-        image, path.InsertBeforeExtensionASCII(pair.name), pair.scale);
-  return succeed;
-}
-
 base::FilePath NormalizePath(const base::FilePath& path) {
   if (!path.ReferencesParent()) {
     return path;
@@ -214,16 +94,6 @@ base::win::ScopedHICON ReadICOFromPath(int size, const base::FilePath& path) {
       static_cast<HICON>(LoadImage(NULL, image_path.value().c_str(), IMAGE_ICON,
                                    size, size, LR_LOADFROMFILE)));
 }
-
-bool ReadImageSkiaFromICO(gfx::ImageSkia* image, HICON icon) {
-  // Convert the icon from the Windows specific HICON to gfx::ImageSkia.
-  SkBitmap bitmap = IconUtil::CreateSkBitmapFromHICON(icon);
-  if (bitmap.isNull())
-    return false;
-
-  image->AddRepresentation(gfx::ImageSkiaRep(bitmap, 1.0f));
-  return true;
-}
 #endif
 
 void Noop(char*, void*) {}
@@ -244,7 +114,7 @@ NativeImage::NativeImage(v8::Isolate* isolate, const base::FilePath& hicon_path)
     : hicon_path_(hicon_path) {
   // Use the 256x256 icon as fallback icon.
   gfx::ImageSkia image_skia;
-  ReadImageSkiaFromICO(&image_skia, GetHICON(256));
+  electron::util::ReadImageSkiaFromICO(&image_skia, GetHICON(256));
   image_ = gfx::Image(image_skia);
   Init(isolate);
   if (image_.HasRepresentation(gfx::Image::kImageRepSkia)) {
@@ -447,18 +317,18 @@ void NativeImage::AddRepresentation(const mate::Dictionary& options) {
   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 = AddImageSkiaRepFromBuffer(&image_skia, data, size, width,
+    skia_rep_added = electron::util::AddImageSkiaRepFromBuffer(
-                                               height, scale_factor);
+        &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)) {
       auto* data_ptr = reinterpret_cast<const unsigned char*>(data.c_str());
       if (mime_type == "image/png") {
-        skia_rep_added = AddImageSkiaRepFromPNG(&image_skia, data_ptr,
+        skia_rep_added = electron::util::AddImageSkiaRepFromPNG(
-                                                data.size(), scale_factor);
+            &image_skia, data_ptr, data.size(), scale_factor);
       } else if (mime_type == "image/jpeg") {
-        skia_rep_added = AddImageSkiaRepFromJPEG(&image_skia, data_ptr,
+        skia_rep_added = electron::util::AddImageSkiaRepFromJPEG(
-                                                 data.size(), scale_factor);
+            &image_skia, data_ptr, data.size(), scale_factor);
       }
     }
   }
@@ -518,7 +388,7 @@ mate::Handle<NativeImage> NativeImage::CreateFromPath(
   }
 #endif
   gfx::ImageSkia image_skia;
-  PopulateImageSkiaRepsFromPath(&image_skia, image_path);
+  electron::util::PopulateImageSkiaRepsFromPath(&image_skia, image_path);
   gfx::Image image(image_skia);
   mate::Handle<NativeImage> handle = Create(isolate, image);
 #if defined(OS_MACOSX)
@@ -597,7 +467,7 @@ mate::Handle<NativeImage> NativeImage::CreateFromBuffer(
   }
 
   gfx::ImageSkia image_skia;
-  AddImageSkiaRepFromBuffer(
+  electron::util::AddImageSkiaRepFromBuffer(
       &image_skia, reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
       node::Buffer::Length(buffer), width, height, scale_factor);
   return Create(args->isolate(), gfx::Image(image_skia));

shell/common/skia_util.cc

@@ -0,0 +1,167 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include <string>
+
+#include "base/files/file_util.h"
+#include "base/strings/pattern.h"
+#include "base/strings/string_util.h"
+#include "base/threading/thread_restrictions.h"
+#include "net/base/data_url.h"
+#include "shell/common/asar/asar_util.h"
+#include "shell/common/node_includes.h"
+#include "shell/common/skia_util.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkImageInfo.h"
+#include "third_party/skia/include/core/SkPixelRef.h"
+#include "ui/gfx/codec/jpeg_codec.h"
+#include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/geometry/size.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image_skia_operations.h"
+#include "ui/gfx/image/image_util.h"
+
+#if defined(OS_WIN)
+#include "ui/gfx/icon_util.h"
+#endif
+
+namespace electron {
+
+namespace util {
+
+struct ScaleFactorPair {
+  const char* name;
+  float scale;
+};
+
+ScaleFactorPair kScaleFactorPairs[] = {
+    // The "@2x" is put as first one to make scale matching faster.
+    {"@2x", 2.0f},   {"@3x", 3.0f},     {"@1x", 1.0f},     {"@4x", 4.0f},
+    {"@5x", 5.0f},   {"@1.25x", 1.25f}, {"@1.33x", 1.33f}, {"@1.4x", 1.4f},
+    {"@1.5x", 1.5f}, {"@1.8x", 1.8f},   {"@2.5x", 2.5f},
+};
+
+float GetScaleFactorFromPath(const base::FilePath& path) {
+  std::string filename(path.BaseName().RemoveExtension().AsUTF8Unsafe());
+
+  // We don't try to convert string to float here because it is very very
+  // expensive.
+  for (const auto& kScaleFactorPair : kScaleFactorPairs) {
+    if (base::EndsWith(filename, kScaleFactorPair.name,
+                       base::CompareCase::INSENSITIVE_ASCII))
+      return kScaleFactorPair.scale;
+  }
+
+  return 1.0f;
+}
+
+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;
+
+  image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
+  return true;
+}
+
+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.writePixels({info, data, bitmap.rowBytes()});
+
+  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;
+    if (!asar::ReadFileToString(path, &file_contents))
+      return false;
+  }
+
+  const unsigned char* data =
+      reinterpret_cast<const unsigned char*>(file_contents.data());
+  size_t size = file_contents.size();
+
+  return AddImageSkiaRepFromBuffer(image, data, size, 0, 0, scale_factor);
+}
+
+bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
+                                   const base::FilePath& path) {
+  bool succeed = false;
+  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 AddImageSkiaRepFromPath(image, path, GetScaleFactorFromPath(path));
+  else
+    succeed |= AddImageSkiaRepFromPath(image, path, 1.0f);
+
+  for (const ScaleFactorPair& pair : kScaleFactorPairs)
+    succeed |= AddImageSkiaRepFromPath(
+        image, path.InsertBeforeExtensionASCII(pair.name), pair.scale);
+  return succeed;
+}
+#if defined(OS_WIN)
+bool ReadImageSkiaFromICO(gfx::ImageSkia* image, HICON icon) {
+  // Convert the icon from the Windows specific HICON to gfx::ImageSkia.
+  SkBitmap bitmap = IconUtil::CreateSkBitmapFromHICON(icon);
+  if (bitmap.isNull())
+    return false;
+
+  image->AddRepresentation(gfx::ImageSkiaRep(bitmap, 1.0f));
+  return true;
+}
+#endif
+
+}  // namespace util
+
+}  // namespace electron

shell/common/skia_util.h

@@ -0,0 +1,44 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_SKIA_UTIL_H_
+#define SHELL_COMMON_SKIA_UTIL_H_
+
+#include <string>
+
+#include "ui/gfx/image/image_skia.h"
+
+namespace electron {
+
+namespace util {
+
+bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
+                                   const base::FilePath& path);
+
+bool AddImageSkiaRepFromBuffer(gfx::ImageSkia* image,
+                               const unsigned char* data,
+                               size_t size,
+                               int width,
+                               int height,
+                               double scale_factor);
+
+bool AddImageSkiaRepFromJPEG(gfx::ImageSkia* image,
+                             const unsigned char* data,
+                             size_t size,
+                             double scale_factor);
+
+bool AddImageSkiaRepFromPNG(gfx::ImageSkia* image,
+                            const unsigned char* data,
+                            size_t size,
+                            double scale_factor);
+
+#if defined(OS_WIN)
+bool ReadImageSkiaFromICO(gfx::ImageSkia* image, HICON icon);
+#endif
+
+}  // namespace util
+
+}  // namespace electron
+
+#endif  // SHELL_COMMON_SKIA_UTIL_H_