From 53f0b00328a9688f36e5b1eeae311c488ae3cae5 Mon Sep 17 00:00:00 2001
From: Stefan Markovic <57057282+stefansjfw@users.noreply.github.com>
Date: Tue, 7 Feb 2023 21:11:31 +0100
Subject: [PATCH] [Thumbnail providers] Cover errors and edge cases (#23947)
* [Thumbnail providers] Cover errors and edge cases
* Add TODO comment
* Fix tests
---
.../GcodeThumbnailProvider.cs | 8 ++-
.../GcodeThumbnailProvider/Program.cs | 7 ++-
.../GcodeThumbnailProvider.cpp | 16 ++++--
.../GcodeThumbnailProvider.h | 2 +-
.../PdfThumbnailProvider.cs | 53 +++++++++----------
.../PdfThumbnailProvider/Program.cs | 7 ++-
.../PdfThumbnailProvider.cpp | 14 +++--
.../PdfThumbnailProvider.h | 2 +-
.../StlThumbnailProvider/Program.cs | 7 ++-
.../StlThumbnailProvider.cs | 15 ++----
.../StlThumbnailProvider.cpp | 15 ++++--
.../StlThumbnailProvider.h | 2 +-
.../SvgThumbnailProvider/Program.cs | 7 ++-
.../SvgThumbnailProvider.cpp | 14 +++--
.../SvgThumbnailProvider.h | 2 +-
.../PdfThumbnailProviderTests.cs | 6 +--
16 files changed, 104 insertions(+), 73 deletions(-)
diff --git a/src/modules/previewpane/GcodeThumbnailProvider/GcodeThumbnailProvider.cs b/src/modules/previewpane/GcodeThumbnailProvider/GcodeThumbnailProvider.cs
index 09a3c98c27..cc30703e2b 100644
--- a/src/modules/previewpane/GcodeThumbnailProvider/GcodeThumbnailProvider.cs
+++ b/src/modules/previewpane/GcodeThumbnailProvider/GcodeThumbnailProvider.cs
@@ -159,12 +159,10 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Gcode
using (var reader = new StreamReader(this.Stream))
{
- using (Bitmap thumbnail = GetThumbnail(reader, cx))
+ Bitmap thumbnail = GetThumbnail(reader, cx);
+ if (thumbnail != null && thumbnail.Size.Width > 0 && thumbnail.Size.Height > 0)
{
- if (thumbnail != null && thumbnail.Size.Width > 0 && thumbnail.Size.Height > 0)
- {
- return (Bitmap)thumbnail.Clone();
- }
+ return thumbnail;
}
}
diff --git a/src/modules/previewpane/GcodeThumbnailProvider/Program.cs b/src/modules/previewpane/GcodeThumbnailProvider/Program.cs
index 8426ca7755..e1b920feb6 100644
--- a/src/modules/previewpane/GcodeThumbnailProvider/Program.cs
+++ b/src/modules/previewpane/GcodeThumbnailProvider/Program.cs
@@ -26,8 +26,11 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Gcode
_thumbnailProvider = new GcodeThumbnailProvider(filePath);
Bitmap thumbnail = _thumbnailProvider.GetThumbnail(cx);
- filePath = filePath.Replace(".gcode", ".bmp");
- thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ if (thumbnail != null)
+ {
+ filePath = filePath.Replace(".gcode", ".bmp");
+ thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ }
}
else
{
diff --git a/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.cpp b/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.cpp
index 7f16288d46..64e2a8067d 100644
--- a/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.cpp
+++ b/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.cpp
@@ -159,11 +159,19 @@ IFACEMETHODIMP GcodeThumbnailProvider::GetThumbnail(UINT cx, HBITMAP* phbmp, WTS
WaitForSingleObject(m_process, INFINITE);
std::filesystem::remove(fileName);
+
std::wstring fileNameBmp = filePath + guid + L".bmp";
- *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
- *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
-
- std::filesystem::remove(fileNameBmp);
+ if (std::filesystem::exists(fileNameBmp))
+ {
+ *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
+ *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
+ std::filesystem::remove(fileNameBmp);
+ }
+ else
+ {
+ Logger::info(L"Bmp file not generated.");
+ return E_FAIL;
+ }
}
catch (std::exception& e)
{
diff --git a/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.h b/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.h
index 8662de66e6..91dabda4ad 100644
--- a/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.h
+++ b/src/modules/previewpane/GcodeThumbnailProviderCpp/GcodeThumbnailProvider.h
@@ -19,7 +19,7 @@ public:
// IInitializeWithStream
IFACEMETHODIMP Initialize(IStream* pstream, DWORD grfMode);
- // IPreviewHandler
+ // IThumbnailProvider
IFACEMETHODIMP GetThumbnail(UINT cx, HBITMAP* phbmp, WTS_ALPHATYPE* pdwAlpha);
GcodeThumbnailProvider();
diff --git a/src/modules/previewpane/PdfThumbnailProvider/PdfThumbnailProvider.cs b/src/modules/previewpane/PdfThumbnailProvider/PdfThumbnailProvider.cs
index 0852369f96..e82607b299 100644
--- a/src/modules/previewpane/PdfThumbnailProvider/PdfThumbnailProvider.cs
+++ b/src/modules/previewpane/PdfThumbnailProvider/PdfThumbnailProvider.cs
@@ -1,14 +1,8 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
-using System;
-using System.Drawing;
-using System.IO;
-using System.Runtime.InteropServices;
-using System.Runtime.InteropServices.ComTypes;
-using Common.ComInterlop;
-using Common.Utilities;
using Windows.Data.Pdf;
+using Windows.Storage;
using Windows.Storage.Streams;
namespace Microsoft.PowerToys.ThumbnailHandler.Pdf
@@ -21,7 +15,6 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Pdf
public PdfThumbnailProvider(string filePath)
{
FilePath = filePath;
- Stream = new FileStream(filePath, FileMode.Open, FileAccess.Read);
}
///
@@ -29,11 +22,6 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Pdf
///
public string FilePath { get; private set; }
- ///
- /// Gets the stream object to access file.
- ///
- public Stream Stream { get; private set; }
-
///
/// The maximum dimension (width or height) thumbnail we will generate.
///
@@ -45,6 +33,16 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Pdf
/// Maximum thumbnail size, in pixels.
/// Generated bitmap
public Bitmap GetThumbnail(uint cx)
+ {
+ return DoGetThumbnail(cx).Result;
+ }
+
+ ///
+ /// Generate thumbnail bitmap for provided Pdf file/stream.
+ ///
+ /// Maximum thumbnail size, in pixels.
+ /// Generated bitmap
+ private async Task DoGetThumbnail(uint cx)
{
if (cx == 0 || cx > MaxThumbnailSize)
{
@@ -57,26 +55,27 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Pdf
return null;
}
- using var memStream = new MemoryStream();
-
- this.Stream.CopyTo(memStream);
- memStream.Position = 0;
-
- // AsRandomAccessStream() extension method from System.Runtime.WindowsRuntime
- var pdf = PdfDocument.LoadFromStreamAsync(memStream.AsRandomAccessStream()).GetAwaiter().GetResult();
-
- if (pdf.PageCount > 0)
+ Bitmap thumbnail = null;
+ try
{
- using var page = pdf.GetPage(0);
+ var file = await StorageFile.GetFileFromPathAsync(FilePath);
+ var pdf = await PdfDocument.LoadFromFileAsync(file);
- var image = PageToImage(page, cx);
+ if (pdf.PageCount > 0)
+ {
+ using var page = pdf.GetPage(0);
- using Bitmap thumbnail = new Bitmap(image);
+ var image = PageToImage(page, cx);
- return (Bitmap)thumbnail.Clone();
+ thumbnail = new Bitmap(image);
+ }
+ }
+ catch (Exception)
+ {
+ // TODO: add logger
}
- return null;
+ return thumbnail;
}
///
diff --git a/src/modules/previewpane/PdfThumbnailProvider/Program.cs b/src/modules/previewpane/PdfThumbnailProvider/Program.cs
index 59cf1acb99..5415a333ad 100644
--- a/src/modules/previewpane/PdfThumbnailProvider/Program.cs
+++ b/src/modules/previewpane/PdfThumbnailProvider/Program.cs
@@ -26,8 +26,11 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Pdf
_thumbnailProvider = new PdfThumbnailProvider(filePath);
Bitmap thumbnail = _thumbnailProvider.GetThumbnail(cx);
- filePath = filePath.Replace(".pdf", ".bmp");
- thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ if (thumbnail != null)
+ {
+ filePath = filePath.Replace(".pdf", ".bmp");
+ thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ }
}
else
{
diff --git a/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.cpp b/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.cpp
index 312512a454..d74fd9bfab 100644
--- a/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.cpp
+++ b/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.cpp
@@ -158,10 +158,18 @@ IFACEMETHODIMP PdfThumbnailProvider::GetThumbnail(UINT cx, HBITMAP* phbmp, WTS_A
std::filesystem::remove(fileName);
std::wstring fileNameBmp = filePath + guid + L".bmp";
- *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
- *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
+ if (std::filesystem::exists(fileNameBmp))
+ {
+ *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
+ *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
+ std::filesystem::remove(fileNameBmp);
+ }
+ else
+ {
+ Logger::info(L"Bmp file not generated.");
+ return E_FAIL;
+ }
- std::filesystem::remove(fileNameBmp);
}
catch (std::exception& e)
{
diff --git a/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.h b/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.h
index 9ae9b43445..c63d6d37bd 100644
--- a/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.h
+++ b/src/modules/previewpane/PdfThumbnailProviderCpp/PdfThumbnailProvider.h
@@ -19,7 +19,7 @@ public:
// IInitializeWithStream
IFACEMETHODIMP Initialize(IStream* pstream, DWORD grfMode);
- // IPreviewHandler
+ // IThumbnailProvider
IFACEMETHODIMP GetThumbnail(UINT cx, HBITMAP* phbmp, WTS_ALPHATYPE* pdwAlpha);
PdfThumbnailProvider();
diff --git a/src/modules/previewpane/StlThumbnailProvider/Program.cs b/src/modules/previewpane/StlThumbnailProvider/Program.cs
index 77e904ba1e..ce22471f67 100644
--- a/src/modules/previewpane/StlThumbnailProvider/Program.cs
+++ b/src/modules/previewpane/StlThumbnailProvider/Program.cs
@@ -25,8 +25,11 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Stl
_thumbnailProvider = new StlThumbnailProvider(filePath);
Bitmap thumbnail = _thumbnailProvider.GetThumbnail(cx);
- filePath = filePath.Replace(".stl", ".bmp");
- thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ if (thumbnail != null)
+ {
+ filePath = filePath.Replace(".stl", ".bmp");
+ thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ }
}
else
{
diff --git a/src/modules/previewpane/StlThumbnailProvider/StlThumbnailProvider.cs b/src/modules/previewpane/StlThumbnailProvider/StlThumbnailProvider.cs
index c7d4ebdd14..aeecf4599a 100644
--- a/src/modules/previewpane/StlThumbnailProvider/StlThumbnailProvider.cs
+++ b/src/modules/previewpane/StlThumbnailProvider/StlThumbnailProvider.cs
@@ -130,19 +130,10 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Stl
return null;
}
- using (var memStream = new MemoryStream())
+ Bitmap thumbnail = GetThumbnail(this.Stream, cx);
+ if (thumbnail != null && thumbnail.Size.Width > 0 && thumbnail.Size.Height > 0)
{
- this.Stream.CopyTo(memStream);
-
- memStream.Position = 0;
-
- using (Bitmap thumbnail = GetThumbnail(memStream, cx))
- {
- if (thumbnail != null && thumbnail.Size.Width > 0 && thumbnail.Size.Height > 0)
- {
- return (Bitmap)thumbnail.Clone();
- }
- }
+ return thumbnail;
}
return null;
diff --git a/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.cpp b/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.cpp
index eada9c4b06..b01d331ce7 100644
--- a/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.cpp
+++ b/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.cpp
@@ -158,10 +158,17 @@ IFACEMETHODIMP StlThumbnailProvider::GetThumbnail(UINT cx, HBITMAP* phbmp, WTS_A
std::filesystem::remove(fileName);
std::wstring fileNameBmp = filePath + guid + L".bmp";
- *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
- *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
-
- std::filesystem::remove(fileNameBmp);
+ if (std::filesystem::exists(fileNameBmp))
+ {
+ *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
+ *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
+ std::filesystem::remove(fileNameBmp);
+ }
+ else
+ {
+ Logger::info(L"Bmp file not generated.");
+ return E_FAIL;
+ }
}
catch (std::exception& e)
{
diff --git a/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.h b/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.h
index ea97d69c82..fe7c4c5119 100644
--- a/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.h
+++ b/src/modules/previewpane/StlThumbnailProviderCpp/StlThumbnailProvider.h
@@ -19,7 +19,7 @@ public:
// IInitializeWithStream
IFACEMETHODIMP Initialize(IStream* pstream, DWORD grfMode);
- // IPreviewHandler
+ // IThumbnailProvider
IFACEMETHODIMP GetThumbnail(UINT cx, HBITMAP* phbmp, WTS_ALPHATYPE* pdwAlpha);
StlThumbnailProvider();
diff --git a/src/modules/previewpane/SvgThumbnailProvider/Program.cs b/src/modules/previewpane/SvgThumbnailProvider/Program.cs
index f81a8f46ce..71172f0ac1 100644
--- a/src/modules/previewpane/SvgThumbnailProvider/Program.cs
+++ b/src/modules/previewpane/SvgThumbnailProvider/Program.cs
@@ -26,8 +26,11 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg
_thumbnailProvider = new SvgThumbnailProvider(filePath);
Bitmap thumbnail = _thumbnailProvider.GetThumbnail(cx);
- filePath = filePath.Replace(".svg", ".bmp");
- thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ if (thumbnail != null )
+ {
+ filePath = filePath.Replace(".svg", ".bmp");
+ thumbnail.Save(filePath, System.Drawing.Imaging.ImageFormat.Bmp);
+ }
}
else
{
diff --git a/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.cpp b/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.cpp
index 0f42b32f3b..7661095e32 100644
--- a/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.cpp
+++ b/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.cpp
@@ -158,10 +158,18 @@ IFACEMETHODIMP SvgThumbnailProvider::GetThumbnail(UINT cx, HBITMAP* phbmp, WTS_A
std::filesystem::remove(fileName);
std::wstring fileNameBmp = filePath + guid + L".bmp";
- *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
- *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
- std::filesystem::remove(fileNameBmp);
+ if (std::filesystem::exists(fileNameBmp))
+ {
+ *phbmp = (HBITMAP)LoadImage(NULL, fileNameBmp.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE);
+ *pdwAlpha = WTS_ALPHATYPE::WTSAT_ARGB;
+ std::filesystem::remove(fileNameBmp);
+ }
+ else
+ {
+ Logger::info(L"Bmp file not generated.");
+ return E_FAIL;
+ }
}
catch (std::exception& e)
{
diff --git a/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.h b/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.h
index 3c8a7f1b9c..b247dfa390 100644
--- a/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.h
+++ b/src/modules/previewpane/SvgThumbnailProviderCpp/SvgThumbnailProvider.h
@@ -19,7 +19,7 @@ public:
// IInitializeWithStream
IFACEMETHODIMP Initialize(IStream* pstream, DWORD grfMode);
- // IPreviewHandler
+ // IThumbnailProvider
IFACEMETHODIMP GetThumbnail(UINT cx, HBITMAP* phbmp, WTS_ALPHATYPE* pdwAlpha);
SvgThumbnailProvider();
diff --git a/src/modules/previewpane/UnitTests-PdfThumbnailProvider/PdfThumbnailProviderTests.cs b/src/modules/previewpane/UnitTests-PdfThumbnailProvider/PdfThumbnailProviderTests.cs
index a406c89ff5..635317e51e 100644
--- a/src/modules/previewpane/UnitTests-PdfThumbnailProvider/PdfThumbnailProviderTests.cs
+++ b/src/modules/previewpane/UnitTests-PdfThumbnailProvider/PdfThumbnailProviderTests.cs
@@ -22,7 +22,7 @@ namespace PdfThumbnailProviderUnitTests
public void GetThumbnailValidStreamPDF()
{
// Act
- var filePath = "HelperFiles/sample.pdf";
+ var filePath = System.IO.Path.GetFullPath("HelperFiles/sample.pdf");
PdfThumbnailProvider provider = new PdfThumbnailProvider(filePath);
@@ -35,7 +35,7 @@ namespace PdfThumbnailProviderUnitTests
public void GetThumbnailInValidSizePDF()
{
// Act
- var filePath = "HelperFiles/sample.pdf";
+ var filePath = System.IO.Path.GetFullPath("HelperFiles/sample.pdf");
PdfThumbnailProvider provider = new PdfThumbnailProvider(filePath);
@@ -48,7 +48,7 @@ namespace PdfThumbnailProviderUnitTests
public void GetThumbnailToBigPDF()
{
// Act
- var filePath = "HelperFiles/sample.pdf";
+ var filePath = System.IO.Path.GetFullPath("HelperFiles/sample.pdf");
PdfThumbnailProvider provider = new PdfThumbnailProvider(filePath);