fix(docking): address reentrancy and validation issues in DockContainer

This commit is contained in:
2026-03-28 22:37:59 +09:00
parent 1d48784a1c
commit 8d3c5ecb1f
2 changed files with 46 additions and 37 deletions

View File

@@ -42,29 +42,15 @@ public abstract class DockContainer : DockModule
/// <param name="module">The module to insert.</param> /// <param name="module">The module to insert.</param>
public virtual void InsertChild(int index, DockModule module) public virtual void InsertChild(int index, DockModule module)
{ {
ArgumentNullException.ThrowIfNull(module); ValidateChild(module);
if (index < 0 || index > _children.Count) if (index < 0 || index > _children.Count)
throw new ArgumentOutOfRangeException(nameof(index)); throw new ArgumentOutOfRangeException(nameof(index));
if (module == this)
throw new ArgumentException("Cannot add a container to itself.", nameof(module));
if (module is DockContainer container)
{
var current = Owner;
while (current != null)
{
if (current == container)
throw new ArgumentException("Cannot add a container that is an ancestor of this container.", nameof(module));
current = current.Owner;
}
}
if (_children.Contains(module)) if (_children.Contains(module))
return; return;
module.Owner?.RemoveChild(module); module.Owner?.RemoveChildInternal(module, false);
module.Owner = this; module.Owner = this;
module.Root = Root; module.Root = Root;
_children.Insert(index, module); _children.Insert(index, module);
@@ -75,6 +61,11 @@ public abstract class DockContainer : DockModule
/// </summary> /// </summary>
/// <param name="module">The module to remove.</param> /// <param name="module">The module to remove.</param>
public virtual void RemoveChild(DockModule module) public virtual void RemoveChild(DockModule module)
{
RemoveChildInternal(module, true);
}
internal void RemoveChildInternal(DockModule module, bool triggerCleanup)
{ {
ArgumentNullException.ThrowIfNull(module); ArgumentNullException.ThrowIfNull(module);
@@ -82,22 +73,28 @@ public abstract class DockContainer : DockModule
{ {
module.Owner = null; module.Owner = null;
module.Root = null; module.Root = null;
if (!_isCleaningUp) if (!_isCleaningUp && triggerCleanup)
{ {
CheckCleanup(); CheckCleanup();
} }
} }
} }
/// <summary>
/// Replaces an existing child module with a new one.
/// </summary>
/// <param name="oldChild">The child module to be replaced.</param>
/// <param name="newChild">The new child module to insert.</param>
public virtual void ReplaceChild(DockModule oldChild, DockModule newChild) public virtual void ReplaceChild(DockModule oldChild, DockModule newChild)
{ {
ArgumentNullException.ThrowIfNull(oldChild); ArgumentNullException.ThrowIfNull(oldChild);
ArgumentNullException.ThrowIfNull(newChild); ValidateChild(newChild);
int index = _children.IndexOf(oldChild); int index = _children.IndexOf(oldChild);
if (index < 0) throw new ArgumentException("oldChild not found"); if (index < 0) throw new ArgumentException("oldChild not found");
// Detach newChild from its current owner if any // Detach newChild from its current owner if any
newChild.Owner?.RemoveChild(newChild); newChild.Owner?.RemoveChildInternal(newChild, false);
// Remove oldChild without triggering cleanup // Remove oldChild without triggering cleanup
_isCleaningUp = true; _isCleaningUp = true;
@@ -115,15 +112,41 @@ public abstract class DockContainer : DockModule
{ {
_isCleaningUp = false; _isCleaningUp = false;
} }
OnChildrenUpdated();
CheckCleanup(); CheckCleanup();
} }
/// <summary>
/// Checks if the container is empty and removes it from its owner if necessary.
/// </summary>
protected virtual void CheckCleanup() protected virtual void CheckCleanup()
{ {
if (_children.Count == 0) if (_children.Count == 0)
{ {
Owner?.RemoveChild(this); Owner?.RemoveChildInternal(this, true);
}
}
/// <summary>
/// Validates if a module can be added as a child to this container.
/// </summary>
/// <param name="module">The module to validate.</param>
protected virtual void ValidateChild(DockModule module)
{
ArgumentNullException.ThrowIfNull(module);
if (module == this)
throw new ArgumentException("Cannot add a container to itself.", nameof(module));
if (module is DockContainer container)
{
var current = Owner;
while (current != null)
{
if (current == container)
throw new ArgumentException("Cannot add a container that is an ancestor of this container.", nameof(module));
current = current.Owner;
}
} }
} }

View File

@@ -18,28 +18,14 @@ public partial class DockGroup : DockContainer
DefaultStyleKey = typeof(DockGroup); DefaultStyleKey = typeof(DockGroup);
} }
public override void AddChild(DockModule module) protected override void ValidateChild(DockModule module)
{ {
ArgumentNullException.ThrowIfNull(module); base.ValidateChild(module);
if (module is not DockDocument) if (module is not DockDocument)
{ {
throw new ArgumentException($"{nameof(DockGroup)} only accepts {nameof(DockDocument)} children.", nameof(module)); throw new ArgumentException($"{nameof(DockGroup)} only accepts {nameof(DockDocument)} children.", nameof(module));
} }
base.AddChild(module);
}
public override void InsertChild(int index, DockModule module)
{
ArgumentNullException.ThrowIfNull(module);
if (module is not DockDocument)
{
throw new ArgumentException($"{nameof(DockGroup)} only accepts {nameof(DockDocument)} children.", nameof(module));
}
base.InsertChild(index, module);
} }
protected override void OnApplyTemplate() protected override void OnApplyTemplate()