From b4dd762364df67225386186dc0e601eaa8cae298 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 27 Oct 2021 17:34:18 -0700 Subject: [PATCH 01/19] Kind of working for Map types --- .../lib/src/debugger/debugger_controller.dart | 4 +- .../lib/src/debugger/debugger_model.dart | 76 +++++++++++++++++-- .../lib/src/debugger/variables.dart | 4 +- packages/devtools_app/lib/src/utils.dart | 8 +- .../lib/src/vm_service_wrapper.dart | 5 +- .../macos/Runner.xcodeproj/project.pbxproj | 5 ++ packages/devtools_app/test/utils_test.dart | 11 ++- 7 files changed, 103 insertions(+), 10 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/debugger_controller.dart b/packages/devtools_app/lib/src/debugger/debugger_controller.dart index a9e1c34e7d5..f5a1a62baa5 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_controller.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_controller.dart @@ -763,6 +763,8 @@ class DebuggerController extends DisposableController // Collecting frames for Dart web applications can be slow. At the potential // cost of a flicker in the stack view, display only the top frame // initially. + // Note: Should find a better solution for this. + //Currently, this means we fetch all variable objects twice. (Once here, and once in _getFullStack) if (await serviceManager.connectedApp.isDartWebApp) { _populateFrameInfo( [ @@ -949,7 +951,7 @@ class DebuggerController extends DisposableController final variables = frame.vars.map((v) => Variable.create(v, isolateRef)).toList(); variables - ..forEach(buildVariablesTree) + ..forEach((v) => buildVariablesTree(v, offset: 0, count: 0)) ..sort((a, b) => sortFieldsByName(a.name, b.name)); return variables; } diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index e76bddf0a7c..38fb47c667f 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -334,6 +334,8 @@ Future addExpandableChildren( Future buildVariablesTree( Variable variable, { bool expandAll = false, + int offset, + int count, }) async { final ref = variable.ref; if (!variable.isExpandable || variable.treeInitializeStarted || ref == null) @@ -382,13 +384,29 @@ Future buildVariablesTree( } if (instanceRef != null && serviceManager.service != null) { try { - final dynamic result = await serviceManager.service - .getObject(variable.ref.isolateRef.id, instanceRef.id); + final dynamic result = await serviceManager.service.getObject( + variable.ref.isolateRef.id, instanceRef.id, + offset: offset, count: count); if (result is Instance) { if (result.associations != null) { - variable.addAllChildren( - _createVariablesForAssociations(result, isolateRef)); + if (variable.childCount > 50) { + var offset = 0; + while (offset + 50 < variable.childCount) { + variable.addChild( + Variable.grouping(variable.ref, offset: offset, count: 50), + ); + offset = offset + 50; + } + final remainder = variable.childCount - offset; + final childVar = Variable.grouping(variable.ref, + offset: offset, count: remainder); + variable.addChild(childVar); + } else { + variable.addAllChildren( + _createVariablesForAssociations(result, isolateRef)); + } } else if (result.elements != null) { + // this is for a list variable .addAllChildren(_createVariablesForElements(result, isolateRef)); } else if (result.bytes != null) { @@ -657,7 +675,12 @@ List _createVariablesForFields( // TODO(jacobr): consider a new class name. This class is more just the data // model for a tree of Dart objects with properties rather than a "Variable". class Variable extends TreeNode { - Variable._(this.name, ref, this.text) : _ref = ref { + Variable._(this.name, ref, this.text, + {int offset, int count, bool isGrouping = false}) + : _ref = ref, + _offset = offset, + _count = count, + _isGrouping = isGrouping { indentChildren = ref?.diagnostic?.style != DiagnosticsTreeStyle.flat; } @@ -702,11 +725,35 @@ class Variable extends TreeNode { return Variable._(null, null, text); } + factory Variable.grouping( + GenericInstanceRef ref, { + @required int offset, + @required int count, + }) { + return Variable._( + null, + ref, + '[$offset - ${offset + count}]', + offset: offset, + count: count, + isGrouping: true, + ); + } + final String text; final String name; GenericInstanceRef get ref => _ref; GenericInstanceRef _ref; + int get offset => _offset; + int _offset; + + int get count => _count; + int _count; + + bool get isGrouping => _isGrouping; + bool _isGrouping; + bool treeInitializeStarted = false; bool treeInitializeComplete = false; @@ -726,6 +773,25 @@ class Variable extends TreeNode { Object get value => ref?.value; + int get childCount { + if (isGrouping) { + return count; + } + + final value = this.value; + if (value is InstanceRef) { + if (value.kind == InstanceKind.kList) { + return value.length; + } else if (value.kind == InstanceKind.kMap) { + return value.length; + } else if (value.kind != null && value.kind.endsWith('List')) { + return value.length; + } + } + + return 0; + } + // TODO(kenz): add custom display for lists with more than 100 elements String get displayValue { if (text != null) { diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index ba97a38ffe1..389d864ef1d 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -45,7 +45,9 @@ class Variables extends StatelessWidget { void onItemPressed(Variable v, DebuggerController controller) { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { - v.children.forEach(buildVariablesTree); + v.children.forEach( + (v) => buildVariablesTree(v, offset: v.offset, count: v.count), + ); } } } diff --git a/packages/devtools_app/lib/src/utils.dart b/packages/devtools_app/lib/src/utils.dart index 1f6e82cad88..27ef3a6c77d 100644 --- a/packages/devtools_app/lib/src/utils.dart +++ b/packages/devtools_app/lib/src/utils.dart @@ -186,7 +186,13 @@ T nullSafeMax(T a, T b) { return max(a, b); } -int log2(num x) => (log(x) / log(2)).floor(); +double logBase(num x, num base) { + return log(x) / log(base); +} + +int log2(num x) => (logBase(x, 2)).floor(); + +int roundToNearestPow10(num x) => pow(10, logBase(x, 10).ceil()).floor(); String isolateName(IsolateRef ref) { // analysis_server.dart.snapshot$main diff --git a/packages/devtools_app/lib/src/vm_service_wrapper.dart b/packages/devtools_app/lib/src/vm_service_wrapper.dart index 49a3d181eed..5230ba9dfeb 100644 --- a/packages/devtools_app/lib/src/vm_service_wrapper.dart +++ b/packages/devtools_app/lib/src/vm_service_wrapper.dart @@ -401,7 +401,10 @@ class VmServiceWrapper implements VmService { int offset, int count, }) { - return trackFuture('getObject', _vmService.getObject(isolateId, objectId)); + return trackFuture( + 'getObject', + _vmService.getObject(isolateId, objectId, + offset: offset, count: count)); } @override diff --git a/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj b/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj index a4d00fa9693..bc03301f8ac 100644 --- a/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj +++ b/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj @@ -308,9 +308,14 @@ files = ( ); inputPaths = ( + "${PODS_ROOT}/Target Support Files/Pods-Runner/Pods-Runner-frameworks.sh", + "${BUILT_PRODUCTS_DIR}/file_selector_macos/file_selector_macos.framework", + "${BUILT_PRODUCTS_DIR}/url_launcher_macos/url_launcher_macos.framework", ); name = "[CP] Embed Pods Frameworks"; outputPaths = ( + "${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/file_selector_macos.framework", + "${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/url_launcher_macos.framework", ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/sh; diff --git a/packages/devtools_app/test/utils_test.dart b/packages/devtools_app/test/utils_test.dart index 63e7bb02084..6a22316ba94 100644 --- a/packages/devtools_app/test/utils_test.dart +++ b/packages/devtools_app/test/utils_test.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. - import 'package:devtools_app/src/utils.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -109,6 +108,16 @@ void main() { expect(log2(4), equals(2)); }); + test('roundToNearestPow10', () { + expect(roundToNearestPow10(0.1), equals(1)); + expect(roundToNearestPow10(1), equals(1)); + expect(roundToNearestPow10(2), equals(10)); + expect(roundToNearestPow10(10), equals(10)); + expect(roundToNearestPow10(11), equals(100)); + expect(roundToNearestPow10(189), equals(1000)); + expect(roundToNearestPow10(6581), equals(10000)); + }); + test('executeWithDelay', () async { const delayMs = 500; int n = 1; From ae2619caf8dfd88cb862d72caab0d4be4e6310ea Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 28 Oct 2021 17:08:57 -0700 Subject: [PATCH 02/19] List and maps working --- .../lib/src/debugger/debugger_model.dart | 59 +++++++++++-------- .../lib/src/debugger/variables.dart | 3 +- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index 38fb47c667f..4bbfa64a674 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -382,29 +382,37 @@ Future buildVariablesTree( } } } - if (instanceRef != null && serviceManager.service != null) { + + if (variable.childCount > Variable.MAX_CHILDREN_IN_GROUPING) { + final numChildrenInGrouping = variable.childCount >= + Variable.MAX_CHILDREN_IN_GROUPING * + Variable.MAX_CHILDREN_IN_GROUPING + ? (roundToNearestPow10(variable.childCount) / + Variable.MAX_CHILDREN_IN_GROUPING) + .floor() + : Variable.MAX_CHILDREN_IN_GROUPING; + + var offset = 0; + while (offset + numChildrenInGrouping < variable.childCount) { + variable.addChild( + Variable.grouping(variable.ref, + offset: offset, count: numChildrenInGrouping), + ); + offset += numChildrenInGrouping; + } + variable.addChild( + Variable.grouping(variable.ref, + offset: offset, count: variable.childCount - offset), + ); + } else if (instanceRef != null && serviceManager.service != null) { try { final dynamic result = await serviceManager.service.getObject( variable.ref.isolateRef.id, instanceRef.id, offset: offset, count: count); if (result is Instance) { if (result.associations != null) { - if (variable.childCount > 50) { - var offset = 0; - while (offset + 50 < variable.childCount) { - variable.addChild( - Variable.grouping(variable.ref, offset: offset, count: 50), - ); - offset = offset + 50; - } - final remainder = variable.childCount - offset; - final childVar = Variable.grouping(variable.ref, - offset: offset, count: remainder); - variable.addChild(childVar); - } else { - variable.addAllChildren( - _createVariablesForAssociations(result, isolateRef)); - } + variable.addAllChildren( + _createVariablesForAssociations(result, isolateRef)); } else if (result.elements != null) { // this is for a list variable @@ -546,8 +554,9 @@ List _createVariablesForAssociations( value: association.value, isolateRef: isolateRef, ); + final entryNum = instance.offset == null ? i : i + instance.offset; variables.add( - Variable.text('[Entry $i]') + Variable.text('[Entry $entryNum]') ..addChild(key) ..addChild(value), ); @@ -622,9 +631,10 @@ List _createVariablesForBytes( } for (int i = 0; i < result.length; i++) { + final name = instance.offset == null ? i : i + instance.offset; variables.add( Variable.fromValue( - name: '[$i]', + name: '[$name]', value: result[i], isolateRef: isolateRef, ), @@ -639,9 +649,10 @@ List _createVariablesForElements( ) { final variables = []; for (int i = 0; i < instance.elements.length; i++) { + final name = instance.offset == null ? i : i + instance.offset; variables.add( Variable.fromValue( - name: '[$i]', + name: '[$name]', value: instance.elements[i], isolateRef: isolateRef, ), @@ -733,13 +744,15 @@ class Variable extends TreeNode { return Variable._( null, ref, - '[$offset - ${offset + count}]', + '[$offset ... ${offset + count - 1}]', offset: offset, count: count, isGrouping: true, ); } + static const MAX_CHILDREN_IN_GROUPING = 100; + final String text; final String name; GenericInstanceRef get ref => _ref; @@ -759,8 +772,8 @@ class Variable extends TreeNode { @override bool get isExpandable { - if (treeInitializeComplete || children.isNotEmpty) { - return children.isNotEmpty; + if (treeInitializeComplete || children.isNotEmpty || childCount > 0) { + return children.isNotEmpty || childCount > 0; } final diagnostic = ref.diagnostic; if (diagnostic != null && diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index 389d864ef1d..47a748da699 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -46,7 +46,8 @@ class Variables extends StatelessWidget { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { v.children.forEach( - (v) => buildVariablesTree(v, offset: v.offset, count: v.count), + (child) => + buildVariablesTree(child, offset: child.offset, count: child.count), ); } } From 9ad13b77c2373069c98e7765958bfd1793acf179 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 29 Oct 2021 13:13:26 -0700 Subject: [PATCH 03/19] Committing to switch branches --- packages/devtools_app/lib/src/console_service.dart | 1 + .../devtools_app/lib/src/debugger/codeview.dart | 3 ++- .../lib/src/debugger/debugger_controller.dart | 5 +++-- .../lib/src/debugger/debugger_model.dart | 1 - .../devtools_app/lib/src/debugger/variables.dart | 13 +++++++++++-- .../devtools_app/lib/src/inspector/diagnostics.dart | 1 + 6 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/devtools_app/lib/src/console_service.dart b/packages/devtools_app/lib/src/console_service.dart index bd6be18ea57..42d5f7a8cec 100644 --- a/packages/devtools_app/lib/src/console_service.dart +++ b/packages/devtools_app/lib/src/console_service.dart @@ -81,6 +81,7 @@ class ConsoleService extends Disposer { isolateRef: isolateRef, ); // TODO(jacobr): fix out of order issues by tracking raw order. + print('build variable in console.'); await buildVariablesTree(variable, expandAll: expandAll); if (expandAll) { variable.expandCascading(); diff --git a/packages/devtools_app/lib/src/debugger/codeview.dart b/packages/devtools_app/lib/src/debugger/codeview.dart index 7d102dc2fd4..51143921a8a 100644 --- a/packages/devtools_app/lib/src/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/debugger/codeview.dart @@ -787,7 +787,8 @@ class _LineItemState extends State { value: response, isolateRef: isolateRef, ); - await buildVariablesTree(variable); + print('===== ON HOVER, BUILD VARIABLE.'); + await buildVariablesTree(variable, offset: 0, count: 0); if (_hasMouseExited) return; _hoverCard?.remove(); _hoverCard = HoverCard.fromHoverEvent( diff --git a/packages/devtools_app/lib/src/debugger/debugger_controller.dart b/packages/devtools_app/lib/src/debugger/debugger_controller.dart index f5a1a62baa5..75e34472430 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_controller.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_controller.dart @@ -763,8 +763,9 @@ class DebuggerController extends DisposableController // Collecting frames for Dart web applications can be slow. At the potential // cost of a flicker in the stack view, display only the top frame // initially. - // Note: Should find a better solution for this. - //Currently, this means we fetch all variable objects twice. (Once here, and once in _getFullStack) + // TODO(elliette): Find a better solution for this. Currently, this means + // we fetch all variable objects twice (once in _getFullStack and once in + // in_createStackFrameWithLocation). if (await serviceManager.connectedApp.isDartWebApp) { _populateFrameInfo( [ diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index 4bbfa64a674..689fed836ec 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -414,7 +414,6 @@ Future buildVariablesTree( variable.addAllChildren( _createVariablesForAssociations(result, isolateRef)); } else if (result.elements != null) { - // this is for a list variable .addAllChildren(_createVariablesForElements(result, isolateRef)); } else if (result.bytes != null) { diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index 47a748da699..876da8e12ff 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -45,6 +45,7 @@ class Variables extends StatelessWidget { void onItemPressed(Variable v, DebuggerController controller) { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { + print('===== A ====='); v.children.forEach( (child) => buildVariablesTree(child, offset: child.offset, count: child.count), @@ -75,7 +76,11 @@ class VariablesList extends StatelessWidget { void onItemPressed(Variable v, DebuggerController controller) { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { - v.children.forEach(buildVariablesTree); + print('====== B ====='); + v.children.forEach( + (child) => + buildVariablesTree(child, offset: child.offset, count: child.count), + ); } } } @@ -114,7 +119,11 @@ class ExpandableVariable extends StatelessWidget { void onItemPressed(Variable v, DebuggerController controller) { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { - v.children.forEach(buildVariablesTree); + print('======= C ======'); + v.children.forEach( + (child) => + buildVariablesTree(child, offset: child.offset, count: child.count), + ); } } } diff --git a/packages/devtools_app/lib/src/inspector/diagnostics.dart b/packages/devtools_app/lib/src/inspector/diagnostics.dart index 009c507d9d2..64182ed020a 100644 --- a/packages/devtools_app/lib/src/inspector/diagnostics.dart +++ b/packages/devtools_app/lib/src/inspector/diagnostics.dart @@ -125,6 +125,7 @@ class DiagnosticsNodeDescription extends StatelessWidget { isolateRef: serviceManager.inspectorService.isolateRef, diagnostic: diagnostic, ); + print('------ build variable in diagnostics? -----'); await buildVariablesTree(variable); for (var child in variable.children) { await buildVariablesTree(child); From 1b9a3499f1d4ad7a96589a4bb63d40279500b46e Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 1 Nov 2021 09:52:22 -0700 Subject: [PATCH 04/19] Offset/count defaults to 0 --- .../lib/src/debugger/codeview.dart | 2 +- .../lib/src/debugger/debugger_controller.dart | 2 +- .../lib/src/debugger/debugger_model.dart | 16 ++++++++-------- .../lib/src/debugger/variables.dart | 18 +++--------------- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/codeview.dart b/packages/devtools_app/lib/src/debugger/codeview.dart index 51143921a8a..8364602987b 100644 --- a/packages/devtools_app/lib/src/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/debugger/codeview.dart @@ -788,7 +788,7 @@ class _LineItemState extends State { isolateRef: isolateRef, ); print('===== ON HOVER, BUILD VARIABLE.'); - await buildVariablesTree(variable, offset: 0, count: 0); + await buildVariablesTree(variable); if (_hasMouseExited) return; _hoverCard?.remove(); _hoverCard = HoverCard.fromHoverEvent( diff --git a/packages/devtools_app/lib/src/debugger/debugger_controller.dart b/packages/devtools_app/lib/src/debugger/debugger_controller.dart index 75e34472430..875665cc642 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_controller.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_controller.dart @@ -952,7 +952,7 @@ class DebuggerController extends DisposableController final variables = frame.vars.map((v) => Variable.create(v, isolateRef)).toList(); variables - ..forEach((v) => buildVariablesTree(v, offset: 0, count: 0)) + ..forEach(buildVariablesTree) ..sort((a, b) => sortFieldsByName(a.name, b.name)); return variables; } diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index 689fed836ec..6a930e09d2d 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -334,8 +334,6 @@ Future addExpandableChildren( Future buildVariablesTree( Variable variable, { bool expandAll = false, - int offset, - int count, }) async { final ref = variable.ref; if (!variable.isExpandable || variable.treeInitializeStarted || ref == null) @@ -392,23 +390,25 @@ Future buildVariablesTree( .floor() : Variable.MAX_CHILDREN_IN_GROUPING; - var offset = 0; - while (offset + numChildrenInGrouping < variable.childCount) { + var index = 0; + while (index + numChildrenInGrouping < variable.childCount) { variable.addChild( Variable.grouping(variable.ref, - offset: offset, count: numChildrenInGrouping), + offset: index + (variable.offset ?? 0), + count: numChildrenInGrouping), ); - offset += numChildrenInGrouping; + index += numChildrenInGrouping; } variable.addChild( Variable.grouping(variable.ref, - offset: offset, count: variable.childCount - offset), + offset: index + (variable.offset ?? 0), + count: variable.childCount - index), ); } else if (instanceRef != null && serviceManager.service != null) { try { final dynamic result = await serviceManager.service.getObject( variable.ref.isolateRef.id, instanceRef.id, - offset: offset, count: count); + offset: variable.offset ?? 0, count: variable.count ?? 0); if (result is Instance) { if (result.associations != null) { variable.addAllChildren( diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index 876da8e12ff..ba97a38ffe1 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -45,11 +45,7 @@ class Variables extends StatelessWidget { void onItemPressed(Variable v, DebuggerController controller) { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { - print('===== A ====='); - v.children.forEach( - (child) => - buildVariablesTree(child, offset: child.offset, count: child.count), - ); + v.children.forEach(buildVariablesTree); } } } @@ -76,11 +72,7 @@ class VariablesList extends StatelessWidget { void onItemPressed(Variable v, DebuggerController controller) { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { - print('====== B ====='); - v.children.forEach( - (child) => - buildVariablesTree(child, offset: child.offset, count: child.count), - ); + v.children.forEach(buildVariablesTree); } } } @@ -119,11 +111,7 @@ class ExpandableVariable extends StatelessWidget { void onItemPressed(Variable v, DebuggerController controller) { // On expansion, lazily build the variables tree for performance reasons. if (v.isExpanded) { - print('======= C ======'); - v.children.forEach( - (child) => - buildVariablesTree(child, offset: child.offset, count: child.count), - ); + v.children.forEach(buildVariablesTree); } } } From b83f2f71bc291319a7834fb28b77ed28a87edb14 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 2 Nov 2021 15:36:43 -0700 Subject: [PATCH 05/19] Add in basic retry logic --- .../lib/src/debugger/codeview.dart | 1 - .../lib/src/debugger/debugger_model.dart | 22 +++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/codeview.dart b/packages/devtools_app/lib/src/debugger/codeview.dart index 8364602987b..7d102dc2fd4 100644 --- a/packages/devtools_app/lib/src/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/debugger/codeview.dart @@ -787,7 +787,6 @@ class _LineItemState extends State { value: response, isolateRef: isolateRef, ); - print('===== ON HOVER, BUILD VARIABLE.'); await buildVariablesTree(variable); if (_hasMouseExited) return; _hoverCard?.remove(); diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index 6a930e09d2d..93b3da240d4 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -406,9 +406,9 @@ Future buildVariablesTree( ); } else if (instanceRef != null && serviceManager.service != null) { try { - final dynamic result = await serviceManager.service.getObject( + final dynamic result = await _getObjectWithRetry( variable.ref.isolateRef.id, instanceRef.id, - offset: variable.offset ?? 0, count: variable.count ?? 0); + offset: variable.offset, count: variable.count); if (result is Instance) { if (result.associations != null) { variable.addAllChildren( @@ -500,6 +500,24 @@ Future buildVariablesTree( variable.treeInitializeComplete = true; } +Future _getObjectWithRetry( + String isolateId, + String objectId, { + int offset, + int count, +}) async { + try { + final dynamic result = await serviceManager.service + .getObject(isolateId, objectId, offset: offset ?? 0, count: count ?? 0); + return result; + } catch (e) { + // TODO(elliette): Remove once https://github.com/dart-lang/webdev/issues/1439 is landed. This fixes + final dynamic result = + await serviceManager.service.getObject(isolateId, objectId); + return result; + } +} + Future _buildVariable( RemoteDiagnosticsNode diagnostic, ObjectGroup inspectorService, From c10ceafa079778b36b421222ad3dee2edbf97723 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 11 Nov 2021 11:11:15 -0800 Subject: [PATCH 06/19] Polish --- .../devtools_app/lib/src/console_service.dart | 1 - .../lib/src/debugger/debugger_model.dart | 83 +++++++++---------- .../lib/src/inspector/diagnostics.dart | 1 - 3 files changed, 39 insertions(+), 46 deletions(-) diff --git a/packages/devtools_app/lib/src/console_service.dart b/packages/devtools_app/lib/src/console_service.dart index 42d5f7a8cec..bd6be18ea57 100644 --- a/packages/devtools_app/lib/src/console_service.dart +++ b/packages/devtools_app/lib/src/console_service.dart @@ -81,7 +81,6 @@ class ConsoleService extends Disposer { isolateRef: isolateRef, ); // TODO(jacobr): fix out of order issues by tracking raw order. - print('build variable in console.'); await buildVariablesTree(variable, expandAll: expandAll); if (expandAll) { variable.expandCascading(); diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index 93b3da240d4..389d70348dd 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -406,9 +406,8 @@ Future buildVariablesTree( ); } else if (instanceRef != null && serviceManager.service != null) { try { - final dynamic result = await _getObjectWithRetry( - variable.ref.isolateRef.id, instanceRef.id, - offset: variable.offset, count: variable.count); + final dynamic result = + await _getObjectWithRetry(instanceRef.id, variable); if (result is Instance) { if (result.associations != null) { variable.addAllChildren( @@ -500,20 +499,21 @@ Future buildVariablesTree( variable.treeInitializeComplete = true; } +// TODO(elliette): Remove once the fix for dart-lang/webdev/issues/1439 +// is landed. This works around a bug in DWDS where an error is thrown if +// `getObject` is called with offset/count for an object that has no length. Future _getObjectWithRetry( - String isolateId, - String objectId, { - int offset, - int count, -}) async { + String objectId, + Variable variable, +) async { try { - final dynamic result = await serviceManager.service - .getObject(isolateId, objectId, offset: offset ?? 0, count: count ?? 0); + final dynamic result = await serviceManager.service.getObject( + variable.ref.isolateRef.id, objectId, + offset: variable.offset, count: variable.childCount); return result; } catch (e) { - // TODO(elliette): Remove once https://github.com/dart-lang/webdev/issues/1439 is landed. This fixes - final dynamic result = - await serviceManager.service.getObject(isolateId, objectId); + final dynamic result = await serviceManager.service + .getObject(variable.ref.isolateRef.id, objectId); return result; } } @@ -703,12 +703,10 @@ List _createVariablesForFields( // TODO(jacobr): consider a new class name. This class is more just the data // model for a tree of Dart objects with properties rather than a "Variable". class Variable extends TreeNode { - Variable._(this.name, ref, this.text, - {int offset, int count, bool isGrouping = false}) + Variable._(this.name, ref, this.text, {int offset, int childCount}) : _ref = ref, _offset = offset, - _count = count, - _isGrouping = isGrouping { + _childCount = childCount { indentChildren = ref?.diagnostic?.style != DiagnosticsTreeStyle.flat; } @@ -761,10 +759,9 @@ class Variable extends TreeNode { return Variable._( null, ref, - '[$offset ... ${offset + count - 1}]', + '[$offset - ${offset + count - 1}]', offset: offset, - count: count, - isGrouping: true, + childCount: count, ); } @@ -775,14 +772,31 @@ class Variable extends TreeNode { GenericInstanceRef get ref => _ref; GenericInstanceRef _ref; - int get offset => _offset; + int get offset { + if (_offset != null) return _offset; + return 0; + } + int _offset; - int get count => _count; - int _count; + int get childCount { + if (_childCount != null) return _childCount; + + final value = this.value; + if (value is InstanceRef) { + if (value.kind == InstanceKind.kList) { + return value.length ?? 0; + } else if (value.kind == InstanceKind.kMap) { + return value.length ?? 0; + } else if (value.kind != null && value.kind.endsWith('List')) { + return value.length ?? 0; + } + } + + return 0; + } - bool get isGrouping => _isGrouping; - bool _isGrouping; + int _childCount; bool treeInitializeStarted = false; bool treeInitializeComplete = false; @@ -803,25 +817,6 @@ class Variable extends TreeNode { Object get value => ref?.value; - int get childCount { - if (isGrouping) { - return count; - } - - final value = this.value; - if (value is InstanceRef) { - if (value.kind == InstanceKind.kList) { - return value.length; - } else if (value.kind == InstanceKind.kMap) { - return value.length; - } else if (value.kind != null && value.kind.endsWith('List')) { - return value.length; - } - } - - return 0; - } - // TODO(kenz): add custom display for lists with more than 100 elements String get displayValue { if (text != null) { diff --git a/packages/devtools_app/lib/src/inspector/diagnostics.dart b/packages/devtools_app/lib/src/inspector/diagnostics.dart index 64182ed020a..009c507d9d2 100644 --- a/packages/devtools_app/lib/src/inspector/diagnostics.dart +++ b/packages/devtools_app/lib/src/inspector/diagnostics.dart @@ -125,7 +125,6 @@ class DiagnosticsNodeDescription extends StatelessWidget { isolateRef: serviceManager.inspectorService.isolateRef, diagnostic: diagnostic, ); - print('------ build variable in diagnostics? -----'); await buildVariablesTree(variable); for (var child in variable.children) { await buildVariablesTree(child); From 75496fd1d1a2ec4f66f68f13fad8f179f16c2877 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 11 Nov 2021 16:49:20 -0800 Subject: [PATCH 07/19] Added tests --- .../test/debugger_screen_test.dart | 376 +++++++++++++----- packages/devtools_test/lib/mocks.dart | 12 + 2 files changed, 278 insertions(+), 110 deletions(-) diff --git a/packages/devtools_app/test/debugger_screen_test.dart b/packages/devtools_app/test/debugger_screen_test.dart index a677dc3f43d..9dc77e5354d 100644 --- a/packages/devtools_app/test/debugger_screen_test.dart +++ b/packages/devtools_app/test/debugger_screen_test.dart @@ -439,54 +439,185 @@ void main() { expect(find.text(''), findsOneWidget); }); - testWidgetsWithWindowSize( - 'Variables shows items', const Size(1000.0, 4000.0), - (WidgetTester tester) async { - when(debuggerController.variables) - .thenReturn(ValueNotifier(testVariables)); - await pumpDebuggerScreen(tester, debuggerController); - expect(find.text('Variables'), findsOneWidget); + group('Variables', () { + setUp(() { + resetRef(); + resetRoot(); + }); - final listFinder = find.selectableText('Root 1: _GrowableList (2 items)'); + testWidgetsWithWindowSize( + 'Variables shows items', const Size(1000.0, 4000.0), + (WidgetTester tester) async { + when(debuggerController.variables).thenReturn( + ValueNotifier( + [ + buildListVariable(), + buildMapVariable(), + buildStringVariable('test str'), + buildBooleanVariable(true), + ], + ), + ); + await pumpDebuggerScreen(tester, debuggerController); + expect(find.text('Variables'), findsOneWidget); - // expect a tooltip for the list value - expect( - find.byTooltip('_GrowableList (2 items)'), - findsOneWidget, - ); + final listFinder = + find.selectableText('Root 1: _GrowableList (2 items)'); - final mapFinder = find - .selectableTextContaining('Root 2: _InternalLinkedHashmap (2 items)'); - final mapElement1Finder = find.selectableTextContaining("['key1']: 1.0"); - final mapElement2Finder = find.selectableTextContaining("['key2']: 1.1"); + // expect a tooltip for the list value + expect( + find.byTooltip('_GrowableList (2 items)'), + findsOneWidget, + ); - expect(listFinder, findsOneWidget); - expect(mapFinder, findsOneWidget); - expect( - find.selectableTextContaining("Root 3: 'test str...'"), - findsOneWidget, - ); - expect( - find.selectableTextContaining('Root 4: true'), - findsOneWidget, + final mapFinder = find.selectableTextContaining( + 'Root 2: _InternalLinkedHashmap (2 items)'); + final mapElement1Finder = + find.selectableTextContaining("['key1']: 1.0"); + final mapElement2Finder = + find.selectableTextContaining("['key2']: 2.0"); + + expect(listFinder, findsOneWidget); + expect(mapFinder, findsOneWidget); + expect( + find.selectableTextContaining("Root 3: 'test str...'"), + findsOneWidget, + ); + expect( + find.selectableTextContaining('Root 4: true'), + findsOneWidget, + ); + + // Expand list. + expect(find.selectableTextContaining('0: 3'), findsNothing); + expect(find.selectableTextContaining('1: 4'), findsNothing); + + await tester.tap(listFinder); + await tester.pump(); + expect(find.selectableTextContaining('0: 0'), findsOneWidget); + expect(find.selectableTextContaining('1: 1'), findsOneWidget); + + // Expand map. + expect(mapElement1Finder, findsNothing); + expect(mapElement2Finder, findsNothing); + await tester.tap(mapFinder); + await tester.pump(); + expect(mapElement1Finder, findsOneWidget); + expect(mapElement2Finder, findsOneWidget); + }); + }); + + testWidgetsWithWindowSize('Children in large list variables are grouped', + const Size(1000.0, 4000.0), (WidgetTester tester) async { + final list = buildParentListVariable(length: 380250); + await buildVariablesTree(list); + when(debuggerController.variables).thenReturn( + ValueNotifier( + [ + list, + ], + ), ); + await pumpDebuggerScreen(tester, debuggerController); - // Expand list. - expect(find.selectableTextContaining('0: 3'), findsNothing); - expect(find.selectableTextContaining('1: 4'), findsNothing); + final listFinder = + find.selectableText('Root 1: _GrowableList (380,250 items)'); + final group0To9999Finder = find.selectableTextContaining('[0 - 9999]'); + final group10000To19999Finder = + find.selectableTextContaining('[10000 - 19999]'); + final group370000To379999Finder = + find.selectableTextContaining('[370000 - 379999]'); + final group380000To380249Finder = + find.selectableTextContaining('[380000 - 380249]'); + + final group370000To370099Finder = + find.selectableTextContaining('[370000 - 370099]'); + final group370100To370199Finder = + find.selectableTextContaining('[370100 - 370199]'); + final group370200To370299Finder = + find.selectableTextContaining('[370200 - 370299]'); + + // Initially list is not expanded. + expect(listFinder, findsOneWidget); + expect(group0To9999Finder, findsNothing); + expect(group10000To19999Finder, findsNothing); + expect(group370000To379999Finder, findsNothing); + expect(group380000To380249Finder, findsNothing); + // Expand list. await tester.tap(listFinder); await tester.pump(); - expect(find.selectableTextContaining('0: 3'), findsOneWidget); - expect(find.selectableTextContaining('1: 4'), findsOneWidget); + expect(group0To9999Finder, findsOneWidget); + expect(group10000To19999Finder, findsOneWidget); + expect(group370000To379999Finder, findsOneWidget); + expect(group380000To380249Finder, findsOneWidget); + + // Initially group [370000 - 379999] is not expanded. + expect(group370000To370099Finder, findsNothing); + expect(group370100To370199Finder, findsNothing); + expect(group370200To370299Finder, findsNothing); + + // Expand group [370000 - 379999]. + await tester.tap(group370000To379999Finder); + await tester.pump(); + expect(group370000To370099Finder, findsOneWidget); + expect(group370100To370199Finder, findsOneWidget); + expect(group370200To370299Finder, findsOneWidget); + }); + + testWidgetsWithWindowSize('Children in large map variables are grouped', + const Size(1000.0, 4000.0), (WidgetTester tester) async { + final map = buildParentMapVariable(length: 243621); + await buildVariablesTree(map); + when(debuggerController.variables).thenReturn( + ValueNotifier( + [ + map, + ], + ), + ); + await pumpDebuggerScreen(tester, debuggerController); + + final listFinder = + find.selectableText('Root 1: _InternalLinkedHashmap (243,621 items)'); + final group0To9999Finder = find.selectableTextContaining('[0 - 9999]'); + final group10000To19999Finder = + find.selectableTextContaining('[10000 - 19999]'); + final group230000To239999Finder = + find.selectableTextContaining('[230000 - 239999]'); + final group240000To243620Finder = + find.selectableTextContaining('[240000 - 243620]'); + + final group0To99Finder = find.selectableTextContaining('[0 - 99]'); + final group100To199Finder = find.selectableTextContaining('[100 - 199]'); + final group200To299Finder = find.selectableTextContaining('[200 - 299]'); + + // Initially map is not expanded. + expect(listFinder, findsOneWidget); + expect(group0To9999Finder, findsNothing); + expect(group10000To19999Finder, findsNothing); + expect(group230000To239999Finder, findsNothing); + expect(group240000To243620Finder, findsNothing); // Expand map. - expect(mapElement1Finder, findsNothing); - expect(mapElement2Finder, findsNothing); - await tester.tap(mapFinder); + await tester.tap(listFinder); + await tester.pump(); + expect(group0To9999Finder, findsOneWidget); + expect(group10000To19999Finder, findsOneWidget); + expect(group230000To239999Finder, findsOneWidget); + expect(group240000To243620Finder, findsOneWidget); + + // Initially group [0 - 9999] is not expanded. + expect(group0To99Finder, findsNothing); + expect(group100To199Finder, findsNothing); + expect(group200To299Finder, findsNothing); + + // Expand group [0 - 9999]. + await tester.tap(group0To9999Finder); await tester.pump(); - expect(mapElement1Finder, findsOneWidget); - expect(mapElement2Finder, findsOneWidget); + expect(group0To99Finder, findsOneWidget); + expect(group100To199Finder, findsOneWidget); + expect(group200To299Finder, findsOneWidget); }); WidgetPredicate createDebuggerButtonPredicate(String title) { @@ -669,19 +800,41 @@ final isolateRef = IsolateRef( isSystemIsolate: false, ); -final testVariables = [ - Variable.create( +int refNumber = 0; + +String incrementRef() { + refNumber = refNumber + 1; + return 'ref$refNumber'; +} + +void resetRef() { + refNumber = 0; +} + +int rootNumber = 0; + +String incrementRoot() { + rootNumber = rootNumber + 1; + return 'Root $rootNumber'; +} + +void resetRoot() { + rootNumber = 0; +} + +Variable buildParentListVariable({int length = 2}) { + return Variable.create( BoundVariable( - name: 'Root 1', + name: incrementRoot(), value: InstanceRef( - id: 'ref1', + id: incrementRef(), kind: InstanceKind.kList, classRef: ClassRef( name: '_GrowableList', - id: 'ref2', + id: incrementRef(), library: libraryRef, ), - length: 2, + length: length, identityHashCode: null, ), declarationTokenPos: null, @@ -689,34 +842,23 @@ final testVariables = [ scopeStartTokenPos: null, ), isolateRef, - )..addAllChildren([ - Variable.create( - BoundVariable( - name: '0', - value: InstanceRef( - id: 'ref3', - kind: InstanceKind.kInt, - classRef: - ClassRef(name: 'Integer', id: 'ref4', library: libraryRef), - valueAsString: '3', - valueAsStringIsTruncated: false, - identityHashCode: null, - ), - declarationTokenPos: null, - scopeEndTokenPos: null, - scopeStartTokenPos: null, - ), - isolateRef, - ), + ); +} + +Variable buildListVariable({int length = 2}) { + final listVariable = buildParentListVariable(length: length); + + for (int i = 0; i < length; i++) { + listVariable.addChild( Variable.create( BoundVariable( - name: '1', + name: '$i', value: InstanceRef( - id: 'ref5', + id: incrementRef(), kind: InstanceKind.kInt, - classRef: - ClassRef(name: 'Integer', id: 'ref6', library: libraryRef), - valueAsString: '4', + classRef: ClassRef( + name: 'Integer', id: incrementRef(), library: libraryRef), + valueAsString: '$i', valueAsStringIsTruncated: false, identityHashCode: null, ), @@ -726,16 +868,24 @@ final testVariables = [ ), isolateRef, ), - ]), - Variable.create( + ); + } + + return listVariable; +} + +Variable buildParentMapVariable({int length = 2}) { + return Variable.create( BoundVariable( - name: 'Root 2', + name: incrementRoot(), value: InstanceRef( - id: 'ref7', + id: incrementRef(), kind: InstanceKind.kMap, classRef: ClassRef( - name: '_InternalLinkedHashmap', id: 'ref8', library: libraryRef), - length: 2, + name: '_InternalLinkedHashmap', + id: incrementRef(), + library: libraryRef), + length: length, identityHashCode: null, ), declarationTokenPos: null, @@ -743,34 +893,23 @@ final testVariables = [ scopeStartTokenPos: null, ), isolateRef, - )..addAllChildren([ - Variable.create( - BoundVariable( - name: "['key1']", - value: InstanceRef( - id: 'ref9', - kind: InstanceKind.kDouble, - classRef: - ClassRef(name: 'Double', id: 'ref10', library: libraryRef), - valueAsString: '1.0', - valueAsStringIsTruncated: false, - identityHashCode: null, - ), - declarationTokenPos: null, - scopeEndTokenPos: null, - scopeStartTokenPos: null, - ), - isolateRef, - ), + ); +} + +Variable buildMapVariable({int length = 2}) { + final mapVariable = buildParentMapVariable(length: length); + + for (int i = 0; i < length; i++) { + mapVariable.addChild( Variable.create( BoundVariable( - name: "['key2']", + name: "['key${i + 1}']", value: InstanceRef( - id: 'ref11', + id: incrementRef(), kind: InstanceKind.kDouble, - classRef: - ClassRef(name: 'Double', id: 'ref12', library: libraryRef), - valueAsString: '1.1', + classRef: ClassRef( + name: 'Double', id: incrementRef(), library: libraryRef), + valueAsString: '${i + 1}.0', valueAsStringIsTruncated: false, identityHashCode: null, ), @@ -780,15 +919,25 @@ final testVariables = [ ), isolateRef, ), - ]), - Variable.create( + ); + } + + return mapVariable; +} + +Variable buildStringVariable(String value) { + return Variable.create( BoundVariable( - name: 'Root 3', + name: incrementRoot(), value: InstanceRef( - id: 'ref13', + id: incrementRef(), kind: InstanceKind.kString, - classRef: ClassRef(name: 'String', id: 'ref14', library: libraryRef), - valueAsString: 'test str', + classRef: ClassRef( + name: 'String', + id: incrementRef(), + library: libraryRef, + ), + valueAsString: value, valueAsStringIsTruncated: true, identityHashCode: null, ), @@ -797,15 +946,22 @@ final testVariables = [ scopeStartTokenPos: null, ), isolateRef, - ), - Variable.create( + ); +} + +Variable buildBooleanVariable(bool value) { + return Variable.create( BoundVariable( - name: 'Root 4', + name: incrementRoot(), value: InstanceRef( - id: 'ref15', + id: incrementRef(), kind: InstanceKind.kBool, - classRef: ClassRef(name: 'Boolean', id: 'ref16', library: libraryRef), - valueAsString: 'true', + classRef: ClassRef( + name: 'Boolean', + id: incrementRef(), + library: libraryRef, + ), + valueAsString: '$value', valueAsStringIsTruncated: false, identityHashCode: null, ), @@ -814,5 +970,5 @@ final testVariables = [ scopeStartTokenPos: null, ), isolateRef, - ), -]; + ); +} diff --git a/packages/devtools_test/lib/mocks.dart b/packages/devtools_test/lib/mocks.dart index 5b8a9ed732a..9a736af6672 100644 --- a/packages/devtools_test/lib/mocks.dart +++ b/packages/devtools_test/lib/mocks.dart @@ -366,6 +366,16 @@ class FakeVmService extends Fake implements VmServiceWrapper { return Future.value(MockIsolate()); } + @override + Future getObject( + String isolateId, + String objectId, { + int offset, + int count, + }) { + return Future.value(MockObj()); + } + @override Future getMemoryUsage(String isolateId) async { if (_memoryData == null) { @@ -644,6 +654,8 @@ class MockVmService extends Mock implements VmServiceWrapper {} class MockIsolate extends Mock implements Isolate {} +class MockObj extends Mock implements Obj {} + class MockConnectedApp extends Mock implements ConnectedApp {} class FakeConnectedApp extends Mock implements ConnectedApp {} From 6bd3ce5aa33e4a1938d9c5f24a3705f8d0136690 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 12 Nov 2021 10:59:30 -0800 Subject: [PATCH 08/19] Updated typed_data_variable_test --- .../lib/src/debugger/debugger_model.dart | 7 +- .../lib/src/debugger/variables.dart | 3 +- .../test/typed_data_variable_test.dart | 142 ++++++++++++++++-- 3 files changed, 135 insertions(+), 17 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index ce3b38b8129..b7190f07ecb 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -463,8 +463,8 @@ Future buildVariablesTree( // TODO(jacobr): cache the full class hierarchy so we can cheaply check // instanceRef is DiagnosticsNode without having to do an eval. if (instanceRef != null && - (instanceRef.classRef.name == 'DiagnosticableTreeNode' || - instanceRef.classRef.name == 'DiagnosticsProperty')) { + (instanceRef.classRef?.name == 'DiagnosticableTreeNode' || + instanceRef.classRef?.name == 'DiagnosticsProperty')) { // The user is expecting to see the object the DiagnosticsNode is // describing not the DiagnosticsNode itself. try { @@ -789,6 +789,9 @@ class Variable extends TreeNode { } else if (value.kind == InstanceKind.kMap) { return value.length ?? 0; } else if (value.kind != null && value.kind.endsWith('List')) { + // Catch-all for Uint8ClampedList, Uint8List, Uint16List, Uint32List, + // Uint64List, Int8List, Int16List, Int32List, Int64List, Flooat32List, + // Float64List, Inst32x3List, Float32x4List, and Float64x2List types: return value.length ?? 0; } } diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index fd8230e331d..fd976c1d4f7 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -221,7 +221,8 @@ class VariableSelectionControls extends MaterialTextSelectionControls { endpoints: endpoints, delegate: delegate, clipboardStatus: clipboardStatus, - handleCut: canCut(delegate) ? () => handleCut(delegate) : null, + handleCut: + canCut(delegate) ? () => handleCut(delegate, clipboardStatus) : null, handleCopy: canCopy(delegate) ? () => handleCopy(delegate, clipboardStatus) : null, diff --git a/packages/devtools_app/test/typed_data_variable_test.dart b/packages/devtools_app/test/typed_data_variable_test.dart index 0e635d44da2..8165a4d498b 100644 --- a/packages/devtools_app/test/typed_data_variable_test.dart +++ b/packages/devtools_app/test/typed_data_variable_test.dart @@ -53,6 +53,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -64,7 +65,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -86,6 +88,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -97,7 +100,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -119,6 +123,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -130,7 +135,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -152,6 +158,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -163,7 +170,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -185,6 +193,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -196,7 +205,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -218,6 +228,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -229,7 +240,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -251,6 +263,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -262,7 +275,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -284,6 +298,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -295,7 +310,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -317,6 +333,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -328,7 +345,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -351,6 +369,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( BoundVariable( @@ -362,7 +381,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -383,6 +403,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( @@ -395,7 +416,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -417,6 +439,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( @@ -429,7 +452,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); await buildVariablesTree(variable); @@ -451,6 +475,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( @@ -463,7 +488,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -485,6 +511,7 @@ void main() { classRef: null, bytes: base64.encode(bytes.buffer.asUint8List()), identityHashCode: null, + length: 4, ); final variable = Variable.create( @@ -497,7 +524,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { return instance; }); @@ -508,6 +536,81 @@ void main() { expect(variable.children.first.displayValue, '[0, -1232.222]', skip: !kIsWeb); }); + + test('Retries getObject calls with no offset/count if error is thrown', + () async { + final bytes = Uint8List.fromList([0, 1, 2, 3]); + final instance = Instance( + kind: InstanceKind.kUint8List, + id: '123', + classRef: null, + bytes: base64.encode(bytes.buffer.asUint8List()), + identityHashCode: null, + length: 4, + ); + final variable = Variable.create( + BoundVariable( + name: 'test', + value: instance, + declarationTokenPos: null, + scopeEndTokenPos: null, + scopeStartTokenPos: null, + ), + isolateRef, + ); + + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenThrow('Unrecognized parameters offset / count.'); + + when(manager.service.getObject(any, any)).thenAnswer((_) async { + return instance; + }); + + await buildVariablesTree(variable); + + expect(variable.children, [ + matchesVariable(name: '[0]', value: 0), + matchesVariable(name: '[1]', value: 1), + matchesVariable(name: '[2]', value: 2), + matchesVariable(name: '[3]', value: 3), + ]); + }); + + test( + 'Creates bound variable with groupings for children for a large Uint8ClampedList instance', + () async { + final instance = Instance( + kind: InstanceKind.kUint8ClampedList, + id: '123', + classRef: null, + bytes: null, + identityHashCode: null, + length: 332, + ); + final variable = Variable.create( + BoundVariable( + name: 'test', + value: instance, + declarationTokenPos: null, + scopeEndTokenPos: null, + scopeStartTokenPos: null, + ), + isolateRef, + ); + when(manager.service.getObject(any, any, offset: 0, count: 4)) + .thenAnswer((_) async { + return instance; + }); + + await buildVariablesTree(variable); + + expect(variable.children, [ + matchesVariableGroup(start: 0, end: 99), + matchesVariableGroup(start: 100, end: 199), + matchesVariableGroup(start: 200, end: 299), + matchesVariableGroup(start: 300, end: 331), + ]); + }); } Matcher matchesVariable({ @@ -521,3 +624,14 @@ Matcher matchesVariable({ .having((v) => v.name, 'name', equals(name)) .having((v) => v.ref.value, 'value', equals(value))); } + +Matcher matchesVariableGroup({ + @required int start, + @required int end, +}) { + return const TypeMatcher().having( + (v) => v, + 'boundVar', + const TypeMatcher() + .having((v) => v.text, 'text', equals('[$start - $end]'))); +} From bca0cf50407c71ab2c89256f1a3d10a224a08adf Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 12 Nov 2021 11:47:01 -0800 Subject: [PATCH 09/19] Remove clipboardStatus --- packages/devtools_app/lib/src/debugger/variables.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index fd976c1d4f7..fd8230e331d 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -221,8 +221,7 @@ class VariableSelectionControls extends MaterialTextSelectionControls { endpoints: endpoints, delegate: delegate, clipboardStatus: clipboardStatus, - handleCut: - canCut(delegate) ? () => handleCut(delegate, clipboardStatus) : null, + handleCut: canCut(delegate) ? () => handleCut(delegate) : null, handleCopy: canCopy(delegate) ? () => handleCopy(delegate, clipboardStatus) : null, From 3e14186ac869be169324cf9b9ebc6f830ccd0c27 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:36:40 -0800 Subject: [PATCH 10/19] Update association_variable_test --- .../devtools_app/test/association_variable_test.dart | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/devtools_app/test/association_variable_test.dart b/packages/devtools_app/test/association_variable_test.dart index e3bf76b7c24..567407fe5b1 100644 --- a/packages/devtools_app/test/association_variable_test.dart +++ b/packages/devtools_app/test/association_variable_test.dart @@ -47,6 +47,7 @@ void main() { kind: InstanceKind.kMap, id: '123', classRef: null, + length: 2, associations: [ MapAssociation( key: InstanceRef( @@ -83,7 +84,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 2)) + .thenAnswer((_) async { return instance; }); @@ -111,6 +113,7 @@ void main() { kind: InstanceKind.kMap, id: '123', classRef: null, + length: 2, associations: [ MapAssociation( key: InstanceRef( @@ -141,7 +144,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 2)) + .thenAnswer((_) async { return instance; }); @@ -168,6 +172,7 @@ void main() { kind: InstanceKind.kMap, id: '123', classRef: null, + length: 2, associations: [ MapAssociation( key: InstanceRef( @@ -197,7 +202,8 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any)).thenAnswer((_) async { + when(manager.service.getObject(any, any, offset: 0, count: 2)) + .thenAnswer((_) async { return instance; }); From 381604aefc2bf22a4c7d1c78092227fe9a345f60 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 12 Nov 2021 13:51:23 -0800 Subject: [PATCH 11/19] Fix nesting --- .../test/debugger_screen_test.dart | 220 +++++++++--------- 1 file changed, 111 insertions(+), 109 deletions(-) diff --git a/packages/devtools_app/test/debugger_screen_test.dart b/packages/devtools_app/test/debugger_screen_test.dart index 9d93e7f9c18..620a3a94946 100644 --- a/packages/devtools_app/test/debugger_screen_test.dart +++ b/packages/devtools_app/test/debugger_screen_test.dart @@ -513,119 +513,121 @@ void main() { expect(mapElement1Finder, findsOneWidget); expect(mapElement2Finder, findsOneWidget); }); - }); - testWidgetsWithWindowSize('Children in large list variables are grouped', - const Size(1000.0, 4000.0), (WidgetTester tester) async { - final list = buildParentListVariable(length: 380250); - await buildVariablesTree(list); - when(debuggerController.variables).thenReturn( - ValueNotifier( - [ - list, - ], - ), - ); - await pumpDebuggerScreen(tester, debuggerController); + testWidgetsWithWindowSize('Children in large list variables are grouped', + const Size(1000.0, 4000.0), (WidgetTester tester) async { + final list = buildParentListVariable(length: 380250); + await buildVariablesTree(list); + when(debuggerController.variables).thenReturn( + ValueNotifier( + [ + list, + ], + ), + ); + await pumpDebuggerScreen(tester, debuggerController); - final listFinder = - find.selectableText('Root 1: _GrowableList (380,250 items)'); - final group0To9999Finder = find.selectableTextContaining('[0 - 9999]'); - final group10000To19999Finder = - find.selectableTextContaining('[10000 - 19999]'); - final group370000To379999Finder = - find.selectableTextContaining('[370000 - 379999]'); - final group380000To380249Finder = - find.selectableTextContaining('[380000 - 380249]'); - - final group370000To370099Finder = - find.selectableTextContaining('[370000 - 370099]'); - final group370100To370199Finder = - find.selectableTextContaining('[370100 - 370199]'); - final group370200To370299Finder = - find.selectableTextContaining('[370200 - 370299]'); - - // Initially list is not expanded. - expect(listFinder, findsOneWidget); - expect(group0To9999Finder, findsNothing); - expect(group10000To19999Finder, findsNothing); - expect(group370000To379999Finder, findsNothing); - expect(group380000To380249Finder, findsNothing); - - // Expand list. - await tester.tap(listFinder); - await tester.pump(); - expect(group0To9999Finder, findsOneWidget); - expect(group10000To19999Finder, findsOneWidget); - expect(group370000To379999Finder, findsOneWidget); - expect(group380000To380249Finder, findsOneWidget); - - // Initially group [370000 - 379999] is not expanded. - expect(group370000To370099Finder, findsNothing); - expect(group370100To370199Finder, findsNothing); - expect(group370200To370299Finder, findsNothing); - - // Expand group [370000 - 379999]. - await tester.tap(group370000To379999Finder); - await tester.pump(); - expect(group370000To370099Finder, findsOneWidget); - expect(group370100To370199Finder, findsOneWidget); - expect(group370200To370299Finder, findsOneWidget); - }); + final listFinder = + find.selectableText('Root 1: _GrowableList (380,250 items)'); + final group0To9999Finder = find.selectableTextContaining('[0 - 9999]'); + final group10000To19999Finder = + find.selectableTextContaining('[10000 - 19999]'); + final group370000To379999Finder = + find.selectableTextContaining('[370000 - 379999]'); + final group380000To380249Finder = + find.selectableTextContaining('[380000 - 380249]'); + + final group370000To370099Finder = + find.selectableTextContaining('[370000 - 370099]'); + final group370100To370199Finder = + find.selectableTextContaining('[370100 - 370199]'); + final group370200To370299Finder = + find.selectableTextContaining('[370200 - 370299]'); + + // Initially list is not expanded. + expect(listFinder, findsOneWidget); + expect(group0To9999Finder, findsNothing); + expect(group10000To19999Finder, findsNothing); + expect(group370000To379999Finder, findsNothing); + expect(group380000To380249Finder, findsNothing); - testWidgetsWithWindowSize('Children in large map variables are grouped', - const Size(1000.0, 4000.0), (WidgetTester tester) async { - final map = buildParentMapVariable(length: 243621); - await buildVariablesTree(map); - when(debuggerController.variables).thenReturn( - ValueNotifier( - [ - map, - ], - ), - ); - await pumpDebuggerScreen(tester, debuggerController); + // Expand list. + await tester.tap(listFinder); + await tester.pump(); + expect(group0To9999Finder, findsOneWidget); + expect(group10000To19999Finder, findsOneWidget); + expect(group370000To379999Finder, findsOneWidget); + expect(group380000To380249Finder, findsOneWidget); + + // Initially group [370000 - 379999] is not expanded. + expect(group370000To370099Finder, findsNothing); + expect(group370100To370199Finder, findsNothing); + expect(group370200To370299Finder, findsNothing); + + // Expand group [370000 - 379999]. + await tester.tap(group370000To379999Finder); + await tester.pump(); + expect(group370000To370099Finder, findsOneWidget); + expect(group370100To370199Finder, findsOneWidget); + expect(group370200To370299Finder, findsOneWidget); + }); - final listFinder = - find.selectableText('Root 1: _InternalLinkedHashmap (243,621 items)'); - final group0To9999Finder = find.selectableTextContaining('[0 - 9999]'); - final group10000To19999Finder = - find.selectableTextContaining('[10000 - 19999]'); - final group230000To239999Finder = - find.selectableTextContaining('[230000 - 239999]'); - final group240000To243620Finder = - find.selectableTextContaining('[240000 - 243620]'); - - final group0To99Finder = find.selectableTextContaining('[0 - 99]'); - final group100To199Finder = find.selectableTextContaining('[100 - 199]'); - final group200To299Finder = find.selectableTextContaining('[200 - 299]'); - - // Initially map is not expanded. - expect(listFinder, findsOneWidget); - expect(group0To9999Finder, findsNothing); - expect(group10000To19999Finder, findsNothing); - expect(group230000To239999Finder, findsNothing); - expect(group240000To243620Finder, findsNothing); - - // Expand map. - await tester.tap(listFinder); - await tester.pump(); - expect(group0To9999Finder, findsOneWidget); - expect(group10000To19999Finder, findsOneWidget); - expect(group230000To239999Finder, findsOneWidget); - expect(group240000To243620Finder, findsOneWidget); - - // Initially group [0 - 9999] is not expanded. - expect(group0To99Finder, findsNothing); - expect(group100To199Finder, findsNothing); - expect(group200To299Finder, findsNothing); - - // Expand group [0 - 9999]. - await tester.tap(group0To9999Finder); - await tester.pump(); - expect(group0To99Finder, findsOneWidget); - expect(group100To199Finder, findsOneWidget); - expect(group200To299Finder, findsOneWidget); + testWidgetsWithWindowSize('Children in large map variables are grouped', + const Size(1000.0, 4000.0), (WidgetTester tester) async { + final map = buildParentMapVariable(length: 243621); + await buildVariablesTree(map); + when(debuggerController.variables).thenReturn( + ValueNotifier( + [ + map, + ], + ), + ); + await pumpDebuggerScreen(tester, debuggerController); + + final listFinder = find + .selectableText('Root 1: _InternalLinkedHashmap (243,621 items)'); + final group0To9999Finder = find.selectableTextContaining('[0 - 9999]'); + final group10000To19999Finder = + find.selectableTextContaining('[10000 - 19999]'); + final group230000To239999Finder = + find.selectableTextContaining('[230000 - 239999]'); + final group240000To243620Finder = + find.selectableTextContaining('[240000 - 243620]'); + + final group0To99Finder = find.selectableTextContaining('[0 - 99]'); + final group100To199Finder = + find.selectableTextContaining('[100 - 199]'); + final group200To299Finder = + find.selectableTextContaining('[200 - 299]'); + + // Initially map is not expanded. + expect(listFinder, findsOneWidget); + expect(group0To9999Finder, findsNothing); + expect(group10000To19999Finder, findsNothing); + expect(group230000To239999Finder, findsNothing); + expect(group240000To243620Finder, findsNothing); + + // Expand map. + await tester.tap(listFinder); + await tester.pump(); + expect(group0To9999Finder, findsOneWidget); + expect(group10000To19999Finder, findsOneWidget); + expect(group230000To239999Finder, findsOneWidget); + expect(group240000To243620Finder, findsOneWidget); + + // Initially group [0 - 9999] is not expanded. + expect(group0To99Finder, findsNothing); + expect(group100To199Finder, findsNothing); + expect(group200To299Finder, findsNothing); + + // Expand group [0 - 9999]. + await tester.tap(group0To9999Finder); + await tester.pump(); + expect(group0To99Finder, findsOneWidget); + expect(group100To199Finder, findsOneWidget); + expect(group200To299Finder, findsOneWidget); + }); }); WidgetPredicate createDebuggerButtonPredicate(String title) { From 05eda968458bdd73942fddb03acd9f2773bd2584 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 12 Nov 2021 14:07:21 -0800 Subject: [PATCH 12/19] Remove redundant arg --- packages/devtools_app/test/typed_data_variable_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/devtools_app/test/typed_data_variable_test.dart b/packages/devtools_app/test/typed_data_variable_test.dart index 8165a4d498b..06f031e4259 100644 --- a/packages/devtools_app/test/typed_data_variable_test.dart +++ b/packages/devtools_app/test/typed_data_variable_test.dart @@ -583,7 +583,6 @@ void main() { kind: InstanceKind.kUint8ClampedList, id: '123', classRef: null, - bytes: null, identityHashCode: null, length: 332, ); From eb4084a1f9fe08763abcfe6d07130ee35f581c91 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 15 Nov 2021 12:43:23 -0800 Subject: [PATCH 13/19] Update in response to comments --- .../lib/src/debugger/debugger_model.dart | 14 ++++++------- packages/devtools_app/lib/src/utils.dart | 7 ++++--- .../lib/src/vm_service_wrapper.dart | 8 +++++-- .../test/typed_data_variable_test.dart | 21 +++++++++++++++++-- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index b7190f07ecb..fe5bcea279a 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:convert'; +import 'dart:math'; import 'dart:typed_data'; import 'package:flutter/foundation.dart'; @@ -382,13 +383,12 @@ Future buildVariablesTree( } if (variable.childCount > Variable.MAX_CHILDREN_IN_GROUPING) { - final numChildrenInGrouping = variable.childCount >= - Variable.MAX_CHILDREN_IN_GROUPING * - Variable.MAX_CHILDREN_IN_GROUPING - ? (roundToNearestPow10(variable.childCount) / - Variable.MAX_CHILDREN_IN_GROUPING) - .floor() - : Variable.MAX_CHILDREN_IN_GROUPING; + final numChildrenInGrouping = + variable.childCount >= pow(Variable.MAX_CHILDREN_IN_GROUPING, 2) + ? (roundToNearestPow10(variable.childCount) / + Variable.MAX_CHILDREN_IN_GROUPING) + .floor() + : Variable.MAX_CHILDREN_IN_GROUPING; var index = 0; while (index + numChildrenInGrouping < variable.childCount) { diff --git a/packages/devtools_app/lib/src/utils.dart b/packages/devtools_app/lib/src/utils.dart index f4fc99f25d3..b5ee29a0f22 100644 --- a/packages/devtools_app/lib/src/utils.dart +++ b/packages/devtools_app/lib/src/utils.dart @@ -188,13 +188,14 @@ T nullSafeMax(T a, T b) { return max(a, b); } -double logBase(num x, num base) { +double logBase({@required int x, @required int base}) { return log(x) / log(base); } -int log2(num x) => (logBase(x, 2)).floor(); +int log2(num x) => (logBase(x: x, base: 2)).floor(); -int roundToNearestPow10(num x) => pow(10, logBase(x, 10).ceil()).floor(); +int roundToNearestPow10(num x) => + pow(10, logBase(x: x, base: 10).ceil()).floor(); String isolateName(IsolateRef ref) { // analysis_server.dart.snapshot$main diff --git a/packages/devtools_app/lib/src/vm_service_wrapper.dart b/packages/devtools_app/lib/src/vm_service_wrapper.dart index f4cab2b5bda..152f18591b7 100644 --- a/packages/devtools_app/lib/src/vm_service_wrapper.dart +++ b/packages/devtools_app/lib/src/vm_service_wrapper.dart @@ -403,8 +403,12 @@ class VmServiceWrapper implements VmService { }) { return trackFuture( 'getObject', - _vmService.getObject(isolateId, objectId, - offset: offset, count: count)); + _vmService.getObject( + isolateId, + objectId, + offset: offset, + count: count, + )); } @override diff --git a/packages/devtools_app/test/typed_data_variable_test.dart b/packages/devtools_app/test/typed_data_variable_test.dart index 06f031e4259..eb7abd2b953 100644 --- a/packages/devtools_app/test/typed_data_variable_test.dart +++ b/packages/devtools_app/test/typed_data_variable_test.dart @@ -559,8 +559,12 @@ void main() { isolateRef, ); - when(manager.service.getObject(any, any, offset: 0, count: 4)) - .thenThrow('Unrecognized parameters offset / count.'); + when(manager.service.getObject( + any, + any, + offset: 0, + count: 4, + )).thenThrow('Unrecognized parameters offset / count.'); when(manager.service.getObject(any, any)).thenAnswer((_) async { return instance; @@ -574,6 +578,19 @@ void main() { matchesVariable(name: '[2]', value: 2), matchesVariable(name: '[3]', value: 3), ]); + + verifyInOrder([ + manager.service.getObject( + any, + any, + offset: 0, + count: 4, + ), + manager.service.getObject( + any, + any, + ), + ]); }); test( From 69577825c42af92be0856b1eb49e46443867fc15 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 15 Nov 2021 13:00:51 -0800 Subject: [PATCH 14/19] Floor double to make it an int --- packages/devtools_app/lib/src/debugger/variables.dart | 3 ++- packages/devtools_app/lib/src/utils.dart | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index fd8230e331d..fd976c1d4f7 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -221,7 +221,8 @@ class VariableSelectionControls extends MaterialTextSelectionControls { endpoints: endpoints, delegate: delegate, clipboardStatus: clipboardStatus, - handleCut: canCut(delegate) ? () => handleCut(delegate) : null, + handleCut: + canCut(delegate) ? () => handleCut(delegate, clipboardStatus) : null, handleCopy: canCopy(delegate) ? () => handleCopy(delegate, clipboardStatus) : null, diff --git a/packages/devtools_app/lib/src/utils.dart b/packages/devtools_app/lib/src/utils.dart index b5ee29a0f22..9030ec2bc1a 100644 --- a/packages/devtools_app/lib/src/utils.dart +++ b/packages/devtools_app/lib/src/utils.dart @@ -192,7 +192,7 @@ double logBase({@required int x, @required int base}) { return log(x) / log(base); } -int log2(num x) => (logBase(x: x, base: 2)).floor(); +int log2(num x) => (logBase(x: x.floor(), base: 2)).floor(); int roundToNearestPow10(num x) => pow(10, logBase(x: x, base: 10).ceil()).floor(); From 22447d4eaa12af91e3d188d5552af7a6740dbf0a Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 15 Nov 2021 13:48:32 -0800 Subject: [PATCH 15/19] Remove clipboardStatus --- packages/devtools_app/lib/src/debugger/variables.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/variables.dart b/packages/devtools_app/lib/src/debugger/variables.dart index fd976c1d4f7..fd8230e331d 100644 --- a/packages/devtools_app/lib/src/debugger/variables.dart +++ b/packages/devtools_app/lib/src/debugger/variables.dart @@ -221,8 +221,7 @@ class VariableSelectionControls extends MaterialTextSelectionControls { endpoints: endpoints, delegate: delegate, clipboardStatus: clipboardStatus, - handleCut: - canCut(delegate) ? () => handleCut(delegate, clipboardStatus) : null, + handleCut: canCut(delegate) ? () => handleCut(delegate) : null, handleCopy: canCopy(delegate) ? () => handleCopy(delegate, clipboardStatus) : null, From 0db2a627a3405e002433f0aa618707a91cae2496 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 15 Nov 2021 14:05:41 -0800 Subject: [PATCH 16/19] Switch roundToNearestPow10 to take an int as well --- packages/devtools_app/lib/src/utils.dart | 2 +- packages/devtools_app/test/utils_test.dart | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/src/utils.dart b/packages/devtools_app/lib/src/utils.dart index 9030ec2bc1a..1f604abb520 100644 --- a/packages/devtools_app/lib/src/utils.dart +++ b/packages/devtools_app/lib/src/utils.dart @@ -194,7 +194,7 @@ double logBase({@required int x, @required int base}) { int log2(num x) => (logBase(x: x.floor(), base: 2)).floor(); -int roundToNearestPow10(num x) => +int roundToNearestPow10(int x) => pow(10, logBase(x: x, base: 10).ceil()).floor(); String isolateName(IsolateRef ref) { diff --git a/packages/devtools_app/test/utils_test.dart b/packages/devtools_app/test/utils_test.dart index 6a22316ba94..df4ae3242df 100644 --- a/packages/devtools_app/test/utils_test.dart +++ b/packages/devtools_app/test/utils_test.dart @@ -109,7 +109,6 @@ void main() { }); test('roundToNearestPow10', () { - expect(roundToNearestPow10(0.1), equals(1)); expect(roundToNearestPow10(1), equals(1)); expect(roundToNearestPow10(2), equals(10)); expect(roundToNearestPow10(10), equals(10)); From a987245bb2a644c0387bb531e462404224637ae7 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 15 Nov 2021 19:27:27 -0800 Subject: [PATCH 17/19] Modified loop and added test case --- .../lib/src/debugger/debugger_model.dart | 17 ++++------ .../test/typed_data_variable_test.dart | 33 ++++++++++++++++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index fe5bcea279a..4655bcc1e1e 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -390,20 +390,15 @@ Future buildVariablesTree( .floor() : Variable.MAX_CHILDREN_IN_GROUPING; - var index = 0; - while (index + numChildrenInGrouping < variable.childCount) { + var start = variable.offset ?? 0; + final end = start + variable.childCount; + while (start < end) { + final count = min(end - start, numChildrenInGrouping); variable.addChild( - Variable.grouping(variable.ref, - offset: index + (variable.offset ?? 0), - count: numChildrenInGrouping), + Variable.grouping(variable.ref, offset: start, count: count), ); - index += numChildrenInGrouping; + start += count; } - variable.addChild( - Variable.grouping(variable.ref, - offset: index + (variable.offset ?? 0), - count: variable.childCount - index), - ); } else if (instanceRef != null && serviceManager.service != null) { try { final dynamic result = diff --git a/packages/devtools_app/test/typed_data_variable_test.dart b/packages/devtools_app/test/typed_data_variable_test.dart index eb7abd2b953..bb8df5fabe4 100644 --- a/packages/devtools_app/test/typed_data_variable_test.dart +++ b/packages/devtools_app/test/typed_data_variable_test.dart @@ -613,10 +613,6 @@ void main() { ), isolateRef, ); - when(manager.service.getObject(any, any, offset: 0, count: 4)) - .thenAnswer((_) async { - return instance; - }); await buildVariablesTree(variable); @@ -627,6 +623,35 @@ void main() { matchesVariableGroup(start: 300, end: 331), ]); }); + + test('Creates groupings of exactly 100 if the length is a multiple of 100', + () async { + final instance = Instance( + kind: InstanceKind.kUint8ClampedList, + id: '123', + classRef: null, + identityHashCode: null, + length: 300, + ); + final variable = Variable.create( + BoundVariable( + name: 'test', + value: instance, + declarationTokenPos: null, + scopeEndTokenPos: null, + scopeStartTokenPos: null, + ), + isolateRef, + ); + + await buildVariablesTree(variable); + + expect(variable.children, [ + matchesVariableGroup(start: 0, end: 99), + matchesVariableGroup(start: 100, end: 199), + matchesVariableGroup(start: 200, end: 299), + ]); + }); } Matcher matchesVariable({ From e3e225fabd6c8c35edafa353175f8df0181157f2 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 17 Nov 2021 09:50:43 -0800 Subject: [PATCH 18/19] Respond to PR comments --- .../lib/src/debugger/debugger_model.dart | 19 +++++++------------ .../test/debugger_screen_test.dart | 11 +++++++---- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index 4655bcc1e1e..fcfe290c77a 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -697,6 +697,8 @@ List _createVariablesForFields( // InstanceRef objects have become sentinels. // TODO(jacobr): consider a new class name. This class is more just the data // model for a tree of Dart objects with properties rather than a "Variable". +// TODO(elliette): Refactor to make name, ref, text named (and potentially +// required?) parameters. Give ref a type. class Variable extends TreeNode { Variable._(this.name, ref, this.text, {int offset, int childCount}) : _ref = ref, @@ -767,10 +769,9 @@ class Variable extends TreeNode { GenericInstanceRef get ref => _ref; GenericInstanceRef _ref; - int get offset { - if (_offset != null) return _offset; - return 0; - } + /// The point to fetch the variable from (in the case of large variables that + /// we fetch only parts of at a time). + int get offset => _offset ?? 0; int _offset; @@ -779,14 +780,8 @@ class Variable extends TreeNode { final value = this.value; if (value is InstanceRef) { - if (value.kind == InstanceKind.kList) { - return value.length ?? 0; - } else if (value.kind == InstanceKind.kMap) { - return value.length ?? 0; - } else if (value.kind != null && value.kind.endsWith('List')) { - // Catch-all for Uint8ClampedList, Uint8List, Uint16List, Uint32List, - // Uint64List, Int8List, Int16List, Int32List, Int64List, Flooat32List, - // Float64List, Inst32x3List, Float32x4List, and Float64x2List types: + if (value.kind != null && + (value.kind.endsWith('List') || value.kind == InstanceKind.kMap)) { return value.length ?? 0; } } diff --git a/packages/devtools_app/test/debugger_screen_test.dart b/packages/devtools_app/test/debugger_screen_test.dart index 620a3a94946..f9cdb98cbb0 100644 --- a/packages/devtools_app/test/debugger_screen_test.dart +++ b/packages/devtools_app/test/debugger_screen_test.dart @@ -496,18 +496,21 @@ void main() { findsOneWidget, ); - // Expand list. + // Initially list is not expanded. expect(find.selectableTextContaining('0: 3'), findsNothing); expect(find.selectableTextContaining('1: 4'), findsNothing); + // Expand list. await tester.tap(listFinder); await tester.pump(); expect(find.selectableTextContaining('0: 0'), findsOneWidget); expect(find.selectableTextContaining('1: 1'), findsOneWidget); - // Expand map. + // Initially map is not expanded. expect(mapElement1Finder, findsNothing); expect(mapElement2Finder, findsNothing); + + // Expand map. await tester.tap(mapFinder); await tester.pump(); expect(mapElement1Finder, findsOneWidget); @@ -813,7 +816,7 @@ final isolateRef = IsolateRef( int refNumber = 0; String incrementRef() { - refNumber = refNumber + 1; + refNumber++; return 'ref$refNumber'; } @@ -824,7 +827,7 @@ void resetRef() { int rootNumber = 0; String incrementRoot() { - rootNumber = rootNumber + 1; + rootNumber++; return 'Root $rootNumber'; } From 4894e831c08fc0479238b842ea9405cb0a3514fe Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 18 Nov 2021 10:38:56 -0800 Subject: [PATCH 19/19] check if list --- packages/devtools_app/lib/src/debugger/debugger_model.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/devtools_app/lib/src/debugger/debugger_model.dart b/packages/devtools_app/lib/src/debugger/debugger_model.dart index fcfe290c77a..1baaa60fa81 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_model.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_model.dart @@ -781,7 +781,9 @@ class Variable extends TreeNode { final value = this.value; if (value is InstanceRef) { if (value.kind != null && - (value.kind.endsWith('List') || value.kind == InstanceKind.kMap)) { + (value.kind.endsWith('List') || + value.kind == InstanceKind.kList || + value.kind == InstanceKind.kMap)) { return value.length ?? 0; } }