[Thumbnail providers] Cover errors and edge cases (#23947)

* [Thumbnail providers] Cover errors and edge cases * Add TODO comment * Fix tests
microsoft · Feb 7, 2023 · d986d2e · d986d2e
1 parent 5e30d1e
commit d986d2e
Show file tree

Hide file tree

Showing 16 changed files with 104 additions and 73 deletions.
diff --git a/src/modules/previewpane/GcodeThumbnailProvider/GcodeThumbnailProvider.cs b/src/modules/previewpane/GcodeThumbnailProvider/GcodeThumbnailProvider.cs
@@ -159,12 +159,10 @@ public Bitmap GetThumbnail(uint cx)
 
             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
@@ -26,8 +26,11 @@ public static void Main(string[] args)
 
                     _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
@@ -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
@@ -19,7 +19,7 @@ class GcodeThumbnailProvider :
     // 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
@@ -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,19 +15,13 @@ public class PdfThumbnailProvider
         public PdfThumbnailProvider(string filePath)
         {
             FilePath = filePath;
-            Stream = new FileStream(filePath, FileMode.Open, FileAccess.Read);
         }
 
         /// <summary>
         /// Gets the file path to the file creating thumbnail for.
         /// </summary>
         public string FilePath { get; private set; }
 
-        /// <summary>
-        /// Gets the stream object to access file.
-        /// </summary>
-        public Stream Stream { get; private set; }
-
         /// <summary>
         ///  The maximum dimension (width or height) thumbnail we will generate.
         /// </summary>
@@ -45,6 +33,16 @@ public PdfThumbnailProvider(string filePath)
         /// <param name="cx">Maximum thumbnail size, in pixels.</param>
         /// <returns>Generated bitmap</returns>
         public Bitmap GetThumbnail(uint cx)
+        {
+            return DoGetThumbnail(cx).Result;
+        }
+
+        /// <summary>
+        /// Generate thumbnail bitmap for provided Pdf file/stream.
+        /// </summary>
+        /// <param name="cx">Maximum thumbnail size, in pixels.</param>
+        /// <returns>Generated bitmap</returns>
+        private async Task<Bitmap> DoGetThumbnail(uint cx)
         {
             if (cx == 0 || cx > MaxThumbnailSize)
             {
@@ -57,26 +55,27 @@ public Bitmap GetThumbnail(uint cx)
                 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;
         }
 
         /// <summary>

diff --git a/src/modules/previewpane/PdfThumbnailProvider/Program.cs b/src/modules/previewpane/PdfThumbnailProvider/Program.cs
@@ -26,8 +26,11 @@ public static void Main(string[] args)
 
                     _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
@@ -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
@@ -19,7 +19,7 @@ class PdfThumbnailProvider :
     // 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
@@ -25,8 +25,11 @@ public static void Main(string[] args)
 
                     _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
@@ -130,19 +130,10 @@ public Bitmap GetThumbnail(uint cx)
                 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
@@ -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
@@ -19,7 +19,7 @@ class StlThumbnailProvider :
     // 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
@@ -26,8 +26,11 @@ public static void Main(string[] args)
 
                     _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
@@ -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
@@ -19,7 +19,7 @@ class SvgThumbnailProvider :
     // 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
@@ -22,7 +22,7 @@ public class PdfThumbnailProviderTests
         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 @@ public void GetThumbnailValidStreamPDF()
         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 @@ public void GetThumbnailInValidSizePDF()
         public void GetThumbnailToBigPDF()
         {
             // Act
-            var filePath = "HelperFiles/sample.pdf";
+            var filePath = System.IO.Path.GetFullPath("HelperFiles/sample.pdf");
 
             PdfThumbnailProvider provider = new PdfThumbnailProvider(filePath);