First round of converting System.Drawing.Common to COMWrappers (#54636)

* First round of converting System.Drawing.Common to COMWrappers Using COM Wrappers makes the library trim compatible. * Add Trimming Test for Icon.Save * Add support for OS specific trimming tests * Use function pointers instead of delegates * Rename Guid to IID * Better interop to closely match the native side * Release any COM pointer that was QueryInterface * Use pointers instead of Marshal.PtrToStructure/StructureToPtr * No need for a VTable struct, just set each function pointer right into the table * Wrap all managed calls in try-catch and return HResult * Use COM naming * Fix method signature to use pointer instead of out. * CheckStatus => ThrowExceptionForHR * Pass -1 to Marshal.GetExceptionForHR so it doesn't query GetErrorInfo, and always returns the correct exception type * Create the PictureWrapper with UniqueInstance, so it doesn't get cached. Caching it causes lifetime issues.
dotnet · Jun 28, 2021 · 8707275 · 8707275
1 parent d0adff8
commit 8707275
Show file tree

Hide file tree

Showing 15 changed files with 603 additions and 123 deletions.
diff --git a/eng/testing/linker/project.csproj.template b/eng/testing/linker/project.csproj.template
@@ -8,7 +8,7 @@
     <WasmAppBuilderTasksAssemblyPath>{WasmAppBuilderTasksAssemblyPath}</WasmAppBuilderTasksAssemblyPath>
     <MicrosoftNetCoreAppRuntimePackRidDir>{MicrosoftNetCoreAppRuntimePackRidDir}</MicrosoftNetCoreAppRuntimePackRidDir>
     <RepositoryEngineeringDir>{RepositoryEngineeringDir}</RepositoryEngineeringDir>
-    <TargetFramework>{NetCoreAppCurrent}</TargetFramework>
+    <TargetFramework>{TestTargetFramework}</TargetFramework>
     <RuntimeIdentifier>{RuntimeIdentifier}</RuntimeIdentifier>
     <UseMonoRuntime>{UseMonoRuntime}</UseMonoRuntime>
     <TargetingPackDir>{TargetingPackDir}</TargetingPackDir>

diff --git a/eng/testing/linker/trimmingTests.targets b/eng/testing/linker/trimmingTests.targets
@@ -25,14 +25,16 @@
       <TestConsoleAppSourceFiles>
         <ProjectDir>$([MSBuild]::NormalizeDirectory('$(TrimmingTestProjectsDir)', '$(MSBuildProjectName)', '%(Filename)', '$(PackageRID)'))</ProjectDir>
         <TestRuntimeIdentifier>$(PackageRID)</TestRuntimeIdentifier>
+        <TestTargetFramework>$(NetCoreAppCurrent)</TestTargetFramework>
+        <TestTargetFramework Condition="'%(TestConsoleAppSourceFiles.TargetOS)' != ''">$(NetCoreAppCurrent)-%(TestConsoleAppSourceFiles.TargetOS)</TestTargetFramework>
       </TestConsoleAppSourceFiles>
-      <!-- We need to separate Item metadata declaration in two in order to be able to use ProjectDir and TestRuntimeIdentifier bellow -->
+      <!-- We need to separate Item metadata declaration in two in order to be able to use ProjectDir and TestRuntimeIdentifier below -->
       <TestConsoleAppSourceFiles>
         <ProjectFile>%(ProjectDir)project.csproj</ProjectFile>
-        <TestCommand Condition="'$(TargetArchitecture)' != 'wasm' Or '$(TargetOs)' != 'browser'">$([MSBuild]::NormalizePath('%(ProjectDir)', 'bin', '$(Configuration)', '$(NetCoreAppCurrent)', '%(TestRuntimeIdentifier)', 'publish', 'project'))</TestCommand>
-        <TestCommand Condition="'$(TargetArchitecture)' == 'wasm' And '$(TargetOs)' == 'browser'">$([MSBuild]::NormalizePath('%(ProjectDir)', 'bin', '$(Configuration)', '$(NetCoreAppCurrent)', '%(TestRuntimeIdentifier)', 'AppBundle', 'run-v8.sh'))</TestCommand>
-        <TestExecutionDirectory Condition="'$(TargetArchitecture)' != 'wasm' Or '$(TargetOs)' != 'browser'">$([MSBuild]::NormalizeDirectory('%(ProjectDir)', 'bin', '$(Configuration)', '$(NetCoreAppCurrent)', '%(TestRuntimeIdentifier)', 'publish'))</TestExecutionDirectory>
-        <TestExecutionDirectory Condition="'$(TargetArchitecture)' == 'wasm' And '$(TargetOs)' == 'browser'">$([MSBuild]::NormalizeDirectory('%(ProjectDir)', 'bin', '$(Configuration)', '$(NetCoreAppCurrent)', '%(TestRuntimeIdentifier)', 'AppBundle'))</TestExecutionDirectory>
+        <TestCommand Condition="'$(TargetArchitecture)' != 'wasm' Or '$(TargetOs)' != 'browser'">$([MSBuild]::NormalizePath('%(ProjectDir)', 'bin', '$(Configuration)', '%(TestTargetFramework)', '%(TestRuntimeIdentifier)', 'publish', 'project'))</TestCommand>
+        <TestCommand Condition="'$(TargetArchitecture)' == 'wasm' And '$(TargetOs)' == 'browser'">$([MSBuild]::NormalizePath('%(ProjectDir)', 'bin', '$(Configuration)', '%(TestTargetFramework)', '%(TestRuntimeIdentifier)', 'AppBundle', 'run-v8.sh'))</TestCommand>
+        <TestExecutionDirectory Condition="'$(TargetArchitecture)' != 'wasm' Or '$(TargetOs)' != 'browser'">$([MSBuild]::NormalizeDirectory('%(ProjectDir)', 'bin', '$(Configuration)', '%(TestTargetFramework)', '%(TestRuntimeIdentifier)', 'publish'))</TestExecutionDirectory>
+        <TestExecutionDirectory Condition="'$(TargetArchitecture)' == 'wasm' And '$(TargetOs)' == 'browser'">$([MSBuild]::NormalizeDirectory('%(ProjectDir)', 'bin', '$(Configuration)', '%(TestTargetFramework)', '%(TestRuntimeIdentifier)', 'AppBundle'))</TestExecutionDirectory>
       </TestConsoleAppSourceFiles>
     </ItemGroup>
 
@@ -70,7 +72,7 @@
     <WriteLinesToFile File="$(_projectFile)"
                       Lines="$([System.IO.File]::ReadAllText('$(ProjectTemplate)')
                                                  .Replace('{MicrosoftNETCoreAppVersion}', '$(MicrosoftNETCoreAppVersion)')
-                                                 .Replace('{NetCoreAppCurrent}', '$(NetCoreAppCurrent)')
+                                                 .Replace('{TestTargetFramework}', '%(TestConsoleApps.TestTargetFramework)')
                                                  .Replace('{NetCoreAppMaximumVersion}', '$(NetCoreAppMaximumVersion)')
                                                  .Replace('{TargetingPackDir}','$(MicrosoftNetCoreAppRefPackDir)')
                                                  .Replace('{RuntimeIdentifier}','%(TestConsoleApps.TestRuntimeIdentifier)')

diff --git a/src/libraries/Common/src/Interop/Windows/Interop.HRESULT.cs b/src/libraries/Common/src/Interop/Windows/Interop.HRESULT.cs
@@ -13,6 +13,7 @@ internal enum HRESULT : int
         E_FAIL = unchecked((int)0x80004005),
         E_UNEXPECTED = unchecked((int)0x8000FFFF),
         STG_E_INVALIDFUNCTION = unchecked((int)0x80030001L),
+        STG_E_INVALIDPOINTER = unchecked((int)0x80030009),
         STG_E_INVALIDPARAMETER = unchecked((int)0x80030057),
         STG_E_INVALIDFLAG = unchecked((int)0x800300FF),
         E_ACCESSDENIED = unchecked((int)0x80070005),

diff --git a/src/libraries/Common/src/Interop/Windows/Ole32/Interop.IStream.cs b/src/libraries/Common/src/Interop/Windows/Ole32/Interop.IStream.cs
@@ -56,8 +56,8 @@ HRESULT UnlockRegion(
                 ulong cb,
                 uint dwLockType);
 
-            void Stat(
-                out STATSTG pstatstg,
+            unsafe void Stat(
+                STATSTG* pstatstg,
                 STATFLAG grfStatFlag);
 
             IStream Clone();

diff --git a/src/libraries/Common/src/Interop/Windows/Ole32/Interop.STATFLAG.cs b/src/libraries/Common/src/Interop/Windows/Ole32/Interop.STATFLAG.cs
@@ -6,7 +6,7 @@ internal static partial class Interop
     internal static partial class Ole32
     {
         /// <summary>
-        /// Stat flags for <see cref="IStream.Stat(out STATSTG, STATFLAG)"/>.
+        /// Stat flags for <see cref="IStream.Stat(STATSTG*, STATFLAG)"/>.
         /// <see href="https://docs.microsoft.com/en-us/windows/desktop/api/wtypes/ne-wtypes-tagstatflag"/>
         /// </summary>
         internal enum STATFLAG : uint

diff --git a/src/libraries/System.Drawing.Common/src/ILLink/ILLink.Suppressions.xml b/src/libraries/System.Drawing.Common/src/ILLink/ILLink.Suppressions.xml
@@ -7,12 +7,6 @@
       <property name="Scope">member</property>
       <property name="Target">M:System.Drawing.Bitmap.#ctor(System.IO.Stream,System.Boolean)</property>
     </attribute>
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2050</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.Drawing.Icon.Save(System.IO.Stream)</property>
-    </attribute>
     <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
       <argument>ILLink</argument>
       <argument>IL2050</argument>

diff --git a/src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj b/src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj
@@ -317,6 +317,13 @@
     <Compile Include="$(CommonPath)Interop\Windows\Interop.HRESULT.cs"
              Link="Common\Interop\Windows\Interop.HRESULT.cs" />
   </ItemGroup>
+  <ItemGroup Condition="'$(TargetsWindows)' == 'true' and $([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net5.0'))">
+    <Compile Include="System\Drawing\Icon.Windows.COMWrappers.cs" />
+    <Compile Include="System\Drawing\DrawingComWrappers.cs" />
+  </ItemGroup>
+  <ItemGroup Condition="'$(TargetsWindows)' == 'true' and !$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net5.0'))">
+    <Compile Include="System\Drawing\Icon.Windows.NoCOMWrappers.cs" />
+  </ItemGroup>
   <!-- Unix specific -->
   <ItemGroup Condition="'$(TargetsUnix)' == 'true'">
     <Compile Include="System\Drawing\Graphics.Unix.cs" />