Skip to content

Commit

Permalink
Divide properties into: public, private, protected. (#62627)
Browse files Browse the repository at this point in the history 
* Description of DebuggerBrowsable behavior.

* Added test for browse attributes.

* Corrected typos in the doc.

* Added Browse Never feature. Corrected Collapse test. ToDo: RootHidden.

* Draft of RootHidden solution.

* Added Array to test cases as it behaves differently than Collection.

* Added name concatenation to make array/list elemetns in debug window unique.

* Update docs/design/mono/debugger.md

Co-authored-by: Ankit Jain <[email protected]>

* Applied PR review suggestions.

* Added a reference to regular Browsable attribute behavior in .net.

* Applied most of review suggestions.

* Stopping GetFieldsValue early.

* Remove unintentional change to the original code.

* Do not skip fields that don't have browsable attributes.

* Changing the expected behavior to match Console Application. EventHandlers are Browsable.Never by default.

* Changed the place of checking if objetc is an array.

* Update src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs

Co-authored-by: Ankit Jain <[email protected]>

* Removed unused variables.

* Removing space and unused import.

* Draft - parameter for choosing sorted and unsorted returned value version.

* Working version.

* Added "protected internal" and "private protected" cases. Changed field name to be consistant with the value.

* Changed approach: placed sorting at the end of the function, not in the middle.

* Fix for tests that use GetProperties.

* Fixed most tests (31 failing).

* Removed whitespaces.

* Fixed CheckAccessorsOnObjectsWithCFO tests.

* Fix for InspectTypeInheritedMembers.

* Revert unnecessary indent change.

* Partially addressed @radical comments.

* Addressed the comment about extension instead of Union.

* Removed string cultural vunerability.

* Added Properties dictionary, the same as for fields.

* Fixed the bug I made by using dynamc.

* Applying @radical comments about refactoring.

* Corrected typo.

* Added tests for properties.

* Draft of changes for properties handling - never and root hidden failing.

* Fix for RootHidden properties.

* Added tests for static fields decorated with Browsable.

* Correct a typo.

* Undo merge unintentional changes.

* Changing expected behavior for MulticastDelegateTest - in Console Application EventHandler is Browsable.Never by default so we should not expect it to be visible in the debug window.

* Removing not relevant changes created after merge with main.

* Remove file added in merge with main.

* Revert "Removing not relevant changes created after merge with main."

This reverts commit b1acf8b.

* Revert.

* Revert revert.

* One broken test for custom getter.

* Ugly fix to make all the tests work.

* Refactored JArray aggregation to Dictionary.

* Better naming.

* Add skipped change from merge.

* Fix getters testing.

* Removed unnecessary doubled execution.

* Decreased complexity by replacing a loop with data aggregation.

* Replaced O(nm) Linq with O(n) loop through dictionary.

* Remove comment, flip conditions.

* Removed not intended comments and changes.

* Missing removal for previous commit.

* Added tests for protection levels.

Co-authored-by: Ankit Jain <[email protected]>
  • Loading branch information
@ilonatommy @radical
ilonatommy and radical authored Jan 4, 2022
1 parent ef357af commit ae2e360
Show file tree
Hide file tree
Showing 6 changed files with 231 additions and 64 deletions.
1 change: 0 additions & 1 deletion src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
using System.Reflection;
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.VisualBasic;

namespace Microsoft.WebAssembly.Diagnostics
{
Expand Down
41 changes: 28 additions & 13 deletions src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,12 +461,12 @@ protected override async Task<bool> AcceptCommand(MessageId id, string method, J
if (!DotnetObjectId.TryParse(args?["objectId"], out DotnetObjectId objectId))
break;

var ret = await RuntimeGetPropertiesInternal(id, objectId, args, token);
var ret = await RuntimeGetPropertiesInternal(id, objectId, args, token, true);
if (ret == null) {
SendResponse(id, Result.Err($"Unable to RuntimeGetProperties '{objectId}'"), token);
}
else
SendResponse(id, Result.OkFromObject(new { result = ret }), token);
SendResponse(id, Result.OkFromObject(ret), token);
return true;
}

Expand Down Expand Up @@ -658,7 +658,7 @@ private async Task<bool> OnSetVariableValue(MessageId id, int scopeId, string va
return true;
}

internal async Task<JToken> RuntimeGetPropertiesInternal(SessionId id, DotnetObjectId objectId, JToken args, CancellationToken token)
internal async Task<JToken> RuntimeGetPropertiesInternal(SessionId id, DotnetObjectId objectId, JToken args, CancellationToken token, bool sortByAccessLevel = false)
{
var context = GetContext(id);
var accessorPropertiesOnly = false;
Expand All @@ -675,33 +675,48 @@ internal async Task<JToken> RuntimeGetPropertiesInternal(SessionId id, DotnetObj
objectValuesOpt |= GetObjectCommandOptions.OwnProperties;
}
}
//Console.WriteLine($"RuntimeGetProperties - {args}");
try {
switch (objectId.Scheme)
{
case "scope":
{
var res = await GetScopeProperties(id, objectId.Value, token);
return res.Value?["result"];
var resScope = await GetScopeProperties(id, objectId.Value, token);
if (sortByAccessLevel)
return resScope.Value;
return resScope.Value?["result"];
}
case "valuetype":
return await context.SdbAgent.GetValueTypeValues(objectId.Value, accessorPropertiesOnly, token);
{
var resValType = await context.SdbAgent.GetValueTypeValues(objectId.Value, accessorPropertiesOnly, token);
return sortByAccessLevel ? JObject.FromObject(new { result = resValType }) : resValType;
}
case "array":
return await context.SdbAgent.GetArrayValues(objectId.Value, token);
{
var resArr = await context.SdbAgent.GetArrayValues(objectId.Value, token);
return sortByAccessLevel ? JObject.FromObject(new { result = resArr }) : resArr;
}
case "methodId":
{
var objRet = await context.SdbAgent.InvokeMethodInObject(objectId.Value, objectId.SubValue, "", token);
return new JArray(objRet);
var resMethod = await context.SdbAgent.InvokeMethodInObject(objectId.Value, objectId.SubValue, "", token);
return sortByAccessLevel ? JObject.FromObject(new { result = resMethod }) : new JArray(resMethod);
}
case "object":
return await context.SdbAgent.GetObjectValues(objectId.Value, objectValuesOpt, token);
{
var resObj = (await context.SdbAgent.GetObjectValues(objectId.Value, objectValuesOpt, token, sortByAccessLevel));
return sortByAccessLevel ? resObj[0] : resObj;
}
case "pointer":
return new JArray{await context.SdbAgent.GetPointerContent(objectId.Value, token)};
{
var resPointer = new JArray { await context.SdbAgent.GetPointerContent(objectId.Value, token) };
return sortByAccessLevel ? JObject.FromObject(new { result = resPointer }) : resPointer;
}
case "cfo_res":
{
Result res = await SendMonoCommand(id, MonoCommands.GetDetails(RuntimeId, objectId.Value, args), token);
string value_json_str = res.Value["result"]?["value"]?["__value_as_json_string__"]?.Value<string>();
return value_json_str != null ? JArray.Parse(value_json_str) : null;
return value_json_str != null ?
(sortByAccessLevel ? JObject.FromObject(new { result = JArray.Parse(value_json_str) }) : JArray.Parse(value_json_str)) :
null;
}
default:
return null;
Expand Down
142 changes: 94 additions & 48 deletions src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -646,13 +646,15 @@ internal class FieldTypeClass
public int Id { get; }
public string Name { get; }
public int TypeId { get; }
public bool IsPublic { get; }
public FieldTypeClass(int id, string name, int typeId, bool isPublic)
public bool IsNotPrivate { get; }
public FieldAttributes ProtectionLevel { get; }
public FieldTypeClass(int id, string name, int typeId, bool isNotPrivate, FieldAttributes protectionLevel)
{
Id = id;
Name = name;
TypeId = typeId;
IsPublic = isPublic;
IsNotPrivate = isNotPrivate;
ProtectionLevel = protectionLevel;
}
}
internal class ValueTypeClass
Expand Down Expand Up @@ -1235,14 +1237,14 @@ public async Task<List<FieldTypeClass>> GetTypeFields(int typeId, CancellationTo

for (int i = 0 ; i < nFields; i++)
{
bool isPublic = false;
bool isNotPrivate = false;
int fieldId = retDebuggerCmdReader.ReadInt32(); //fieldId
string fieldNameStr = retDebuggerCmdReader.ReadString();
int fieldTypeId = retDebuggerCmdReader.ReadInt32(); //typeId
int attrs = retDebuggerCmdReader.ReadInt32(); //attrs
int isSpecialStatic = retDebuggerCmdReader.ReadInt32(); //is_special_static
if (((attrs & (int)MethodAttributes.Public) != 0))
isPublic = true;
isNotPrivate = true;
if (isSpecialStatic == 1)
continue;
if (fieldNameStr.Contains("k__BackingField"))
Expand All @@ -1251,7 +1253,7 @@ public async Task<List<FieldTypeClass>> GetTypeFields(int typeId, CancellationTo
fieldNameStr = fieldNameStr.Replace("<", "");
fieldNameStr = fieldNameStr.Replace(">", "");
}
ret.Add(new FieldTypeClass(fieldId, fieldNameStr, fieldTypeId, isPublic));
ret.Add(new FieldTypeClass(fieldId, fieldNameStr, fieldTypeId, isNotPrivate, (FieldAttributes)((attrs) & (int)FieldAttributes.FieldAccessMask)));
}
typeInfo.FieldsList = ret;
return ret;
Expand Down Expand Up @@ -2315,41 +2317,46 @@ public async Task<JArray> GetValuesFromDebuggerProxyAttribute(int objectId, int
return null;
}

public async Task<JArray> GetObjectValues(int objectId, GetObjectCommandOptions getCommandType, CancellationToken token)
public async Task<JArray> GetObjectValues(int objectId, GetObjectCommandOptions getCommandType, CancellationToken token, bool sortByAccessLevel = false)
{
var typeIdsIncludingParents = await GetTypeIdFromObject(objectId, true, token);
if (!getCommandType.HasFlag(GetObjectCommandOptions.ForDebuggerDisplayAttribute))
{
var debuggerProxy = await GetValuesFromDebuggerProxyAttribute(objectId, typeIdsIncludingParents[0], token);
if (debuggerProxy != null)
return debuggerProxy;
return sortByAccessLevel ?
new JArray(JObject.FromObject( new { result = debuggerProxy, internalProperties = new JArray(), privateProperties = new JArray() } )) :
debuggerProxy;
}
if (await IsDelegate(objectId, token))
{
var description = await GetDelegateMethodDescription(objectId, token);
return new JArray(
JObject.FromObject(new
var objValues = new JArray(JObject.FromObject(new
{
value = new
{
value = new
{
type = "symbol",
value = description,
description
},
name = "Target"
}));
type = "symbol",
value = description,
description
},
name = "Target"
}));
return sortByAccessLevel ?
new JArray(JObject.FromObject(new { result = objValues, internalProperties = new JArray(), privateProperties = new JArray() })) :
objValues;
}

var allFields = new List<FieldTypeClass>();
var objects = new Dictionary<string, JToken>();
for (int i = 0; i < typeIdsIncludingParents.Count; i++)
{
int typeId = typeIdsIncludingParents[i];
// 0th id is for the object itself, and then its parents
bool isOwn = i == 0;
var fields = await GetTypeFields(typeId, token);
allFields.AddRange(fields);

if (!getCommandType.HasFlag(GetObjectCommandOptions.AccessorPropertiesOnly))
{
var fields = await GetTypeFields(typeId, token);
var (collapsedFields, rootHiddenFields) = await FilterFieldsByDebuggerBrowsable(fields, typeId, token);

var collapsedFieldsValues = await GetFieldsValues(collapsedFields, isOwn);
Expand All @@ -2358,8 +2365,12 @@ public async Task<JArray> GetObjectValues(int objectId, GetObjectCommandOptions
objects.TryAddRange(collapsedFieldsValues);
objects.TryAddRange(hiddenFieldsValues);
}

if (!getCommandType.HasFlag(GetObjectCommandOptions.WithProperties))
return new JArray(objects.Values);
return sortByAccessLevel ?
SegregatePropertiesByAccessLevel(allFields, objects, token) :
new JArray(objects.Values);

using var commandParamsObjWriter = new MonoBinaryWriter();
commandParamsObjWriter.WriteObj(new DotnetObjectId("object", objectId), this);
var props = await CreateJArrayForProperties(
Expand All @@ -2370,41 +2381,60 @@ public async Task<JArray> GetObjectValues(int objectId, GetObjectCommandOptions
$"dotnet:object:{objectId}",
i == 0,
token);
var properties = await GetProperties(props, typeId, token);
var properties = await GetProperties(props, allFields, typeId, token);
objects.TryAddRange(properties);

// ownProperties
// Note: ownProperties should mean that we return members of the klass itself,
// but we are going to ignore that here, because otherwise vscode/chrome don't
// seem to ask for inherited fields at all.
//if (ownProperties)
//break;
//break;
/*if (accessorPropertiesOnly)
break;*/
}
if (getCommandType.HasFlag(GetObjectCommandOptions.AccessorPropertiesOnly))
{
var ownId = typeIdsIncludingParents[0];
var ownFields = await GetTypeFields(ownId, token);

var parentFields = new List<FieldTypeClass>();
for (int i = 1; i < typeIdsIncludingParents.Count; i++)
parentFields.AddRange(await GetTypeFields(typeIdsIncludingParents[i], token));

var ownDuplicatedFields = ownFields.Where(field => objects.Any(obj =>
field.Name.Equals(obj.Key)));
return sortByAccessLevel ?
SegregatePropertiesByAccessLevel(allFields, objects, token) :
new JArray(objects.Values);

var parentDuplicatedFields = parentFields.Where(field => objects.Any(obj =>
field.Name.Equals(obj.Key) &&
(obj.Value["isOwn"] == null ||
!obj.Value["isOwn"].Value<bool>())));

foreach (var duplicate in ownDuplicatedFields)
objects.Remove(duplicate.Name);
foreach (var duplicate in parentDuplicatedFields)
objects.Remove(duplicate.Name);
JArray SegregatePropertiesByAccessLevel(List<FieldTypeClass> fields, Dictionary<string, JToken> objectsDict, CancellationToken token)
{
if (fields.Count == 0)
return new JArray(JObject.FromObject(new { result = new JArray(objectsDict.Values) }));

var pubNames = fields.Where(field => field.ProtectionLevel == FieldAttributes.Public)
.Select(field => field.Name).ToList();
var privNames = fields.Where(field =>
(field.ProtectionLevel == FieldAttributes.Private ||
field.ProtectionLevel == FieldAttributes.FamANDAssem) &&
// when field is inherited it is listed both in Private and Public, to avoid duplicates:
pubNames.All(pubFieldName => pubFieldName != field.Name))
.Select(field => field.Name).ToList();
//protected == family, internal == assembly
var protectedAndInternalNames = fields.Where(field =>
field.ProtectionLevel == FieldAttributes.Family ||
field.ProtectionLevel == FieldAttributes.Assembly ||
field.ProtectionLevel == FieldAttributes.FamORAssem)
.Select(field => field.Name).ToList();
var accessorProperties = objectsDict
.Where(obj => (
pubNames.All(name => name != obj.Key) &&
privNames.All(name => name != obj.Key) &&
protectedAndInternalNames.All(name => name != obj.Key)))
.Select((k, v) => k.Value);

var pubProperties = objectsDict.GetValuesByKeys(pubNames);
var privProperties = objectsDict.GetValuesByKeys(privNames);
var protAndIntProperties = objectsDict.GetValuesByKeys(protectedAndInternalNames);
pubProperties.AddRange(new JArray(accessorProperties));

return new JArray(JObject.FromObject(new
{
result = pubProperties,
internalProperties = protAndIntProperties,
privateProperties = privProperties
}));
}
return new JArray(objects.Values);

async Task AppendRootHiddenChildren(JObject root, JArray expandedCollection)
{
Expand Down Expand Up @@ -2439,7 +2469,7 @@ async Task<JArray> GetFieldsValues(List<FieldTypeClass> fields, bool isOwn, bool
return objFields;

if (getCommandType.HasFlag(GetObjectCommandOptions.ForDebuggerProxyAttribute))
fields = fields.Where(field => field.IsPublic).ToList();
fields = fields.Where(field => field.IsNotPrivate).ToList();

using var commandParamsWriter = new MonoBinaryWriter();
commandParamsWriter.Write(objectId);
Expand Down Expand Up @@ -2527,14 +2557,18 @@ async Task<JArray> GetFieldsValues(List<FieldTypeClass> fields, bool isOwn, bool
return (collapsedFields, rootHiddenFields);
}

async Task<JArray> GetProperties(JArray props, int typeId, CancellationToken token)
async Task<JArray> GetProperties(JArray props, List<FieldTypeClass> fields, int typeId, CancellationToken token)
{
var typeInfo = await GetTypeInfo(typeId, token);
var typeProperitesBrowsableInfo = typeInfo?.Info?.DebuggerBrowsableProperties;
var regularProps = new JArray();
foreach (var p in props)
{
if (!typeProperitesBrowsableInfo.TryGetValue(p["name"].Value<string>(), out DebuggerBrowsableState? state))
var propName = p["name"].Value<string>();
// if property has a backing field - avoid adding a duplicate
if (fields.Any(field => field.Name == propName))
continue;
if (!typeProperitesBrowsableInfo.TryGetValue(propName, out DebuggerBrowsableState? state))
{
regularProps.Add(p);
continue;
Expand All @@ -2546,7 +2580,7 @@ async Task<JArray> GetProperties(JArray props, int typeId, CancellationToken tok
case DebuggerBrowsableState.RootHidden:
DotnetObjectId rootObjId;
DotnetObjectId.TryParse(p["get"]["objectId"].Value<string>(), out rootObjId);
var rootObject = await InvokeMethodInObject(rootObjId.Value, rootObjId.SubValue, p["name"].Value<string>(), token);
var rootObject = await InvokeMethodInObject(rootObjId.Value, rootObjId.SubValue, propName, token);
await AppendRootHiddenChildren(rootObject, regularProps);
break;
case DebuggerBrowsableState.Collapsed:
Expand Down Expand Up @@ -2663,5 +2697,17 @@ public static void TryAddRange(this Dictionary<string, JToken> dict, JArray adde
dict.TryAdd(key, item);
}
}

public static JArray GetValuesByKeys(this Dictionary<string, JToken> dict, List<string> keys)
{
var result = new JArray();
foreach (var name in keys)
{
if (!dict.TryGetValue(name, out var obj))
continue;
result.Add(obj);
}
return result;
}
}
}
Loading

0 comments on commit ae2e360

Please sign in to comment.