Implement HeapSnapshot.describeNode

RESOLVED FIXED in Firefox 46

Status

()

Firefox
Developer Tools: Memory
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: fitzgen, Assigned: fitzgen)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

This commit implements the HeapSnapshot.describeNode method which allows chrome
JS code to request a description of a given node as specified by our existing
"breakdown" language. This description can be used to generate a human-readable
label for the node.
Created attachment 8697479 [details] [diff] [review]
Implement HeapSnapshot.describeNode

This commit implements the HeapSnapshot.describeNode method which allows chrome
JS code to request a description of a given node as specified by our existing
"breakdown" language. This description can be used to generate a human-readable
label for the node.
Attachment #8697479 - Flags: review?(mrbkap)
Assignee: nobody → nfitzgerald
Blocks: 1229229, 961331
Status: NEW → ASSIGNED
Depends on: 1231763
Blocks: 1232390
Depends on: 1233096
Created attachment 8699641 [details] [diff] [review]
Implement HeapSnapshot.describeNode

Fix the test to work after small and mostly unrelated changes in bug 1233096.
Attachment #8699641 - Flags: review?(mrbkap)
Attachment #8697479 - Attachment is obsolete: true
Attachment #8697479 - Flags: review?(mrbkap)
Comment on attachment 8699641 [details] [diff] [review]
Implement HeapSnapshot.describeNode

Review of attachment 8699641 [details] [diff] [review]:
-----------------------------------------------------------------

This seems reasonable.

::: devtools/shared/heapsnapshot/HeapSnapshot.cpp
@@ +526,5 @@
> +HeapSnapshot::DescribeNode(JSContext* cx, JS::HandleObject breakdown, uint64_t nodeId,
> +                           JS::MutableHandleValue rval, ErrorResult& rv) {
> +  MOZ_ASSERT(breakdown);
> +  JS::RootedValue breakdownVal(cx, JS::ObjectValue(*breakdown));
> +  JS::ubi::CountTypePtr rootType = JS::ubi::ParseBreakdown(cx, breakdownVal);

IMO it'd be nice to 'using namespace JS; using namespace JS::ubi;' either in this file or in this function to avoid having to repeat the namespace everywhere.

::: dom/webidl/HeapSnapshot.webidl
@@ +58,5 @@
>    any takeCensus(object? options);
>  
>    /**
> +   * Describe `node` with the specified `breakdown`. See the comment above
> +   * `takeCensus` or `js/src/dom/Debugger/Debugger.Memory.md` for detailed

Nit: .../doc/... (instead of "dom").
Attachment #8699641 - Flags: review?(mrbkap) → review+
Created attachment 8700128 [details] [diff] [review]
Implement HeapSnapshot.describeNode

This commit implements the HeapSnapshot.describeNode method which allows chrome
JS code to request a description of a given node as specified by our existing
"breakdown" language. This description can be used to generate a human-readable
label for the node.
Attachment #8700128 - Flags: review+
Attachment #8699641 - Attachment is obsolete: true

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/85f5b4e4c9a2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46

Comment 9

2 years ago
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel	        beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected
You need to log in before you can comment on or make changes to this bug.