RESOLVED FIXED 112596
Web Inspector: Auto expand retaining path until there are more than one retainer.
https://bugs.webkit.org/show_bug.cgi?id=112596
Summary Web Inspector: Auto expand retaining path until there are more than one retai...
Alexei Filippov
Reported 2013-03-18 10:55:11 PDT
When retainers view gets opened automatically expand retaining path until reaching an object that has more than one retainer.
Attachments
Patch (2.42 KB, patch)
2013-03-18 10:59 PDT, Alexei Filippov
no flags
Patch (12.84 KB, patch)
2013-03-20 07:31 PDT, Alexei Filippov
no flags
Patch (15.77 KB, patch)
2013-03-21 02:18 PDT, Alexei Filippov
no flags
Archive of layout-test-results from gce-cr-linux-01 (998.57 KB, application/zip)
2013-03-21 03:02 PDT, WebKit Review Bot
no flags
Patch (15.90 KB, patch)
2013-03-21 08:29 PDT, Alexei Filippov
no flags
Patch (16.56 KB, patch)
2013-03-27 08:03 PDT, Alexei Filippov
no flags
Alexei Filippov
Comment 1 2013-03-18 10:59:58 PDT
Pavel Feldman
Comment 2 2013-03-19 09:38:10 PDT
Comment on attachment 193606 [details] Patch Please provide a test.
Alexei Filippov
Comment 3 2013-03-20 07:31:23 PDT
Alexei Filippov
Comment 4 2013-03-20 07:59:04 PDT
(In reply to comment #2) > (From update of attachment 193606 [details]) > Please provide a test. Added a test.
Yury Semikhatsky
Comment 5 2013-03-20 08:05:59 PDT
(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 6 2013-03-20 08:10:37 PDT
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
Comment 7 2013-03-21 02:18:07 PDT
Alexei Filippov
Comment 8 2013-03-21 02:22:42 PDT
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
Comment 9 2013-03-21 02:23:56 PDT
(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 10 2013-03-21 03:02:05 PDT
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
Comment 11 2013-03-21 03:02:09 PDT
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
Comment 12 2013-03-21 08:29:18 PDT
Yury Semikhatsky
Comment 13 2013-03-27 02:11:10 PDT
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 14 2013-03-27 02:23:24 PDT
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 15 2013-03-27 02:28:04 PDT
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
Comment 16 2013-03-27 08:03:52 PDT
Alexei Filippov
Comment 17 2013-03-27 08:04:23 PDT
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 18 2013-03-28 02:48:16 PDT
Comment on attachment 195323 [details] Patch Clearing flags on attachment: 195323 Committed r147090: <http://trac.webkit.org/changeset/147090>
WebKit Review Bot
Comment 19 2013-03-28 02:48:20 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.