Skip to content

Commit

Permalink
Property Store Refactor (#11779)
Browse files Browse the repository at this point in the history 
* Refactor PropertyStore GetSize/SetSize

* Remove GetColor/SetColor

* Refactor ContainsInteger usage

* Refactor RemoveInteger usage

* Refactor PropertyStore.RemoveObject usage

* Refactor PropertyStore.GetInteger

* Changes from review

* fix GetWrapContents

* Fix GetWrapContents and simplify ShouldSerializeSelectionForeColor
  • Loading branch information
@elachlan
elachlan authored Aug 1, 2024
1 parent 363f9cf commit 7446e03
Show file tree
Hide file tree
Showing 11 changed files with 152 additions and 258 deletions.
4 changes: 2 additions & 2 deletions src/System.Windows.Forms/src/System/Windows/Forms/Control.Ime.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,14 @@ internal int ImeWmCharsToIgnore
// after all messages are sent, corresponding WM_CHAR messages are also sent. (in non-unicode
// windows two WM_CHAR messages are sent per char in the IME). We need to keep a counter
// not to process each character twice or more.
get => Properties.GetInteger(s_imeWmCharsToIgnoreProperty);
get => Properties.GetValueOrDefault<int>(s_imeWmCharsToIgnoreProperty);
set
{
// WM_CHAR is not send after WM_IME_CHAR when the composition has been closed by either, changing the conversion mode or
// dissociating the IME (for instance when loosing focus and conversion is forced to complete).
if (ImeWmCharsToIgnore != ImeCharsToIgnoreDisabled)
{
Properties.SetInteger(s_imeWmCharsToIgnoreProperty, value);
Properties.AddValue(s_imeWmCharsToIgnoreProperty, value);
}
}
}
Expand Down
42 changes: 21 additions & 21 deletions src/System.Windows.Forms/src/System/Windows/Forms/Control.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,14 +758,14 @@ public virtual object? DataContext

// When DataContext was different than its parent before, but now it is about to become the same,
// we're removing it altogether, so it can inherit the value from its parent.
if (Properties.ContainsObject(s_dataContextProperty) && Equals(ParentInternal?.DataContext, value))
if (Properties.ContainsKey(s_dataContextProperty) && Equals(ParentInternal?.DataContext, value))
{
Properties.RemoveObject(s_dataContextProperty);
Properties.RemoveValue(s_dataContextProperty);
OnDataContextChanged(EventArgs.Empty);
return;
}

Properties.SetObject(s_dataContextProperty, value);
Properties.AddValue(s_dataContextProperty, value);
OnDataContextChanged(EventArgs.Empty);
}
}
Expand All @@ -774,7 +774,7 @@ private bool ShouldSerializeDataContext()
=> Properties.ContainsObject(s_dataContextProperty);

private void ResetDataContext()
=> Properties.RemoveObject(s_dataContextProperty);
=> Properties.RemoveValue(s_dataContextProperty);

/// <summary>
/// The background color of this control. This is an ambient property and
Expand Down Expand Up @@ -834,9 +834,9 @@ public virtual Color BackColor
}

Color c = BackColor;
if (!value.IsEmpty || Properties.ContainsObject(s_backColorProperty))
if (!value.IsEmpty || Properties.ContainsKey(s_backColorProperty))
{
Properties.SetColor(s_backColorProperty, value);
Properties.AddValue(s_backColorProperty, value);
}

if (!c.Equals(BackColor))
Expand Down Expand Up @@ -2095,7 +2095,7 @@ public virtual Color ForeColor
{
get
{
Color color = Properties.GetColor(s_foreColorProperty);
Properties.TryGetValue(s_foreColorProperty, out Color color);
if (!color.IsEmpty)
{
return color;
Expand Down Expand Up @@ -2125,9 +2125,9 @@ public virtual Color ForeColor
set
{
Color color = ForeColor;
if (!value.IsEmpty || Properties.ContainsObject(s_foreColorProperty))
if (!value.IsEmpty || Properties.ContainsKey(s_foreColorProperty))
{
Properties.SetColor(s_foreColorProperty, value);
Properties.AddValue(s_foreColorProperty, value);
}

if (!color.Equals(ForeColor))
Expand Down Expand Up @@ -2794,7 +2794,7 @@ internal virtual Control? ParentInternal
internal PropertyStore Properties { get; }

// Returns the value of the backColor field -- no asking the parent with its color is, etc.
internal Color RawBackColor => Properties.GetColor(s_backColorProperty);
internal Color RawBackColor => Properties.GetValueOrDefault<Color>(s_backColorProperty);

/// <summary>
/// Indicates whether the control is currently recreating its handle. This
Expand Down Expand Up @@ -6976,9 +6976,9 @@ protected virtual void OnFontChanged(EventArgs e)

Invalidate();

if (Properties.ContainsInteger(s_fontHeightProperty))
if (Properties.ContainsKey(s_fontHeightProperty))
{
Properties.SetInteger(s_fontHeightProperty, -1);
Properties.AddValue(s_fontHeightProperty, -1);
}

// Cleanup any font handle wrapper.
Expand Down Expand Up @@ -7086,7 +7086,7 @@ protected virtual void OnNotifyMessage(Message m)
[EditorBrowsable(EditorBrowsableState.Advanced)]
protected virtual void OnParentBackColorChanged(EventArgs e)
{
Color backColor = Properties.GetColor(s_backColorProperty);
Color backColor = Properties.GetValueOrDefault<Color>(s_backColorProperty);
if (backColor.IsEmpty)
{
OnBackColorChanged(e);
Expand Down Expand Up @@ -7123,10 +7123,10 @@ protected virtual void OnParentDataContextChanged(EventArgs e)
if (Properties.ContainsObject(s_dataContextProperty))
{
// If this DataContext was the same as the Parent's just became,
if (Equals(Properties.GetObject(s_dataContextProperty), Parent?.DataContext))
if (Equals(Properties.GetValueOrDefault<object>(s_dataContextProperty), Parent?.DataContext))
{
// we need to make it ambient again by removing it.
Properties.RemoveObject(s_dataContextProperty);
Properties.RemoveValue(s_dataContextProperty);

// Even though internally we don't store it any longer, and the
// value we had stored therefore changed, technically the value
Expand Down Expand Up @@ -7209,7 +7209,7 @@ internal virtual void OnParentHandleRecreating()
// Move this control over to the parking window.

// If we left it parented to the parent control, DestroyWindow would force us to destroy our handle as well.
// Temporarilty parenting to the parking window avoids having to recreate our handle from scratch.
// Temporarily parenting to the parking window avoids having to recreate our handle from scratch.

if (IsHandleCreated)
{
Expand All @@ -7220,7 +7220,7 @@ internal virtual void OnParentHandleRecreating()
[EditorBrowsable(EditorBrowsableState.Advanced)]
protected virtual void OnParentForeColorChanged(EventArgs e)
{
Color foreColor = Properties.GetColor(s_foreColorProperty);
Color foreColor = Properties.GetValueOrDefault<Color>(s_foreColorProperty);
if (foreColor.IsEmpty)
{
OnForeColorChanged(e);
Expand Down Expand Up @@ -10686,7 +10686,7 @@ internal virtual bool ShouldPerformContainerValidation()
[EditorBrowsable(EditorBrowsableState.Never)]
internal virtual bool ShouldSerializeBackColor()
{
Color backColor = Properties.GetColor(s_backColorProperty);
Color backColor = Properties.GetValueOrDefault<Color>(s_backColorProperty);
return !backColor.IsEmpty;
}

Expand Down Expand Up @@ -10714,7 +10714,7 @@ private bool ShouldSerializeEnabled()
[EditorBrowsable(EditorBrowsableState.Never)]
internal virtual bool ShouldSerializeForeColor()
{
Color foreColor = Properties.GetColor(s_foreColorProperty);
Color foreColor = Properties.GetValueOrDefault<Color>(s_foreColorProperty);
return !foreColor.IsEmpty;
}

Expand Down Expand Up @@ -10936,9 +10936,9 @@ private protected void SetScaledFont(Font scaledFont, bool raiseOnFontChangedEve
// Dispose old FontHandle.
DisposeFontHandle();

if (Properties.ContainsInteger(s_fontHeightProperty))
if (Properties.ContainsKey(s_fontHeightProperty))
{
Properties.SetInteger(s_fontHeightProperty, scaledFont.Height);
Properties.AddValue(s_fontHeightProperty, scaledFont.Height);
}

if (!raiseOnFontChangedEvent)
Expand Down
63 changes: 20 additions & 43 deletions src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -447,18 +447,14 @@ public DrawMode DrawMode
[SRDescription(nameof(SR.ComboBoxDropDownWidthDescr))]
public int DropDownWidth
{
get
{
int dropDownWidth = Properties.GetInteger(s_propDropDownWidth, out bool found);
return found ? dropDownWidth : Width;
}
get => Properties.TryGetValue(s_propDropDownWidth, out int dropDownWidth) ? dropDownWidth : Width;
set
{
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);

if (Properties.GetInteger(s_propDropDownWidth) != value)
if (Properties.GetValueOrDefault<int>(s_propDropDownWidth) != value)
{
Properties.SetInteger(s_propDropDownWidth, value);
Properties.AddValue(s_propDropDownWidth, value);
if (IsHandleCreated)
{
PInvoke.SendMessage(this, PInvoke.CB_SETDROPPEDWIDTH, (WPARAM)value);
Expand All @@ -477,35 +473,24 @@ public int DropDownWidth
[DefaultValue(106)]
public int DropDownHeight
{
get
{
int dropDownHeight = Properties.GetInteger(s_propDropDownHeight, out bool found);
if (found)
{
return dropDownHeight;
}
else
{
return DefaultDropDownHeight;
}
}
get => Properties.TryGetValue(s_propDropDownHeight, out int dropDownHeight) ? dropDownHeight : DefaultDropDownHeight;
set
{
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);

if (Properties.GetInteger(s_propDropDownHeight) != value)
if (Properties.GetValueOrDefault<int>(s_propDropDownHeight) != value)
{
Properties.SetInteger(s_propDropDownHeight, value);
Properties.AddValue(s_propDropDownHeight, value);

// The dropDownHeight is not reflected unless the
// combobox integralHeight == false..
// ComboBox integralHeight == false..
IntegralHeight = false;
}
}
}

/// <summary>
/// Indicates whether the DropDown of the combo is currently dropped down.
/// Indicates whether the DropDown of the combo is currently dropped down.
/// </summary>
[Browsable(false)]
[DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]
Expand Down Expand Up @@ -636,37 +621,29 @@ public int ItemHeight
drawMode == DrawMode.OwnerDrawVariable ||
!IsHandleCreated)
{
int itemHeight = Properties.GetInteger(s_propItemHeight, out bool found);
if (found)
{
return itemHeight;
}
else
{
return FontHeight + 2;
}
return Properties.TryGetValue(s_propItemHeight, out int itemHeight) ? itemHeight : FontHeight + 2;
}

// Note that the above if clause deals with the case when the handle has not yet been created
Debug.Assert(IsHandleCreated, "Handle should be created at this point");

int h = (int)PInvoke.SendMessage(this, PInvoke.CB_GETITEMHEIGHT);
if (h == -1)
int height = (int)PInvoke.SendMessage(this, PInvoke.CB_GETITEMHEIGHT);
if (height == -1)
{
throw new Win32Exception();
}

return h;
return height;
}
set
{
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);

ResetHeightCache();

if (Properties.GetInteger(s_propItemHeight) != value)
if (Properties.GetValueOrDefault<int>(s_propItemHeight) != value)
{
Properties.SetInteger(s_propItemHeight, value);
Properties.AddValue(s_propItemHeight, value);
if (DrawMode != DrawMode.Normal)
{
UpdateItemHeight();
Expand Down Expand Up @@ -759,7 +736,7 @@ public int MaxLength
{
get
{
return Properties.GetInteger(s_propMaxLength);
return Properties.GetValueOrDefault<int>(s_propMaxLength);
}
set
{
Expand All @@ -770,7 +747,7 @@ public int MaxLength

if (MaxLength != value)
{
Properties.SetInteger(s_propMaxLength, value);
Properties.AddValue(s_propMaxLength, value);
if (IsHandleCreated)
{
PInvoke.SendMessage(this, PInvoke.CB_LIMITTEXT, (WPARAM)value);
Expand Down Expand Up @@ -3201,12 +3178,12 @@ private void ResetAutoCompleteCustomSource()

private void ResetDropDownWidth()
{
Properties.RemoveInteger(s_propDropDownWidth);
Properties.RemoveValue(s_propDropDownWidth);
}

private void ResetItemHeight()
{
Properties.RemoveInteger(s_propItemHeight);
Properties.RemoveValue(s_propItemHeight);
}

public override void ResetText()
Expand Down Expand Up @@ -3433,15 +3410,15 @@ private bool ShouldSerializeAutoCompleteCustomSource()

internal bool ShouldSerializeDropDownWidth()
{
return (Properties.ContainsInteger(s_propDropDownWidth));
return (Properties.ContainsKey(s_propDropDownWidth));
}

/// <summary>
/// Indicates whether the itemHeight property should be persisted.
/// </summary>
internal bool ShouldSerializeItemHeight()
{
return (Properties.ContainsInteger(s_propItemHeight));
return (Properties.ContainsKey(s_propItemHeight));
}

/// <summary>
Expand Down
Loading

0 comments on commit 7446e03

Please sign in to comment.