Skip to content

Commit

Permalink
Remove BitmapSelector.Suffix property (#54364)
Browse files Browse the repository at this point in the history 
With #22761 and dotnet/corefx#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.
  • Loading branch information
@eerhardt
eerhardt committed Jun 21, 2021
1 parent dbcb387 commit 9199eb6
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 235 deletions.
14 changes: 1 addition & 13 deletions src/libraries/System.Drawing.Common/src/ILLink/ILLink.Suppressions.xml
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<linker>
<assembly fullname="System.Drawing.Common, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51">
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Drawing.BitmapSelector.SameAssemblyOptIn(System.Reflection.Assembly)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2026</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Drawing.BitmapSelector.SatelliteAssemblyOptIn(System.Reflection.Assembly)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2050</argument>
Expand Down Expand Up @@ -80,4 +68,4 @@
<property name="Target">M:System.Drawing.SafeNativeMethods.Gdip.GdipSaveImageToStream(System.Runtime.InteropServices.HandleRef,Interop.Ole32.IStream,System.Guid@,System.Runtime.InteropServices.HandleRef)</property>
</attribute>
</assembly>
</linker>
</linker>
209 changes: 6 additions & 203 deletions src/libraries/System.Drawing.Common/src/System/Drawing/BitmapSelector.cs
View file
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// Provides methods to select from multiple bitmaps depending on a "bitmapSuffix" config setting.
/// Provides methods to select bitmaps.
/// </summary>
internal static class BitmapSelector
{
/// <summary>
/// Gets the bitmap ID suffix defined in the application configuration, or string.Empty if
/// the suffix is not specified. Internal for unit tests
/// </summary>
/// <remarks>
/// For performance, the suffix is cached in a static variable so it only has to be read
/// once per appdomain.
/// </remarks>
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;
}
}

/// <summary>
/// Appends the current suffix to <paramref name="filePath"/>. The suffix is appended
/// before the existing extension (if any). Internal for unit tests.
/// </summary>
/// <returns>
/// 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.
/// </returns>
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;
}
}

/// <summary>
/// Returns <paramref name="originalPath"/> with the current suffix appended (before the
/// existing extension) if the resulting file path exists; otherwise the original path is
/// returned.
/// </summary>
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");
}

/// <summary>
/// Returns a resource stream loaded from the appropriate location according to the current
/// suffix.
Expand All @@ -138,57 +19,10 @@ internal static bool SameAssemblyOptIn(Assembly assembly)
/// <param name="type">The type whose namespace is used to scope the manifest resource name</param>
/// <param name="originalName">The name of the manifest resource being requested</param>
/// <returns>
/// The manifest resource stream corresponding to <paramref name="originalName"/> with the
/// current suffix applied; or if that is not found, the stream corresponding to <paramref name="originalName"/>.
/// The manifest resource stream corresponding to <paramref name="originalName"/>.
/// </returns>
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);
}

Expand All @@ -199,42 +33,11 @@ internal static bool SameAssemblyOptIn(Assembly assembly)
/// <param name="type">The type from whose assembly the stream is loaded and whose namespace is used to scope the resource name</param>
/// <param name="originalName">The name of the manifest resource being requested</param>
/// <returns>
/// The manifest resource stream corresponding to <paramref name="originalName"/> with the
/// current suffix applied; or if that is not found, the stream corresponding to <paramref name="originalName"/>.
/// The manifest resource stream corresponding to <paramref name="originalName"/>.
/// </returns>
public static Stream? GetResourceStream(Type type, string originalName)
{
return GetResourceStream(type.Module.Assembly, type, originalName);
}

/// <summary>
/// Returns an Icon created from a resource stream loaded from the appropriate location according to the current
/// suffix.
/// </summary>
/// <param name="type">The type from whose assembly the stream is loaded and whose namespace is used to scope the resource name</param>
/// <param name="originalName">The name of the manifest resource being requested</param>
/// <returns>
/// The icon created from a manifest resource stream corresponding to <paramref name="originalName"/> with the
/// current suffix applied; or if that is not found, the stream corresponding to <paramref name="originalName"/>.
/// </returns>
public static Icon CreateIcon(Type type, string originalName)
{
return new Icon(GetResourceStream(type, originalName)!);
}

/// <summary>
/// Returns an Bitmap created from a resource stream loaded from the appropriate location according to the current
/// suffix.
/// </summary>
/// <param name="type">The type from whose assembly the stream is loaded and whose namespace is used to scope the resource name</param>
/// <param name="originalName">The name of the manifest resource being requested</param>
/// <returns>
/// The bitmap created from a manifest resource stream corresponding to <paramref name="originalName"/> with the
/// current suffix applied; or if that is not found, the stream corresponding to <paramref name="originalName"/>.
/// </returns>
public static Bitmap CreateBitmap(Type type, string originalName)
{
return new Bitmap(GetResourceStream(type, originalName)!);
}
}
}
19 changes: 0 additions & 19 deletions src/libraries/System.Drawing.Common/src/System/Drawing/ToolboxBitmapAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand All @@ -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))
{
Expand Down

0 comments on commit 9199eb6

Please sign in to comment.