Bug 112596 – Web Inspector: Auto expand retaining path until there are more than one retainer.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Alexei Filippov]

When retainers view gets opened automatically expand retaining path until reaching an object that has more than one retainer.

[Alexei Filippov]

Created attachment 193606 [details]
Patch

[Pavel Feldman]

Comment on attachment 193606 [details]
Patch

Please provide a test.

[Alexei Filippov]

Created attachment 194050 [details]
Patch

[Alexei Filippov]

(In reply to comment #2)
> (From update of attachment 193606 [details])
> Please provide a test.

Added a test.

[Yury Semikhatsky]

(In reply to comment #0)
> When retainers view gets opened automatically expand retaining path until reaching an object that has more than one retainer.

What about expanding shortest path to the Window?

[Yury Semikhatsky]

Comment on attachment 194050 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194050&action=review

> Source/WebCore/inspector/front-end/HeapSnapshotDataGrids.js:508
> +            if (this.children.length === 1) {

Can it happen that we didn't receive response from the worker and don't know number of children here?

[Alexei Filippov]

Created attachment 194206 [details]
Patch

[Alexei Filippov]

Comment on attachment 194050 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194050&action=review

>> Source/WebCore/inspector/front-end/HeapSnapshotDataGrids.js:508
>> +            if (this.children.length === 1) {
> 
> Can it happen that we didn't receive response from the worker and don't know number of children here?

AFAICT populateComplete event arrives on response from the worker. If for some reason there's no response it just stops expanding the chain.

[Alexei Filippov]

(In reply to comment #5)
> (In reply to comment #0)
> > When retainers view gets opened automatically expand retaining path until reaching an object that has more than one retainer.
> 
> What about expanding shortest path to the Window?

Good idea.
However it sounds like a task for a separate CL.

[WebKit Review Bot]

Comment on attachment 194206 [details]
Patch

Attachment 194206 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17112602

New failing tests:
inspector/profiler/heap-snapshot-summary-retainers.html
platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-month-popup.html

[WebKit Review Bot]

Created attachment 194212 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

[Alexei Filippov]

Created attachment 194265 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 194265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review

> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:106
> +                setTimeout(step5.bind(null, retainersRoot), 500);

This is bound to flake. Please replace the timeout with some deterministic condition.

> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183
> +                setTimeout(step5.bind(null, retainersRoot), 500);

Ditto.

[Alexei Filippov]

Comment on attachment 194265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review

>> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:106
>> +                setTimeout(step5.bind(null, retainersRoot), 500);
> 
> This is bound to flake. Please replace the timeout with some deterministic condition.

ok

>> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183
>> +                setTimeout(step5.bind(null, retainersRoot), 500);
> 
> Ditto.

This one is not a flake. It is a deterministic poll loop. It will wait until there's at least 20 items expanded. See lines 189-190.

[Yury Semikhatsky]

Comment on attachment 194265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review

>>> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183
>>> +                setTimeout(step5.bind(null, retainersRoot), 500);
>> 
>> Ditto.
> 
> This one is not a flake. It is a deterministic poll loop. It will wait until there's at least 20 items expanded. See lines 189-190.

You should be able to listen to some event instead of polling with 500ms interval.

[Alexei Filippov]

Created attachment 195323 [details]
Patch

[Alexei Filippov]

Comment on attachment 194265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review

>>>> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183
>>>> +                setTimeout(step5.bind(null, retainersRoot), 500);
>>> 
>>> Ditto.
>> 
>> This one is not a flake. It is a deterministic poll loop. It will wait until there's at least 20 items expanded. See lines 189-190.
> 
> You should be able to listen to some event instead of polling with 500ms interval.

done

[WebKit Review Bot]

Comment on attachment 195323 [details]
Patch

Clearing flags on attachment: 195323

Committed r147090: <http://trac.webkit.org/changeset/147090>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.