Fixed dispose of chrome processes

Fixed dispose of chrome processes

.github/dependabot.yml

@@ -1,11 +1,18 @@
-# To get started with Dependabot version updates, you'll need to specify which
-# package ecosystems to update and where the package manifests are located.
-# Please see the documentation for all configuration options:
-# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates
-
 version: 2
 updates:
   - package-ecosystem: "nuget" # See documentation for possible values
     directory: "/" # Location of package manifests
     schedule:
       interval: "daily"
+    ignore:
+      - dependency-name: "nunit"
+      - dependency-name: "SonarAnalyzer.CSharp"
+      - dependency-name: "AngleSharp"
+      - dependency-name: "Microsoft.NET.Test.Sdk"
+      - dependency-name: "Microsoft.AspNetCore.Mvc.Testing"
+      - dependency-name: "Moq"
+      - dependency-name: "xunit"
+      - dependency-name: "xunit.runner.visualstudio"
+      - dependency-name: "MSTest.TestAdapter"
+      - dependency-name: "ImageSharpCompare"
+      - dependency-name: "PdfjsSharp"

PuppeteerSharp.Renderer/PuppeteerSharp.Renderer.csproj

@@ -24,7 +24,7 @@
     <SymbolPackageFormat>snupkg</SymbolPackageFormat>
     <PublishRepositoryUrl>true</PublishRepositoryUrl>
     <EmbedUntrackedSources>true</EmbedUntrackedSources>
-    <LangVersion>8.0</LangVersion>
+    <LangVersion>9.0</LangVersion>
     <Nullable>enable</Nullable>
     <PackageId>PuppeteerSharp.Renderer</PackageId>
     <Product>PuppeteerSharp.Renderer</Product>
@@ -36,7 +36,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="all" />
     <PackageReference Include="PuppeteerSharp" Version="4.0.0" />
-    <PackageReference Include="SonarAnalyzer.CSharp" Version="8.21.0.30542">
+    <PackageReference Include="SonarAnalyzer.CSharp" Version="8.22.0.31243">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>

PuppeteerSharp.Renderer/Renderer.cs

@@ -9,13 +9,13 @@ namespace Codeuctivity.PuppeteerSharp
     /// <summary>
     /// Renders HTML files
     /// </summary>
-    public class Renderer : IAsyncDisposable
+    public class Renderer : IAsyncDisposable, IDisposable
     {
         private Browser Browser { get; set; } = default!;
         private int LastProgressValue { get; set; }
 
         /// <summary>
-        /// Browserfetcher - used to get chromium bins
+        /// Browser fetcher - used to get chromium bins
         /// </summary>
         public BrowserFetcher BrowserFetcher { get; private set; } = default!;
 
@@ -54,7 +54,6 @@ private async Task<Renderer> InitializeAsync(BrowserFetcher browserFetcher)
         /// </summary>
         /// <param name="sourceHtmlFilePath"></param>
         /// <param name="destinationPdfFilePath"></param>
-        /// <returns></returns>
         public async Task ConvertHtmlToPdf(string sourceHtmlFilePath, string destinationPdfFilePath)
         {
             if (!File.Exists(sourceHtmlFilePath))
@@ -63,7 +62,7 @@ public async Task ConvertHtmlToPdf(string sourceHtmlFilePath, string destination
             }
 
             var absolutePath = Path.GetFullPath(sourceHtmlFilePath);
-            var page = await Browser.NewPageAsync().ConfigureAwait(false);
+            await using var page = await Browser.NewPageAsync().ConfigureAwait(false);
             await page.GoToAsync($"file://{absolutePath}").ConfigureAwait(false);
             await page.PdfAsync(destinationPdfFilePath).ConfigureAwait(false);
         }
@@ -73,7 +72,6 @@ public async Task ConvertHtmlToPdf(string sourceHtmlFilePath, string destination
         /// </summary>
         /// <param name="sourceHtmlFilePath"></param>
         /// <param name="destinationPngFilePath"></param>
-        /// <returns></returns>
         public async Task ConvertHtmlToPng(string sourceHtmlFilePath, string destinationPngFilePath)
         {
             if (!File.Exists(sourceHtmlFilePath))
@@ -82,9 +80,9 @@ public async Task ConvertHtmlToPng(string sourceHtmlFilePath, string destination
             }
 
             var absolutePath = Path.GetFullPath(sourceHtmlFilePath);
-            var page = await Browser.NewPageAsync().ConfigureAwait(false);
+            await using var page = await Browser.NewPageAsync().ConfigureAwait(false);
             await page.GoToAsync($"file://{absolutePath}").ConfigureAwait(false);
-            await page.ScreenshotAsync(destinationPngFilePath, new ScreenshotOptions() { FullPage = true }).ConfigureAwait(false);
+            await page.ScreenshotAsync(destinationPngFilePath, new ScreenshotOptions { FullPage = true }).ConfigureAwait(false);
         }
 
         private void DownloadProgressChanged(object sender, DownloadProgressChangedEventArgs e)
@@ -95,15 +93,50 @@ private void DownloadProgressChanged(object sender, DownloadProgressChangedEvent
             }
         }
 
-        ValueTask IAsyncDisposable.DisposeAsync()
+        /// <summary>
+        /// Dispose
+        /// </summary>
+        public void Dispose()
+        {
+            Dispose(disposing: true);
+            GC.SuppressFinalize(this);
+        }
+
+        /// <summary>
+        /// DisposeAsync
+        /// </summary>
+        public async ValueTask DisposeAsync()
         {
-            if (Browser == null)
+            await DisposeAsyncCore();
+
+            Dispose(disposing: false);
+#pragma warning disable CA1816 // Dispose methods should call SuppressFinalize
+            GC.SuppressFinalize(this);
+#pragma warning restore CA1816 // Dispose methods should call SuppressFinalize
+        }
+
+        /// <summary>
+        /// Dispose
+        /// </summary>
+        /// <param name="disposing"></param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
             {
-                return new ValueTask();
+                Browser?.Dispose();
             }
+        }
 
-            Browser.CloseAsync().ConfigureAwait(false);
-            return ((IAsyncDisposable)Browser).DisposeAsync();
+        /// <summary>
+        /// DisposeAsync
+        /// </summary>
+        protected virtual async ValueTask DisposeAsyncCore()
+        {
+            if (Browser is not null)
+            {
+                await Browser.CloseAsync();
+                await Browser.DisposeAsync();
+            }
         }
     }
 }

PuppeteerSharp.RendererCliTests/PuppeteerSharp.RendererCliTests.csproj

@@ -8,6 +8,10 @@
 
   <ItemGroup>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.9.4" />
+    <PackageReference Include="SonarAnalyzer.CSharp" Version="8.22.0.31243">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
     <PackageReference Include="xunit" Version="2.4.1" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

PuppeteerSharp.RendererCliTests/RendererCliTests.cs

@@ -11,7 +11,8 @@ public class RendererCliTests
         [Fact]
         public void VersionShouldBeProcessed()
         {
-            Version.TryParse(Program.Version.ToString(), out _);
+            var success = Version.TryParse(Program.Version.ToString(), out _);
+            Assert.True(success);
         }
 
         [Fact]

PuppeteerSharp.RendererTests/Infrastrukture/ChromiumProcessDisposedAsserter.cs

@@ -0,0 +1,25 @@
+using System.Diagnostics;
+using System.Linq;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace PuppeteerSharp.RendererTests.Infrastrukture
+{
+    public static class ChromiumProcessDisposedAsserter
+    {
+        public static async Task AssertNoChromeProcessIsRunning()
+        {
+            for (var i = 0; i < 20 && CountChromiumTasks() > 0; i++)
+            {
+                await Task.Delay(200);
+            }
+            Assert.Equal(0, CountChromiumTasks());
+        }
+
+        public static int CountChromiumTasks()
+        {
+            var processes = Process.GetProcesses().Where(process => process.ProcessName.Contains("chrome")).ToList();
+            return processes.Count;
+        }
+    }
+}

PuppeteerSharp.RendererTests/PuppeteerSharp.RendererTests.csproj

@@ -3,7 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>net5.0</TargetFrameworks>
     <IsPackable>false</IsPackable>
-    <LangVersion>8.0</LangVersion>
+    <LangVersion>9.0</LangVersion>
     <Nullable>enable</Nullable>
     <IsPackable>false</IsPackable>
     <EnableNETAnalyzers>true</EnableNETAnalyzers>
@@ -18,7 +18,7 @@
   <ItemGroup>
     <PackageReference Include="ImageSharpCompare" Version="1.1.34" />
     <PackageReference Include="PdfjsSharp" Version="1.0.50" />
-    <PackageReference Include="SonarAnalyzer.CSharp" Version="8.21.0.30542">
+    <PackageReference Include="SonarAnalyzer.CSharp" Version="8.22.0.31243">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>

PuppeteerSharp.RendererTests/RendererTests.cs

@@ -1,5 +1,6 @@
 using Codeuctivity.PdfjsSharp;
 using Codeuctivity.PuppeteerSharp;
+using PuppeteerSharp.RendererTests.Infrastrukture;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
@@ -22,16 +23,19 @@ public async Task ShouldConvertHtmlToPdf(string testFileName)
                 File.Delete(actualFilePath);
             }
 
-            await using var chromiumRenderer = await Renderer.CreateAsync();
-            await chromiumRenderer.ConvertHtmlToPdf(sourceHtmlFilePath, actualFilePath);
+            await using (var chromiumRenderer = await Renderer.CreateAsync())
+            {
+                await chromiumRenderer.ConvertHtmlToPdf(sourceHtmlFilePath, actualFilePath);
 
-            var actualImagePathDirectory = Path.Combine(Path.GetTempPath(), testFileName);
+                var actualImagePathDirectory = Path.Combine(Path.GetTempPath(), testFileName);
 
-            using var rasterize = new Rasterizer();
-            var actualImages = await rasterize.ConvertToPngAsync(actualFilePath, actualImagePathDirectory);
+                using var rasterize = new Rasterizer();
+                var actualImages = await rasterize.ConvertToPngAsync(actualFilePath, actualImagePathDirectory);
 
-            Assert.Single(actualImages);
-            DocumentAsserter.AssertImageIsEqual(actualImages.Single(), expectReferenceFilePath, 50);
+                Assert.Single(actualImages);
+                DocumentAsserter.AssertImageIsEqual(actualImages.Single(), expectReferenceFilePath, 50);
+            }
+            await ChromiumProcessDisposedAsserter.AssertNoChromeProcessIsRunning();
         }
 
         [Theory]
@@ -47,20 +51,27 @@ public async Task ShouldConvertHtmlToPng(string testFileName)
                 File.Delete(actualFilePath);
             }
 
-            await using var chromiumRenderer = await Renderer.CreateAsync();
+            await using (var chromiumRenderer = await Renderer.CreateAsync())
+            {
+                await chromiumRenderer.ConvertHtmlToPng(sourceHtmlFilePath, actualFilePath);
 
-            await chromiumRenderer.ConvertHtmlToPng(sourceHtmlFilePath, actualFilePath);
+                DocumentAsserter.AssertImageIsEqual(actualFilePath, expectReferenceFilePath, 3500);
+            }
 
-            DocumentAsserter.AssertImageIsEqual(actualFilePath, expectReferenceFilePath, 3100);
+            await ChromiumProcessDisposedAsserter.AssertNoChromeProcessIsRunning();
         }
 
         [Fact]
         public async Task ShouldDisposeGracefull()
         {
+            var initialChromiumTasks = ChromiumProcessDisposedAsserter.CountChromiumTasks();
+
             await using (var chromiumRenderer = new Renderer())
             {
                 Assert.Null(chromiumRenderer.BrowserFetcher);
             }
+            var afterDisposeChromiumTasks = ChromiumProcessDisposedAsserter.CountChromiumTasks();
+            Assert.Equal(afterDisposeChromiumTasks, initialChromiumTasks);
         }
     }
 }