From 9199eb6952a4569cf5be57b68732989651ca55e2 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 21 Jun 2021 12:10:47 -0500 Subject: [PATCH] Remove BitmapSelector.Suffix property (#54364) With https://github.com/dotnet/runtime/issues/22761 and https://github.com/dotnet/corefx/pull/22833, BitmapSelector.Suffix will always be null. This means this feature is dead code, and users are unable to use it. Removing this dead code because: 1. It doesn't do anything. 2. It is causing ILLink warnings, and it is easier to delete than to try to address the warnings. I logged https://github.com/dotnet/runtime/issues/54363 to follow up and either re-implement this feature, or obsolete the attributes that no longer have any effect on the app. --- .../src/ILLink/ILLink.Suppressions.xml | 14 +- .../src/System/Drawing/BitmapSelector.cs | 209 +----------------- .../System/Drawing/ToolboxBitmapAttribute.cs | 19 -- 3 files changed, 7 insertions(+), 235 deletions(-) diff --git a/src/libraries/System.Drawing.Common/src/ILLink/ILLink.Suppressions.xml b/src/libraries/System.Drawing.Common/src/ILLink/ILLink.Suppressions.xml index ec106d4867087..d5734aab1a2a9 100644 --- a/src/libraries/System.Drawing.Common/src/ILLink/ILLink.Suppressions.xml +++ b/src/libraries/System.Drawing.Common/src/ILLink/ILLink.Suppressions.xml @@ -1,18 +1,6 @@  - - ILLink - IL2026 - member - M:System.Drawing.BitmapSelector.SameAssemblyOptIn(System.Reflection.Assembly) - - - ILLink - IL2026 - member - M:System.Drawing.BitmapSelector.SatelliteAssemblyOptIn(System.Reflection.Assembly) - ILLink IL2050 @@ -80,4 +68,4 @@ M:System.Drawing.SafeNativeMethods.Gdip.GdipSaveImageToStream(System.Runtime.InteropServices.HandleRef,Interop.Ole32.IStream,System.Guid@,System.Runtime.InteropServices.HandleRef) - \ No newline at end of file + diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/BitmapSelector.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/BitmapSelector.cs index fcec880055ae2..b5daa033aa805 100644 --- a/src/libraries/System.Drawing.Common/src/System/Drawing/BitmapSelector.cs +++ b/src/libraries/System.Drawing.Common/src/System/Drawing/BitmapSelector.cs @@ -1,135 +1,16 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO; +using System.Reflection; + namespace System.Drawing { - using System.Diagnostics.CodeAnalysis; - using System.IO; - using System.Reflection; - /// - /// Provides methods to select from multiple bitmaps depending on a "bitmapSuffix" config setting. + /// Provides methods to select bitmaps. /// internal static class BitmapSelector { - /// - /// Gets the bitmap ID suffix defined in the application configuration, or string.Empty if - /// the suffix is not specified. Internal for unit tests - /// - /// - /// For performance, the suffix is cached in a static variable so it only has to be read - /// once per appdomain. - /// - private static string? s_suffix; - internal static string? Suffix - { - get - { - // NOTE: This value is read from the "SystemDrawingSection" of the ConfigurationManager on - // the .NET Framework. To avoid pulling in a direct dependency to that assembly, we are not - // reading the value in this implementation. - return s_suffix; - } - set - { - // So unit tests can clear the cached suffix - s_suffix = value; - } - } - - /// - /// Appends the current suffix to . The suffix is appended - /// before the existing extension (if any). Internal for unit tests. - /// - /// - /// The new path with the suffix included. If there is no suffix defined or there are - /// invalid characters in the original path, the original path is returned. - /// - internal static string AppendSuffix(string filePath) - { - try - { - return Path.ChangeExtension(filePath, Suffix + Path.GetExtension(filePath)); - } - catch (ArgumentException) - { // there are invalid characters in the path - return filePath; - } - } - - /// - /// Returns with the current suffix appended (before the - /// existing extension) if the resulting file path exists; otherwise the original path is - /// returned. - /// - public static string GetFileName(string originalPath) - { - if (string.IsNullOrEmpty(Suffix)) - return originalPath; - - string newPath = AppendSuffix(originalPath); - return File.Exists(newPath) ? newPath : originalPath; - } - - // Calls assembly.GetManifestResourceStream in a try/catch and returns null if not found - private static Stream? GetResourceStreamHelper(Assembly assembly, Type type, string name) - { - Stream? stream = null; - try - { - stream = assembly.GetManifestResourceStream(type, name); - } - catch (FileNotFoundException) - { - } - return stream; - } - - [RequiresUnreferencedCode("Calls Assembly.GetType which may be trimmed")] - private static bool DoesAssemblyHaveCustomAttribute(Assembly assembly, string typeName) - { - return DoesAssemblyHaveCustomAttribute(assembly, assembly.GetType(typeName)); - } - - private static bool DoesAssemblyHaveCustomAttribute(Assembly assembly, Type? attrType) - { - if (attrType != null) - { - var attr = assembly.GetCustomAttributes(attrType, false); - if (attr.Length > 0) - { - return true; - } - } - return false; - } - - // internal for unit tests - internal static bool SatelliteAssemblyOptIn(Assembly assembly) - { - // Try 4.5 public attribute type first - if (DoesAssemblyHaveCustomAttribute(assembly, typeof(BitmapSuffixInSatelliteAssemblyAttribute))) - { - return true; - } - - // Also load attribute type by name for dlls compiled against older frameworks - return DoesAssemblyHaveCustomAttribute(assembly, "System.Drawing.BitmapSuffixInSatelliteAssemblyAttribute"); - } - - // internal for unit tests - internal static bool SameAssemblyOptIn(Assembly assembly) - { - // Try 4.5 public attribute type first - if (DoesAssemblyHaveCustomAttribute(assembly, typeof(BitmapSuffixInSameAssemblyAttribute))) - { - return true; - } - - // Also load attribute type by name for dlls compiled against older frameworks - return DoesAssemblyHaveCustomAttribute(assembly, "System.Drawing.BitmapSuffixInSameAssemblyAttribute"); - } - /// /// Returns a resource stream loaded from the appropriate location according to the current /// suffix. @@ -138,57 +19,10 @@ internal static bool SameAssemblyOptIn(Assembly assembly) /// The type whose namespace is used to scope the manifest resource name /// The name of the manifest resource being requested /// - /// The manifest resource stream corresponding to with the - /// current suffix applied; or if that is not found, the stream corresponding to . + /// The manifest resource stream corresponding to . /// public static Stream? GetResourceStream(Assembly assembly, Type type, string originalName) { - if (Suffix != string.Empty) - { - try - { - // Resource with suffix has highest priority - if (SameAssemblyOptIn(assembly)) - { - string newName = AppendSuffix(originalName); - Stream? stream = GetResourceStreamHelper(assembly, type, newName); - if (stream != null) - { - return stream; - } - } - } - catch - { - // Ignore failures and continue to try other options - } - - try - { - // Satellite assembly has second priority, using the original name - if (SatelliteAssemblyOptIn(assembly)) - { - AssemblyName assemblyName = assembly.GetName(); - assemblyName.Name += Suffix; - assemblyName.ProcessorArchitecture = ProcessorArchitecture.None; - Assembly satellite = Assembly.Load(assemblyName); - if (satellite != null) - { - Stream? stream = GetResourceStreamHelper(satellite, type, originalName); - if (stream != null) - { - return stream; - } - } - } - } - catch - { - // Ignore failures and continue to try other options - } - } - - // Otherwise fall back to specified assembly and original name requested return assembly.GetManifestResourceStream(type, originalName); } @@ -199,42 +33,11 @@ internal static bool SameAssemblyOptIn(Assembly assembly) /// The type from whose assembly the stream is loaded and whose namespace is used to scope the resource name /// The name of the manifest resource being requested /// - /// The manifest resource stream corresponding to with the - /// current suffix applied; or if that is not found, the stream corresponding to . + /// The manifest resource stream corresponding to . /// public static Stream? GetResourceStream(Type type, string originalName) { return GetResourceStream(type.Module.Assembly, type, originalName); } - - /// - /// Returns an Icon created from a resource stream loaded from the appropriate location according to the current - /// suffix. - /// - /// The type from whose assembly the stream is loaded and whose namespace is used to scope the resource name - /// The name of the manifest resource being requested - /// - /// The icon created from a manifest resource stream corresponding to with the - /// current suffix applied; or if that is not found, the stream corresponding to . - /// - public static Icon CreateIcon(Type type, string originalName) - { - return new Icon(GetResourceStream(type, originalName)!); - } - - /// - /// Returns an Bitmap created from a resource stream loaded from the appropriate location according to the current - /// suffix. - /// - /// The type from whose assembly the stream is loaded and whose namespace is used to scope the resource name - /// The name of the manifest resource being requested - /// - /// The bitmap created from a manifest resource stream corresponding to with the - /// current suffix applied; or if that is not found, the stream corresponding to . - /// - public static Bitmap CreateBitmap(Type type, string originalName) - { - return new Bitmap(GetResourceStream(type, originalName)!); - } } } diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/ToolboxBitmapAttribute.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/ToolboxBitmapAttribute.cs index 2cc414e3e184a..aad75a27f3516 100644 --- a/src/libraries/System.Drawing.Common/src/System/Drawing/ToolboxBitmapAttribute.cs +++ b/src/libraries/System.Drawing.Common/src/System/Drawing/ToolboxBitmapAttribute.cs @@ -28,10 +28,6 @@ public class ToolboxBitmapAttribute : Attribute private static readonly Size s_largeSize = new Size(32, 32); private static readonly Size s_smallSize = new Size(16, 16); - // Used to help cache the last result of BitmapSelector.GetFileName. - private static string? s_lastOriginalFileName; - private static string? s_lastUpdatedFileName; - public ToolboxBitmapAttribute(string imageFile) : this(GetImageFromFile(imageFile, false), GetImageFromFile(imageFile, true)) { _imageFile = imageFile; @@ -168,19 +164,6 @@ public override bool Equals([NotNullWhen(true)] object? value) return b; } - // Cache the last result of BitmapSelector.GetFileName because we commonly load images twice - // in succession from the same file and we don't need to compute the name twice. - private static string? GetFileNameFromBitmapSelector(string originalName) - { - if (originalName != s_lastOriginalFileName) - { - s_lastOriginalFileName = originalName; - s_lastUpdatedFileName = BitmapSelector.GetFileName(originalName); - } - - return s_lastUpdatedFileName; - } - // Just forwards to Image.FromFile eating any non-critical exceptions that may result. private static Image? GetImageFromFile(string? imageFile, bool large, bool scaled = true) { @@ -189,8 +172,6 @@ public override bool Equals([NotNullWhen(true)] object? value) { if (imageFile != null) { - imageFile = GetFileNameFromBitmapSelector(imageFile); - string? ext = Path.GetExtension(imageFile); if (ext != null && string.Equals(ext, ".ico", StringComparison.OrdinalIgnoreCase)) {